sentry-symfony icon indicating copy to clipboard operation
sentry-symfony copied to clipboard

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

Open liviucmg opened this issue 1 year ago • 9 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.

liviucmg avatar Sep 25 '24 19:09 liviucmg

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!

stayallive avatar Sep 26 '24 10:09 stayallive

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...

stayallive avatar Oct 07 '24 13:10 stayallive

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...

Jean85 avatar Oct 08 '24 05:10 Jean85

Have you checked how this interacts with resolve: environment variable processor? It's supposed to unpack the "gibberish" value you presented into an actual value.

It's supposed to work with environment variables containing parameters, but maybe the addition of bool: and float: makes it start trying to make it an implicit parameter.

Steveb-p avatar Nov 26 '24 09:11 Steveb-p

The "issue" is that we are using the values in the "build" (?) step of the container. Here the variables are not yet resolved (which makes sense). It looks like we might be able to reimplement parts of this ourselves but fundamentally the way we configure the services makes it impractical to use dynamic variables. I have also not been able to find a good example yet elsewhere in other bundles.

stayallive avatar Nov 27 '24 10:11 stayallive

Hi @liviucmg! After reading through the conversation I agree with @stayallive that it would be impractical for us to resolve the env variables at this stage.

registerTracingConfiguration runs only at compile time and is cached afterwards, so if we would implement env var resolving at this step, it could create a confusing situation where one runs it and it creates a cached version with the current env value and after changing the env variable and running it again the behaviour is not changed because it was compiled with the initial value.

A better approach imo would be to use environment specific config overrides that symfony offers, like when@prod.

I'm closing this issue now, feel free to reply if you think this should remain open

Litarnus avatar Aug 12 '25 14:08 Litarnus

Hi @Litarnus, thanks for replying.

the behaviour is not changed because it was compiled with the initial value.

I think that would be ok, because whenever I change .env.local in production, I have to run composer dump-env and bin/console cache:clear. Also, in dev mode I believe the cache is invalidated automatically.

A better approach imo would be to use environment specific config overrides that symfony offers, like when@prod.

At least for our use case this doesn't work, because when@prod targets all production environments. In our case we have let's say 20 different single-tenant deployments, and we need to change parameters for each of them individually. We do that with .env.local for all other 3rd party libraries/bundles as well as our own custom code. That's why I'm thinking Sentry should also support this, since it seems to be common practice.

feel free to reply if you think this should remain open

It should be at least documented. I expected it to work with environment variables, and it took a while to figure out what was going on.

If it helps anyone, here's our current (hackish) solution using a custom extension with sentry/sentry-symfony version 5.3.0:

// Kernel.php
<?php

namespace App;

use App\HelperNonServices\CustomSentryExtension;
use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\HttpKernel\Kernel as BaseKernel;

class Kernel extends BaseKernel implements CompilerPassInterface
{
    use MicroKernelTrait;

    protected function build(ContainerBuilder $container): void
    {
        $container->registerExtension(new CustomSentryExtension());
        parent::build($container);
    }

    public function process(ContainerBuilder $container)
    {
    }
}
// CustomSentryExtension.php
<?php

declare(strict_types=1);

namespace App\HelperNonServices;

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface;
use Symfony\Component\HttpKernel\DependencyInjection\Extension;

class CustomSentryExtension extends Extension implements PrependExtensionInterface
{
    public function load(array $configs, ContainerBuilder $container): void
    {
    }

    public function prepend(ContainerBuilder $container): void
    {
        // Get config from "sentry.yaml".
        $configs = $container->getExtensionConfig('sentry');
        if (empty($configs)) {
            return;
        }

        // Modify the config using ".env.local".
        $modifiedConfig = $configs[0];
        $modifiedConfig['tracing']['enabled'] = ($_ENV['SENTRY_TRACING'] ?? '') === 'true';
        $modifiedConfig['options']['traces_sample_rate'] = (float)($_ENV['SENTRY_PHP_TRACES_SAMPLE_RATE'] ?? 0.0);
        $modifiedConfig['options']['profiles_sample_rate'] = (float)($_ENV['SENTRY_PHP_PROFILING_SAMPLE_RATE'] ?? 0.0);

        // Save changes.
        $reflection = new \ReflectionClass($container);
        $property = $reflection->getProperty('extensionConfigs');
        $extensionConfigs = $property->getValue($container);
        $extensionConfigs['sentry'] = [$modifiedConfig];
        $property->setValue($container, $extensionConfigs);
    }
}

liviucmg avatar Aug 12 '25 15:08 liviucmg

Thank you for the detailed reply and describing your use case @liviucmg!

With the current architecture, I'm reluctant to implement this because of the caching implications. However, I see that it's unexpected to find out that it doesn't work for sentry as it does for different libraries.

I can reopen this issue, although I would change this to be an improvement since this is not necessarily a bug in Sentry but a limitation of symfony how environment variables are resolved at the compilation step.

I will have to give this more thought if it makes sense to remove the definition based on env variable values or if we can change the services to be aware of the value at runtime and change behaviour based on the value.

It should be at least documented. I expected it to work with environment variables, and it took a while to figure out what was going on.

I agree, I will make sure to add it to the documentation

Litarnus avatar Aug 13 '25 11:08 Litarnus