is_valid_sample_rate may be too strict

[timharsch]

How do you use Sentry?

Sentry Saas (sentry.io)

Version

1.5.6

Steps to Reproduce

One day sentry stopped sending transactions to sentry.io. After much digging I found the issue to be here:

sentry-python/sentry_sdk/tracing.py

Line 668 in 0ba75fe

if not is_valid_sample_rate(sample_rate):

We have a custom traces_sampler function that reads in from a simple JSON file and sets the sample rate based on the endpoint. An example of our JSON:

{
  "^$": 0.05,
  "default_sample_rate": 1.0
}

We limit our root path /, because it gets polled by our load balancer once a minute. And the endpoint is blindingly simple and uninteresting, so we don't want to track all events and blow through our quota.

The problem came about when I switch from the json package to parse the json, to the json_encoder package, which generally is more robust json parser. The issue is that the package json will create a type of float when it deserializes the json, json_encoder will create a type of Decimal

Expected Result

• sample_rate with type Decimal should be sufficient for a return type of the traces_sampler function
• sentry documentation should state the exact allowable types for the traces_sampler function

Actual Result

transactions will be skipped if the type returned by traces_sampler function is not of type float.

[antonpirker]

Hey @timharsch !

Thanks for writing in. A really nice solution you have there for having dynamic sample rates for your endpoints.

I think there would be an easy fix by just adding a check for decimal here:
https://github.com/getsentry/sentry-python/blob/master/sentry_sdk/tracing_utils.py#L183

You can supply a pull request with a fix, if you need this change fast.

I have put this in the internal backlog at low priority so it will take quite some time until we can tackle this, because we have a lot on our plates.

[timharsch]

Hmm.. not sure why you are suggesting a change in the record_sql_queries function. It doesn't seem to relate.

I believe the fix should be to go from this code on line 132

sentry-python/sentry_sdk/tracing_utils.py

Line 132 in 0ba75fe

if not isinstance(rate, Real) or math.isnan(rate):

if not isinstance(rate, Real) or math.isnan(rate):
to
if not isinstance(rate, Real) or not isinstance(rate, Decimal) or math.isnan(rate):

The float cast on line 141 should cast the Decimal value to float.

[Arvind2222]

@timharsch - Can you please review - #1672

[sl0thentr0py]

fixed by #1672