fix(sentry): Add sampler to fuction to block /health transactions - #232
Conversation
|
Actually sorry @gagantrivedi but this needs to move out of the
I think this code should live in Having said that - I've tested this and its working for me - stripping out health endpoint checks correctly. It just needs moving out of integrations. |
|
I don't really see any reason why it needs to move out of the |
|
OK lets leave it here - but add that test pls @gagantrivedi |
Sure, but is this working for you locally? i.e: can you see transaction generated locally on the sentry dashboard? |
|
Yes - transactions other than /health show up |
|
|
||
| def test_block_health_sampler_empty_context_returns_defaut_sample_rate(): | ||
| # Given | ||
| settings.SENTRY_TRACE_SAMPLE_RATE = 10 |
There was a problem hiding this comment.
It's better to use the @override_settings() decorator to do this so your tests become something like the following:
sample_rate = 10
@override_settings(SENTRY_TRACE_SAMPLE_RATE=sample_rate)
def test_block_health_sampler_empty_context_returns_defaut_sample_rate():
assert block_health_sampler({}) == sample_rateSee documentation links here:
matthewelwell
left a comment
matthewelwell
left a comment
There was a problem hiding this comment.
Some minor changes so that we're reusing the variable we defined as much as possible.
| sample_rate = block_health_sampler({}) | ||
|
|
||
| # Then | ||
| assert sample_rate == 10 |
There was a problem hiding this comment.
| assert sample_rate == 10 | |
| assert sample_rate == SAMPLE_RATE |
| sample_rate = block_health_sampler(ctx) | ||
|
|
||
| # Then | ||
| assert sample_rate == 10 |
There was a problem hiding this comment.
| assert sample_rate == 10 | |
| assert sample_rate == SAMPLE_RATE |
3 participants
No description provided.