Skip to content

Commit 8e2fbda

Browse files
Jean85ste93cry
Jean85
and
ste93cry
authored
Add global switch to allow disabling all instrumentations for the tracing feature (#467)
Co-authored-by: Stefano Arlandini <[email protected]>
1 parent 531b218 commit 8e2fbda

File tree

12 files changed

+158
-38
lines changed

12 files changed

+158
-38
lines changed

‎phpstan-baseline.neon

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,6 @@ parameters:
1010
count: 1
1111
path: src/DependencyInjection/Configuration.php
1212

13-
-
14-
message: "#^Call to an undefined method Symfony\\\\Component\\\\Config\\\\Definition\\\\Builder\\\\NodeParentInterface\\:\\:end\\(\\)\\.$#"
15-
count: 1
16-
path: src/DependencyInjection/Configuration.php
17-
1813
-
1914
message: "#^Else branch is unreachable because previous condition is always true\\.$#"
2015
count: 1

‎src/DependencyInjection/Compiler/DbalTracingPass.php

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,27 @@ final class DbalTracingPass implements CompilerPassInterface
3131
*/
3232
public function process(ContainerBuilder $container): void
3333
{
34-
if (!$container->hasParameter('doctrine.connections')) {
34+
if (
35+
!$container->hasParameter('doctrine.connections')
36+
|| !$container->getParameter('sentry.tracing.enabled')
37+
|| !$container->getParameter('sentry.tracing.dbal.enabled')
38+
) {
3539
return;
3640
}
3741

38-
/** @var string[] $connections */
39-
$connections = $container->getParameter('doctrine.connections');
40-
4142
/** @var string[] $connectionsToTrace */
4243
$connectionsToTrace = $container->getParameter('sentry.tracing.dbal.connections');
4344

45+
/** @var array<string, string> $connections */
46+
$connections = $container->getParameter('doctrine.connections');
47+
48+
if (empty($connectionsToTrace)) {
49+
$connectionsToTrace = array_keys($connections);
50+
}
51+
4452
foreach ($connectionsToTrace as $connectionName) {
4553
if (!\in_array(sprintf(self::CONNECTION_SERVICE_NAME_FORMAT, $connectionName), $connections, true)) {
46-
continue;
54+
throw new \InvalidArgumentException(sprintf('The Doctrine connection "%s" does not exists and cannot be instrumented.', $connectionName));
4755
}
4856

4957
if (!interface_exists(ResultStatement::class)) {

‎src/DependencyInjection/Configuration.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Jean85\PrettyVersions;
99
use Sentry\Options;
1010
use Sentry\SentryBundle\ErrorTypesParser;
11+
use Symfony\Bundle\TwigBundle\TwigBundle;
1112
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
1213
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
1314
use Symfony\Component\Config\Definition\ConfigurationInterface;
@@ -147,20 +148,20 @@ private function addDistributedTracingSection(ArrayNodeDefinition $rootNode): vo
147148
$rootNode
148149
->children()
149150
->arrayNode('tracing')
151+
->canBeDisabled()
150152
->addDefaultsIfNotSet()
151153
->children()
152154
->arrayNode('dbal')
153-
->canBeEnabled()
155+
->{class_exists(DoctrineBundle::class) ? 'canBeDisabled' : 'canBeEnabled'}()
154156
->fixXmlConfig('connection')
155157
->children()
156158
->arrayNode('connections')
157-
->defaultValue(class_exists(DoctrineBundle::class) ? ['%doctrine.default_connection%'] : [])
158159
->scalarPrototype()->end()
159160
->end()
160161
->end()
161162
->end()
162163
->arrayNode('twig')
163-
->canBeEnabled()
164+
->{class_exists(TwigBundle::class) ? 'canBeDisabled' : 'canBeEnabled'}()
164165
->end()
165166
->end()
166167
->end()

‎src/DependencyInjection/SentryExtension.php

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,17 @@
1717
use Sentry\SentryBundle\EventListener\ConsoleListener;
1818
use Sentry\SentryBundle\EventListener\ErrorListener;
1919
use Sentry\SentryBundle\EventListener\MessengerListener;
20+
use Sentry\SentryBundle\EventListener\TracingConsoleListener;
21+
use Sentry\SentryBundle\EventListener\TracingRequestListener;
22+
use Sentry\SentryBundle\EventListener\TracingSubRequestListener;
2023
use Sentry\SentryBundle\SentryBundle;
2124
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\ConnectionConfigurator;
2225
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware;
2326
use Sentry\SentryBundle\Tracing\Twig\TwigTracingExtension;
2427
use Sentry\Serializer\RepresentationSerializer;
2528
use Sentry\Serializer\Serializer;
2629
use Sentry\Transport\TransportFactoryInterface;
30+
use Symfony\Bundle\TwigBundle\TwigBundle;
2731
use Symfony\Component\Config\FileLocator;
2832
use Symfony\Component\DependencyInjection\ContainerBuilder;
2933
use Symfony\Component\DependencyInjection\Definition;
@@ -51,9 +55,7 @@ public function getNamespace()
5155
}
5256

5357
/**
54-
* @param array<string, mixed> $mergedConfig
55-
*
56-
* @psalm-suppress MoreSpecificImplementedParamType
58+
* @param mixed[] $mergedConfig
5759
*/
5860
protected function loadInternal(array $mergedConfig, ContainerBuilder $container): void
5961
{
@@ -63,6 +65,7 @@ protected function loadInternal(array $mergedConfig, ContainerBuilder $container
6365
$this->registerConfiguration($container, $mergedConfig);
6466
$this->registerErrorListenerConfiguration($container, $mergedConfig);
6567
$this->registerMessengerListenerConfiguration($container, $mergedConfig['messenger']);
68+
$this->registerTracingConfiguration($container, $mergedConfig['tracing']);
6669
$this->registerDbalTracingConfiguration($container, $mergedConfig['tracing']);
6770
$this->registerTwigTracingConfiguration($container, $mergedConfig['tracing']);
6871
}
@@ -159,17 +162,31 @@ private function registerMessengerListenerConfiguration(ContainerBuilder $contai
159162
$container->getDefinition(MessengerListener::class)->setArgument(1, $config['capture_soft_fails']);
160163
}
161164

165+
/**
166+
* @param array<string, mixed> $config
167+
*/
168+
private function registerTracingConfiguration(ContainerBuilder $container, $config): void
169+
{
170+
if (!$this->isConfigEnabled($container, $config)) {
171+
$container->removeDefinition(TracingRequestListener::class);
172+
$container->removeDefinition(TracingSubRequestListener::class);
173+
$container->removeDefinition(TracingConsoleListener::class);
174+
}
175+
}
176+
162177
/**
163178
* @param array<string, mixed> $config
164179
*/
165180
private function registerDbalTracingConfiguration(ContainerBuilder $container, array $config): void
166181
{
167-
$isConfigEnabled = $this->isConfigEnabled($container, $config['dbal']);
182+
$isConfigEnabled = $this->isConfigEnabled($container, $config)
183+
&& $this->isConfigEnabled($container, $config['dbal']);
168184

169185
if ($isConfigEnabled && !class_exists(DoctrineBundle::class)) {
170-
throw new LogicException('DBAL tracing support cannot be enabled as the DoctrineBundle bundle is not installed.');
186+
throw new LogicException('DBAL tracing support cannot be enabled because the doctrine/doctrine-bundle Composer package is not installed.');
171187
}
172188

189+
$container->setParameter('sentry.tracing.dbal.enabled', $isConfigEnabled);
173190
$container->setParameter('sentry.tracing.dbal.connections', $isConfigEnabled ? $config['dbal']['connections'] : []);
174191

175192
if (!$isConfigEnabled) {
@@ -183,7 +200,12 @@ private function registerDbalTracingConfiguration(ContainerBuilder $container, a
183200
*/
184201
private function registerTwigTracingConfiguration(ContainerBuilder $container, array $config): void
185202
{
186-
$isConfigEnabled = $this->isConfigEnabled($container, $config['twig']);
203+
$isConfigEnabled = $this->isConfigEnabled($container, $config)
204+
&& $this->isConfigEnabled($container, $config['twig']);
205+
206+
if ($isConfigEnabled && !class_exists(TwigBundle::class)) {
207+
throw new LogicException('Twig tracing support cannot be enabled because the symfony/twig-bundle Composer package is not installed.');
208+
}
187209

188210
if (!$isConfigEnabled) {
189211
$container->removeDefinition(TwigTracingExtension::class);

‎src/Resources/config/schema/sentry-1.0.xsd

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@
8686
<xsd:element name="dbal" type="tracing-dbal" minOccurs="0" maxOccurs="1" />
8787
<xsd:element name="twig" type="tracing-twig" minOccurs="0" maxOccurs="1" />
8888
</xsd:choice>
89+
90+
<xsd:attribute name="enabled" type="xsd:boolean" default="true"/>
8991
</xsd:complexType>
9092

9193
<xsd:complexType name="tracing-dbal">

‎tests/DependencyInjection/Compiler/DbalTracingPassTest.php

Lines changed: 53 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@ public function testProcessWithDoctrineDBALVersionAtLeast30(): void
2323
}
2424

2525
$container = $this->createContainerBuilder();
26-
$container->setParameter('doctrine.connections', ['doctrine.dbal.foo_connection', 'doctrine.dbal.bar_connection', 'doctrine.dbal.baz_connection']);
27-
$container->setParameter('sentry.tracing.dbal.connections', ['foo', 'bar', 'baz', 'qux']);
26+
$container->setParameter('sentry.tracing.dbal.connections', ['foo', 'bar', 'baz']);
2827

2928
$container
3029
->register('foo.service', \stdClass::class)
@@ -90,30 +89,65 @@ public function testProcessWithDoctrineDBALVersionLowerThan30(): void
9089

9190
$connection1 = (new Definition(Connection::class))->setPublic(true);
9291
$connection2 = (new Definition(Connection::class))->setPublic(true);
92+
$connection3 = (new Definition(Connection::class))->setPublic(true);
9393

9494
$container = $this->createContainerBuilder();
95-
$container->setParameter('doctrine.connections', ['doctrine.dbal.foo_connection', 'doctrine.dbal.bar_connection']);
9695
$container->setParameter('sentry.tracing.dbal.connections', ['foo', 'baz']);
9796
$container->setDefinition('doctrine.dbal.foo_connection', $connection1);
9897
$container->setDefinition('doctrine.dbal.bar_connection', $connection2);
98+
$container->setDefinition('doctrine.dbal.baz_connection', $connection3);
9999
$container->compile();
100100

101101
$this->assertEquals([new Reference(ConnectionConfigurator::class), 'configure'], $connection1->getConfigurator());
102+
$this->assertEquals([new Reference(ConnectionConfigurator::class), 'configure'], $connection3->getConfigurator());
102103
$this->assertNull($connection2->getConfigurator());
103104
}
104105

105-
public function testProcessDoesNothingIfDoctrineConnectionsParamIsMissing(): void
106+
/**
107+
* @dataProvider processDoesNothingIfConditionsForEnablingTracingAreMissingDataProvider
108+
*/
109+
public function testProcessDoesNothingIfConditionsForEnablingTracingAreMissing(ContainerBuilder $container): void
110+
{
111+
$connectionConfigDefinition = new Definition();
112+
$connectionConfigDefinition->setClass(Configuration::class);
113+
$connectionConfigDefinition->setPublic(true);
114+
115+
$container->setDefinition('doctrine.dbal.foo_connection.configuration', $connectionConfigDefinition);
116+
$container->compile();
117+
118+
$this->assertEmpty($connectionConfigDefinition->getMethodCalls());
119+
}
120+
121+
/**
122+
* @return \Generator<array<mixed>>
123+
*/
124+
public function processDoesNothingIfConditionsForEnablingTracingAreMissingDataProvider(): \Generator
106125
{
107126
$container = $this->createContainerBuilder();
108-
$container->setParameter('sentry.tracing.dbal.connections', ['foo']);
127+
$container->setParameter('sentry.tracing.enabled', false);
109128

110-
$container
111-
->register('doctrine.dbal.foo_connection.configuration', Configuration::class)
112-
->setPublic(true);
129+
yield [$container];
113130

114-
$container->compile();
131+
$container = $this->createContainerBuilder();
132+
$container->setParameter('doctrine.connections', []);
115133

116-
$this->assertEmpty($container->getDefinition('doctrine.dbal.foo_connection.configuration')->getMethodCalls());
134+
yield [$container];
135+
136+
$container = $this->createContainerBuilder();
137+
$container->setParameter('sentry.tracing.dbal.enabled', false);
138+
139+
yield [$container];
140+
}
141+
142+
public function testContainerCompilationFailsIfConnectionDoesntExist(): void
143+
{
144+
$container = $this->createContainerBuilder();
145+
$container->setParameter('sentry.tracing.dbal.connections', ['missing']);
146+
147+
$this->expectException(\InvalidArgumentException::class);
148+
$this->expectExceptionMessage('The Doctrine connection "missing" does not exists and cannot be instrumented.');
149+
150+
$container->compile();
117151
}
118152

119153
private function createContainerBuilder(): ContainerBuilder
@@ -129,6 +163,15 @@ private function createContainerBuilder(): ContainerBuilder
129163
->register(ConnectionConfigurator::class, ConnectionConfigurator::class)
130164
->setPublic(true);
131165

166+
$container->setParameter('sentry.tracing.enabled', true);
167+
$container->setParameter('sentry.tracing.dbal.enabled', true);
168+
$container->setParameter('sentry.tracing.dbal.connections', []);
169+
$container->setParameter('doctrine.connections', [
170+
'doctrine.dbal.foo_connection',
171+
'doctrine.dbal.bar_connection',
172+
'doctrine.dbal.baz_connection',
173+
]);
174+
132175
return $container;
133176
}
134177
}

‎tests/DependencyInjection/ConfigurationTest.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Jean85\PrettyVersions;
99
use PHPUnit\Framework\TestCase;
1010
use Sentry\SentryBundle\DependencyInjection\Configuration;
11+
use Symfony\Bundle\TwigBundle\TwigBundle;
1112
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
1213
use Symfony\Component\Config\Definition\Processor;
1314
use Symfony\Component\Messenger\MessageBusInterface;
@@ -16,13 +17,11 @@ final class ConfigurationTest extends TestCase
1617
{
1718
public function testProcessConfigurationWithDefaultConfiguration(): void
1819
{
19-
$defaultPrefixes = array_filter(explode(\PATH_SEPARATOR, get_include_path() ?: ''));
20-
2120
$expectedBundleDefaultConfig = [
2221
'register_error_listener' => true,
2322
'options' => [
2423
'integrations' => [],
25-
'prefixes' => array_merge(['%kernel.project_dir%'], $defaultPrefixes),
24+
'prefixes' => array_merge(['%kernel.project_dir%'], array_filter(explode(\PATH_SEPARATOR, get_include_path() ?: ''))),
2625
'environment' => '%kernel.environment%',
2726
'release' => PrettyVersions::getRootPackageVersion()->getPrettyVersion(),
2827
'tags' => [],
@@ -39,12 +38,13 @@ public function testProcessConfigurationWithDefaultConfiguration(): void
3938
'capture_soft_fails' => true,
4039
],
4140
'tracing' => [
41+
'enabled' => true,
4242
'dbal' => [
43-
'enabled' => false,
44-
'connections' => class_exists(DoctrineBundle::class) ? ['%doctrine.default_connection%'] : [],
43+
'enabled' => class_exists(DoctrineBundle::class),
44+
'connections' => [],
4545
],
4646
'twig' => [
47-
'enabled' => false,
47+
'enabled' => class_exists(TwigBundle::class),
4848
],
4949
],
5050
];

‎tests/DependencyInjection/Fixtures/php/tracing_disabled.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use Symfony\Component\DependencyInjection\ContainerBuilder;
6+
7+
/** @var ContainerBuilder $container */
8+
$container->loadFromExtension('sentry', [
9+
'tracing' => [
10+
'enabled' => false,
11+
],
12+
]);

‎tests/DependencyInjection/Fixtures/xml/tracing_disabled.xml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?xml version="1.0" ?>
2+
3+
<container xmlns="http://symfony.com/schema/dic/services"
4+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
5+
xmlns:sentry="https://sentry.io/schema/dic/sentry-symfony"
6+
xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd
7+
https://sentry.io/schema/dic/sentry-symfony https://sentry.io/schema/dic/sentry-symfony/sentry-1.0.xsd">
8+
9+
<sentry:config>
10+
<sentry:tracing enabled="false" />
11+
</sentry:config>
12+
</container>

‎tests/DependencyInjection/Fixtures/yml/tracing_disabled.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
sentry:
2+
tracing:
3+
enabled: false

0 commit comments

Comments
 (0)