Skip to content
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

Consider using normal random instead of crypto random for IDs #1367

Closed
anuraaga opened this issue Nov 25, 2020 · 2 comments
Closed

Consider using normal random instead of crypto random for IDs #1367

anuraaga opened this issue Nov 25, 2020 · 2 comments
Labels
area:trace Part of OpenTelemetry tracing pkg:SDK Related to an SDK package
Milestone

Comments

@anuraaga
Copy link

Currently, cryptographic randoms are used for IDs I think.

https://github.com/open-telemetry/opentelemetry-go/blob/master/sdk/trace/trace.go#L18

This generally has a far larger impact on the CPU usage of a user's app than normal random (I've seen 10% of CPU used on ID generation before when using crypto random). You can see some benchmarks and discussion for JS here

open-telemetry/opentelemetry-js#1334

It'd probably be good to use normal random here too, like in JS, Java, Python (when I filed the JS issue, I misread the Go code thinking it was already using normal random ;) ).

/cc @wilguo

@MrAlias MrAlias added area:trace Part of OpenTelemetry tracing pkg:SDK Related to an SDK package priority:p3 labels Dec 3, 2020
@MrAlias MrAlias added this to the RC1 milestone Dec 3, 2020
@Aneurysm9
Copy link
Member

I'm not sure this change is useful. We're currently only using the crypto/rand package for obtaining seed data for a PRNG from the math/rand package. ID generation is performed with a PRNG from the math/rand package already.

@anuraaga
Copy link
Author

anuraaga commented Jan 3, 2021

Ah got it, didn't realize the crand is only for the seed. Looks good then.

@anuraaga anuraaga closed this as completed Jan 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:trace Part of OpenTelemetry tracing pkg:SDK Related to an SDK package
Projects
None yet
Development

No branches or pull requests

3 participants