Using environment variables `%env(...)` in `sentry.yaml` does not work
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:
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...
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.
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.
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
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);
}
}
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