Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Commit

Permalink
Defer IDGenerator initialization until first use (#1228)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jlebon authored Oct 6, 2020
1 parent 5bb2445 commit 3fb168f
Showing 1 changed file with 23 additions and 12 deletions.
35 changes: 23 additions & 12 deletions trace/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand Down

0 comments on commit 3fb168f

Please sign in to comment.