Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
liviucmg opened this issue Sep 25, 2024 · 3 comments
Open

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

liviucmg opened this issue Sep 25, 2024 · 3 comments

Comments

@liviucmg
Copy link

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 liviucmg changed the title Using environment variables %env( in sentry.yaml does not work Using environment variables %env(...) in sentry.yaml does not work Sep 25, 2024
@stayallive
Copy link
Collaborator

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
Copy link
Collaborator

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

@Jean85
Copy link
Collaborator

Jean85 commented Oct 8, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

4 participants