Traces sample rate cannot be disabled

[koenhoeijmakers]

Environment

sentry/sentry-laravel: 3.0.1
laravel/framework: 9.39.0

Steps to Reproduce

1. Use the default config file containing: 'traces_sample_rate' => (float) (env('SENTRY_TRACES_SAMPLE_RATE', 0.0)),

Expected Result

I would expect Performance tracing to be disabled

Actual Result

It is not disabled and we're almost at the point where we have to upgrade our sentry plan because we're running out of events rapidly

[koenhoeijmakers]

Update:

Seems that our front-end has enabled tracing with a 0.2 sample rate which in turn sends a sentry-trace header which therefore enables tracing in our back-end.

[koenhoeijmakers]

Update:

Reopening because it seems that the sentry-trace header force overrides the back-end settings. (as added in getsentry/sentry-laravel#600)

This also seems to open up an issue where i could spam an application their sentry quota by adding a fake sentry-trace header in all of my requests.

What i think should happen:
If our config defines that no sampling should happen, then no sampling should happen. The header should not override our config as this gives us no control over whether sampling should happen and possibly opens up an issue where external users could send a faux sentry-trace header to ruin our quota.

[cleptric]

I would advise updating your tracingOrigins config in your JS application, to not send sentry-trace & baggage headers to your BE if you do not want to enable traces.
The change in behavior was to fix a bug, as we have to take the sampling decision of the head SDK into account (see getsentry/sentry-laravel#595).

As your FE DSN is public, there are similar concerns when it comes to abuse, you can read more about it here https://docs.sentry.io/product/sentry-basics/dsn-explainer/#dsn-utilization

[cleptric]

To add to this, you can also disable tracing by setting a traces_sampler in your Laravel app, which might be the better option in your case.

https://docs.sentry.io/platforms/php/guides/laravel/configuration/options/#traces-sampler
https://docs.sentry.io/platforms/php/guides/laravel/performance/#configure-the-sample-rate

[koenhoeijmakers]

Doesn't sound like good design if anyone sending an extra header can use up our quota for sole back-end projects (without a known dsn) to be honest. I'd prefer just having a boolean setting that disables sampling and can't be overridden by a randomly sent header.

Something like allow_sampling and allow_parent_sampling to me sound like a proper addition to give us fine grained control.

Example:
Lets say we host a weather api that is using sentry and is being used by a ton of front-end systems that we have no control over.

Now if those front-end systems also use sentry and send us a trace header (as default by sentryjs sdk) this will mean there is no way to prevent their application from using up the quota in our sentry environment.

Also:
If we were to add a traces_sampler in the config that would prevent us from caching our configs.

[stayallive]

You can add a traces_sampler like this:

<?php

namespace App;

use Sentry\Tracing\SamplingContext;

class Sentry 
{
    public static function tracesSampler(SamplingContext $context): float
    {
        return 0.0;
    }
}

And in your config:

<?php

return [
    'traces_sampler' => [App\Sentry::class, 'tracesSampler'],
];

This works with config caching 😄

This should work for now, until we figure out what we can do to prevent this.

[cleptric]

I moved this to the core SDK, as the issue needs to be fixed there.

I think we are missing the following behavior:

• By default, traces_sample_rate should be null, so we do exit early and do not take anything else into account.
• If a user applies traces_sample_rate 0 or higher, we check in the order of traces_sampler -> parent_sampled -> traces_sample_rate. See https://github.com/getsentry/sentry-python/blob/e6238d828e11d63833b9a1400aaf8286b05d1c02/sentry_sdk/tracing.py#L763-L772

@stayallive How does this sound to you?

[mfb]

So zero doesn't really mean zero I guess? I will have to document this carefully in my downstream package :) Having a toggle + sample rate seems potentially more intuitive.

[stayallive]

@cleptric this sounds good to me, as long as we document this correctly. Are you aware if other SDKs handle it a similar way?

Currently the docs aren't a 100% correct either too: https://docs.sentry.io/platforms/php/configuration/options/#tracing-options

[cleptric]

Yes, our Python SDK uses None https://github.com/getsentry/sentry-python/blob/e6238d828e11d63833b9a1400aaf8286b05d1c02/sentry_sdk/tracing_utils.py#L113##

[cleptric]

Fixed in 3.12.0 https://github.com/getsentry/sentry-php/releases/tag/3.12.0