-
Notifications
You must be signed in to change notification settings - Fork 439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ddtrace/tracer: use math/rand/v2 for SpanID and TraceID #2689
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
knusbaum
force-pushed
the
knusbaum/new-rand
branch
from
May 9, 2024 16:48
4ee1a83
to
aee9c87
Compare
With the new version 2 of the math/rand standard library package, there is no longer a need to perform our own complicated initialization of an RNG, and in fact, we should avoid doing so. See more details about the new rand package here: https://go.dev/blog/randv2 Instead, we can rely on the global RNG provided by the Go runtime. We previously chose not to do that because we could not rely on the global RNG to be initialized, meaning there would likely be many trace and span ID clashes. Likewise, it was a bad idea to initialize the RNG ourselves, since as a library it's bad practice to mutate global state in a way that will have visible effects on the application. We previously chose to initialize our own RNG instance for the tracer and synchronize access to it. Due to the low entropy of the RNG seed (it only used 32 bits of the 64 provided) we also chose to xor the produced random numbers with the current unix time in order to decrease collisions. math/rand/v2 removes the need to do either of those things. The new global RNG is automatically seeded at startup with enough entropy to remove the need to xor with a timestamp. It also has the added benefit of being per-goroutine, meaning there is no synchronization involved. This PR adds a new implementation of the span/trace ID generator which will be used for all Go applications compiled with go1.22.0 and later. The old implementation remains for older versions of Go, but should be removed once those versions fall out of the support window. I've also added some benchmarks and used them to compare the old and new implementations. The benefits of this change are a smaller implementation and less historical baggage, as well as improved performance if there are lots of threads trying to generate spans. ``` goos: linux goarch: amd64 pkg: gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer cpu: 13th Gen Intel(R) Core(TM) i5-1340P │ old.bench │ new.bench │ │ sec/op │ sec/op vs base │ StartSpan-16 1.681µ ± 3% 1.662µ ± 3% ~ (p=0.447 n=10) StartSpanConcurrent-16 4.673µ ± 1% 3.955µ ± 3% -15.37% (p=0.000 n=10) GenSpanID-16 13.840n ± 0% 4.502n ± 1% -67.47% (p=0.000 n=10) geomean 477.3n 309.3n -35.20% │ old.bench │ new.bench │ │ B/op │ B/op vs base │ StartSpan-16 1.603Ki ± 0% 1.603Ki ± 0% ~ (p=1.000 n=10) ¹ StartSpanConcurrent-16 16.05Ki ± 0% 16.04Ki ± 0% -0.03% (p=0.000 n=10) GenSpanID-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ geomean ² -0.01% ² ¹ all samples are equal ² summaries must be >0 to compute geomean │ old.bench │ new.bench │ │ allocs/op │ allocs/op vs base │ StartSpan-16 16.00 ± 0% 16.00 ± 0% ~ (p=1.000 n=10) ¹ StartSpanConcurrent-16 160.0 ± 0% 160.0 ± 0% ~ (p=1.000 n=10) ¹ GenSpanID-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ geomean ² +0.00% ² ¹ all samples are equal ² summaries must be >0 to compute geomean ```
darccio
approved these changes
May 10, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With the new version 2 of the math/rand standard library package, there is no longer a need to perform our own complicated initialization of an RNG, and in fact, we should avoid doing so.
See more details about the new rand package here: https://go.dev/blog/randv2
Instead, we can rely on the global RNG provided by the Go runtime. We previously chose not to do that because we could not rely on the global RNG to be initialized, meaning there would likely be many trace and span ID clashes. Likewise, it was a bad idea to initialize the RNG ourselves, since as a library it's bad practice to mutate global state in a way that will have visible effects on the application.
We previously chose to initialize our own RNG instance for the tracer and synchronize access to it. Due to the low entropy of the RNG seed (it only used 32 bits of the 64 provided) we also chose to xor the produced random numbers with the current unix time in order to decrease collisions.
math/rand/v2 removes the need to do either of those things. The new global RNG is automatically seeded at startup with enough entropy to remove the need to xor with a timestamp. It also has the added benefit of being per-goroutine, meaning there is no synchronization involved.
This PR adds a new implementation of the span/trace ID generator which will be used for all Go applications compiled with go1.22.0 and later. The old implementation remains for older versions of Go, but should be removed once those versions fall out of the support window.
I've also added some benchmarks and used them to compare the old and new implementations.
The benefits of this change are a smaller implementation and less historical baggage, as well as improved performance if there are lots of threads trying to generate spans.
Motivation
Unsure? Have a question? Request a review!