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

sampling: use session id for source of randomness #185

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

seemk
Copy link
Contributor

@seemk seemk commented Nov 24, 2023

Use the session ID bytes for the source of randomness when making a sampling decision.

The algorithm for the session ID value calculation is the same as is in the JS RUM SDK. This fixes the issue where, if sampling is used with the iOS SDK and inside a WebView, the SDKs come to a different sampling decision due to differing algorithms.

Note: session ID callbacks should have the session ID as a parameter, but this would be a breaking change in the public API, so I decided to keep the plain callbacks for now.

@seemk seemk requested review from a team as code owners November 24, 2023 11:45
Copy link
Contributor

@johnbley johnbley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked android?

@seemk
Copy link
Contributor Author

seemk commented Nov 27, 2023

Have you checked android?

Android has the same issue. The lower 8 bytes of the session ID are converted to a number, this differs from the JS RUM implementation which xors the whole 16 bytes in 4 byte chunks.

@seemk seemk merged commit 9a137be into main Nov 27, 2023
8 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 27, 2023
@johnbley johnbley deleted the use-traceid-based-sampler branch February 26, 2024 13:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants