getsentry/sentry-symfony

Using environment variables `%env(...)` in `sentry.yaml` does not work

liviucmg opened this issue · 3 comments

How do you use Sentry?

Sentry SaaS (sentry.io)

SDK version

4.9.0

Steps to reproduce

We exceeded our Sentry quota after some unusually high usage, so we started investigating. Looks like it started right when we upgraded sentry/sentry-symfony from v4 to v5. We're using environment variables in our sentry.yaml file like so:

    sentry:
        tracing:
            enabled: '%env(bool:SENTRY_TRACING)%'
        options:
            traces_sample_rate: '%env(float:SENTRY_PHP_TRACES_SAMPLE_RATE)%'
            ...

The variables are in our .env file:

SENTRY_TRACING=false # or true for production
SENTRY_PHP_TRACES_SAMPLE_RATE=0 # or 0.01 for production

This didn't actually turn off tracing, which is unfortunate since we only want it turned on in production, and all of our other environments like dev/testing/staging were supposed to have it turned off.

The root cause is in src/DependencyInjection/SentryExtension.php which doesn't receive the fully compiled variables. Here's a hackish solution, with some var_dumps to understand what's going on:

    private function registerTracingConfiguration(ContainerBuilder $container, array $config): void
    {
        // Outputs "env_da73b6f612a932f0_bool_SENTRY_TRACING_57d3ad6b7e96da0e8f15dbe2298f150b"
        // When evaluated, this string is true-ish, so Sentry enables tracing.
        var_dump($config['enabled']);

        // We need to resolve any parameters first.
        // Symfony doesn't support "bool:" in this stage, so we remove it and cast it to bool manually.
        $enabled = $container->resolveEnvPlaceholders(str_replace($config['enabled'], 'env(bool:', 'env('), true);
        if (is_string($enabled)) {
            $enabled = strtolower($enabled) === 'true';
        }

        // Outputs false.
        var_dump($enabled);

        $container->setParameter('sentry.tracing.enabled', $enabled);

        if (!$enabled) {
            $container->removeDefinition(TracingRequestListener::class);
            $container->removeDefinition(TracingSubRequestListener::class);
            $container->removeDefinition(TracingConsoleListener::class);

            return;
        }

        $container->getDefinition(TracingConsoleListener::class)->replaceArgument(1, $config['console']['excluded_commands']);
    }

Same thing as a diff:

    private function registerTracingConfiguration(ContainerBuilder $container, array $config): void
    {
+        $enabled = $container->resolveEnvPlaceholders(str_replace($config['enabled'], 'env(bool:', 'env('), true);
+        if (is_string($enabled)) {
+            $enabled = strtolower($enabled) === 'true';
+        }
+
-        $container->setParameter('sentry.tracing.enabled', $config['enabled']);
+        $container->setParameter('sentry.tracing.enabled', $enabled);
+
-        if (!$this->isConfigEnabled($container, $config)) {
+        if (!$enabled) {
            $container->removeDefinition(TracingRequestListener::class);
            $container->removeDefinition(TracingSubRequestListener::class);
            $container->removeDefinition(TracingConsoleListener::class);

            return;
        }

        $container->getDefinition(TracingConsoleListener::class)->replaceArgument(1, $config['console']['excluded_commands']);
    }

The same thing should be done for registerDbalTracingConfiguration, registerTwigTracingConfiguration, registerCacheTracingConfiguration, and probably more.

Similarly, I believe that the sample rate was parsed as 1 instead of our SENTRY_PHP_TRACES_SAMPLE_RATE=0.01. Might be a duplicate of #877. Here's our usage after we upgraded to v5 on September 18:

Image

Expected result

enabled: '%env(bool:SENTRY_TRACING)%' with SENTRY_TRACING=false should fully disable tracing.

traces_sample_rate: '%env(float:SENTRY_PHP_TRACES_SAMPLE_RATE)%' with SENTRY_PHP_TRACES_SAMPLE_RATE=0.01 should set the sample rate to 0.01.

Actual result

enabled: '%env(bool:SENTRY_TRACING)%' with SENTRY_TRACING=false sets tracing on.

traces_sample_rate: '%env(float:SENTRY_PHP_TRACES_SAMPLE_RATE)%' with SENTRY_PHP_TRACES_SAMPLE_RATE=0.01 sets tracing to 1.

That is interesting, I wouldn't have expected us to do much special handling to allow for using env variables. I'll look into this! Appreciate the detailed report!

The problem is indeed as indicated. However it seems like we are using the values "too early" (as you mentioned) because in that stage of the container compilation env vars are not properly processed as it's meant as a static build step and env vars are resolved at runtime.

We can work around this like you are proposing but that feels a bit odd to do, it is a possibility though. I was hoping maybe @ste93cry or @Jean85 might have an excellent suggestion on how to resolve this or that the proposed solution might be the best way to go here however that does feel like re-inventing the environment compilation a bit...

IIRC there's a standard solution for this kind of issues, and Symfony itself had to fix it in multiple places and multiple "standard" bundles configurations when they transitioned to using env vars, but I can't recall it. I would have to dig in the history of the Doctrine bundle or the Framework Bundle...