summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJonathan Lebon <jonathan@jlebon.com>2020-10-06 19:12:41 -0400
committerGitHub <noreply@github.com>2020-10-07 10:12:41 +1100
commit3fb168f674736c026e623310bfccb0691e6dec8a (patch)
tree91f9ebed00270e046f90333d906650cc97e66b03
parent5bb2445655b9222ddc851834f89ad746d1b5ed5a (diff)
downloadopencensus-go-3fb168f674736c026e623310bfccb0691e6dec8a.tar.gz
Defer IDGenerator initialization until first use (#1228)
Initializing the `IDGenerator` from `init()` means that any downstream project which transitively imports this package somewhere in its dependency tree will incur `getrandom()` syscalls it has no control over at startup. This causes problems for us in [Ignition](https://github.com/coreos/ignition), where we're transitively pulling in this package via cloud.google.com/go/storage. Ignition runs very early during the boot process, which means that even though this isn't using `GRND_RANDOM`, the `getrandom` syscall can block until the entropy pool is ready. This is a real problem when running in VMs on systems which don't provide a virtualized RNG device (such as VMware) or which lack RDRAND. I can't find a good reference for this, but I think in general it should be considered good practice to avoid I/O like this in `init()` in favour of a more lazy approach (or an explicit `Initialize()` function for clients to call). Otherwise, *every* program which pulls in the package will pay for it, whether or not they intend to actually make use of the functionality those syscalls are priming. (While it's not relevant here, another advantage of not using `init()` for this is that I/O is fallible, and `init()` semantics means errors can't be handled sanely.) Let's rework things here so that we don't actually initialize the `IDGenerator` fields until the first time it's used.
-rw-r--r--trace/trace.go35
1 files changed, 23 insertions, 12 deletions
diff --git a/trace/trace.go b/trace/trace.go
index 125e2cd..daf8955 100644
--- a/trace/trace.go
+++ b/trace/trace.go
@@ -206,6 +206,10 @@ func startSpanInternal(name string, hasParent bool, parent SpanContext, remotePa
span.spanContext = parent
cfg := config.Load().(*Config)
+ if gen, ok := cfg.IDGenerator.(*defaultIDGenerator); ok {
+ // lazy initialization
+ gen.init()
+ }
if !hasParent {
span.spanContext.TraceID = cfg.IDGenerator.NewTraceID()
@@ -534,20 +538,9 @@ func (s *Span) String() string {
var config atomic.Value // access atomically
func init() {
- gen := &defaultIDGenerator{}
- // initialize traceID and spanID generators.
- var rngSeed int64
- for _, p := range []interface{}{
- &rngSeed, &gen.traceIDAdd, &gen.nextSpanID, &gen.spanIDInc,
- } {
- binary.Read(crand.Reader, binary.LittleEndian, p)
- }
- gen.traceIDRand = rand.New(rand.NewSource(rngSeed))
- gen.spanIDInc |= 1
-
config.Store(&Config{
DefaultSampler: ProbabilitySampler(defaultSamplingProbability),
- IDGenerator: gen,
+ IDGenerator: &defaultIDGenerator{},
MaxAttributesPerSpan: DefaultMaxAttributesPerSpan,
MaxAnnotationEventsPerSpan: DefaultMaxAnnotationEventsPerSpan,
MaxMessageEventsPerSpan: DefaultMaxMessageEventsPerSpan,
@@ -571,6 +564,24 @@ type defaultIDGenerator struct {
traceIDAdd [2]uint64
traceIDRand *rand.Rand
+
+ initOnce sync.Once
+}
+
+// init initializes the generator on the first call to avoid consuming entropy
+// unnecessarily.
+func (gen *defaultIDGenerator) init() {
+ gen.initOnce.Do(func() {
+ // initialize traceID and spanID generators.
+ var rngSeed int64
+ for _, p := range []interface{}{
+ &rngSeed, &gen.traceIDAdd, &gen.nextSpanID, &gen.spanIDInc,
+ } {
+ binary.Read(crand.Reader, binary.LittleEndian, p)
+ }
+ gen.traceIDRand = rand.New(rand.NewSource(rngSeed))
+ gen.spanIDInc |= 1
+ })
}
// NewSpanID returns a non-zero span ID from a randomly-chosen sequence.