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

Update sample rate in DSC #4018

Merged
merged 10 commits into from
Feb 12, 2025
Merged

Update sample rate in DSC #4018

merged 10 commits into from
Feb 12, 2025

Conversation

sentrivana
Copy link
Contributor

@sentrivana sentrivana commented Feb 6, 2025

  • update sample_rate in DSC after the initial sampling decision is made
  • fix some typos

Part of #3999

Copy link

codecov bot commented Feb 6, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
22190 1 22189 5756
View the top 1 failed tests by shortest run time
tests.test_basics test_classmethod_tracing
Stack Traces | 0.168s run time
tests/test_basics.py:946: in test_classmethod_tracing
    assert fake_start_child.call_count == 1
E   AssertionError: assert 2 == 1
E    +  where 2 = <MagicMock name='mock.start_child' id='140163119671328'>.call_count

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

meh
@sentrivana sentrivana marked this pull request as ready for review February 6, 2025 14:37
Comment on lines +1048 to +1056
propagation_context = self.get_active_propagation_context()
if propagation_context:
dsc = propagation_context.dynamic_sampling_context
if dsc is not None:
dsc["sample_rate"] = str(transaction.sample_rate)
if transaction._baggage:
transaction._baggage.sentry_items["sample_rate"] = str(
transaction.sample_rate
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason, the baggage on the transaction is independent from the propagation context? In any case, I'm changing both here...

Copy link
Member

Choose a reason for hiding this comment

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

I think that's ok. transaction._baggage is for the current transaction and on the scope it is for future transactions. So looks correct.

Copy link
Member

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

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

This should work. Nice!

Comment on lines +1048 to +1056
propagation_context = self.get_active_propagation_context()
if propagation_context:
dsc = propagation_context.dynamic_sampling_context
if dsc is not None:
dsc["sample_rate"] = str(transaction.sample_rate)
if transaction._baggage:
transaction._baggage.sentry_items["sample_rate"] = str(
transaction.sample_rate
)
Copy link
Member

Choose a reason for hiding this comment

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

I think that's ok. transaction._baggage is for the current transaction and on the scope it is for future transactions. So looks correct.

@sentrivana sentrivana merged commit 221f105 into master Feb 12, 2025
151 checks passed
@sentrivana sentrivana deleted the ivana/update-sample-rate branch February 12, 2025 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants