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

Implementation of optional sentry tracing with twig #430

Merged
merged 5 commits into from
Feb 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ jobs:
restore-keys: ${{ runner.os }}-${{ matrix.php }}-composer-${{ matrix.dependencies }}-

- name: Remove optional packages
run: composer remove doctrine/dbal doctrine/doctrine-bundle symfony/messenger --dev --no-update
run: composer remove doctrine/dbal doctrine/doctrine-bundle symfony/messenger symfony/twig-bundle --dev --no-update

- name: Install highest dependencies
run: composer update --no-progress --no-interaction --prefer-dist
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Unreleased

- Add support for distributed tracing of Twig template rendering (#430)
- Add support for distributed tracing of SQL queries while using Doctrine DBAL (#426)
- Added missing `capture-soft-fails` config schema option (#417)
- Deprecate the `Sentry\SentryBundle\EventListener\ConsoleCommandListener` class in favor of its parent class `Sentry\SentryBundle\EventListener\ConsoleListener` (#429)
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ psalm:
test:
vendor/bin/phpunit

pre-commit-check: cs phpstan test
pre-commit-check: cs phpstan psalm test

setup-git:
git config branch.autosetuprebase always
20 changes: 11 additions & 9 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@
"jean85/pretty-package-versions": "^1.5 || ^2.0",
"php-http/discovery": "^1.11",
"sentry/sdk": "^3.1",
"symfony/config": "^3.4.44||^4.4.11||^5.0.11",
"symfony/console": "^3.4.44||^4.4.11||^5.0.11",
"symfony/dependency-injection": "^3.4.44||^4.4.11||^5.0.11",
"symfony/event-dispatcher": "^3.4.44||^4.4.11||^5.0.11",
"symfony/http-kernel": "^3.4.44||^4.4.11||^5.0.11",
"symfony/config": "^3.4.44||^4.4.12||^5.0.11",
"symfony/console": "^3.4.44||^4.4.12||^5.0.11",
"symfony/dependency-injection": "^3.4.44||^4.4.12||^5.0.11",
"symfony/event-dispatcher": "^3.4.44||^4.4.12||^5.0.11",
"symfony/http-kernel": "^3.4.44||^4.4.12||^5.0.11",
"symfony/psr-http-message-bridge": "^2.0",
"symfony/security-core": "^3.4.44||^4.4.11||^5.0.11"
"symfony/security-core": "^3.4.44||^4.4.12||^5.0.11"
},
"require-dev": {
"doctrine/dbal": "^2.10||^3.0",
Expand All @@ -46,16 +46,18 @@
"symfony/browser-kit": "^3.4.44||^4.4.12||^5.0.11",
"symfony/dom-crawler": "^3.4.44||^4.4.12||^5.0.11",
"symfony/framework-bundle": "^3.4.44||^4.4.12||^5.0.11",
"symfony/messenger": "^4.4.11||^5.0.11",
"symfony/messenger": "^4.4.12||^5.0.11",
"symfony/monolog-bundle": "^3.4",
"symfony/phpunit-bridge": "^5.0",
"symfony/polyfill-php80": "^1.22",
"symfony/yaml": "^3.4.44||^4.4.11||^5.0.11",
"symfony/twig-bundle": "^3.4.44||^4.4.12||^5.0.11",
"symfony/yaml": "^3.4.44||^4.4.12||^5.0.11",
"vimeo/psalm": "^4.3"
},
"suggest": {
"monolog/monolog": "Allow sending log messages to Sentry by using the included Monolog handler.",
"doctrine/doctrine-bundle": "Allow distributed tracing of database queries using Sentry."
"doctrine/doctrine-bundle": "Allow distributed tracing of database queries using Sentry.",
"symfony/twig-bundle": "Allow distributed tracing of Twig template rendering using Sentry."
},
"autoload": {
"files": [
Expand Down
1 change: 0 additions & 1 deletion phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -219,4 +219,3 @@ parameters:
message: "#^Trying to mock an undefined method convertException\\(\\) on class Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Doctrine\\\\DBAL\\\\StubExceptionConverterDriverInterface\\.$#"
count: 1
path: tests/Tracing/Doctrine/DBAL/TracingDriverTest.php

2 changes: 2 additions & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ parameters:
dynamicConstantNames:
- Symfony\Component\HttpKernel\Kernel::VERSION
- Doctrine\DBAL\Version::VERSION
stubFiles:
- tests/Stubs/Profile.phpstub
4 changes: 4 additions & 0 deletions psalm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,8 @@
<directory name="vendor" />
</ignoreFiles>
</projectFiles>

<stubs>
<file name="tests/Stubs/Profile.phpstub"/>
</stubs>
</psalm>
3 changes: 3 additions & 0 deletions src/DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,9 @@ private function addDistributedTracingSection(ArrayNodeDefinition $rootNode): vo
->end()
->end()
->end()
->arrayNode('twig')
->canBeEnabled()
rjd22 marked this conversation as resolved.
Show resolved Hide resolved
->end()
rjd22 marked this conversation as resolved.
Show resolved Hide resolved
->end()
->end()
->end();
Expand Down
14 changes: 14 additions & 0 deletions src/DependencyInjection/SentryExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use Sentry\SentryBundle\SentryBundle;
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\ConnectionConfigurator;
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware;
use Sentry\SentryBundle\Tracing\Twig\TwigTracingExtension;
use Sentry\Serializer\RepresentationSerializer;
use Sentry\Serializer\Serializer;
use Sentry\Transport\TransportFactoryInterface;
Expand Down Expand Up @@ -62,6 +63,7 @@ protected function loadInternal(array $mergedConfig, ContainerBuilder $container
$this->registerErrorListenerConfiguration($container, $mergedConfig);
$this->registerMessengerListenerConfiguration($container, $mergedConfig['messenger']);
$this->registerTracingConfiguration($container, $mergedConfig['tracing']);
$this->registerTracingTwigExtensionConfiguration($container, $mergedConfig['tracing']);
}

/**
Expand Down Expand Up @@ -173,6 +175,18 @@ private function registerTracingConfiguration(ContainerBuilder $container, array
}
}

/**
* @param array<string, mixed> $config
*/
private function registerTracingTwigExtensionConfiguration(ContainerBuilder $container, array $config): void
{
$isConfigEnabled = $this->isConfigEnabled($container, $config['twig']);

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

/**
* @param string[] $integrations
* @param array<string, mixed> $config
Expand Down
5 changes: 5 additions & 0 deletions src/Resources/config/schema/sentry-1.0.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
<xsd:complexType name="tracing">
<xsd:choice maxOccurs="unbounded">
<xsd:element name="dbal" type="tracing-dbal" minOccurs="0" maxOccurs="1" />
<xsd:element name="twig" type="tracing-twig" minOccurs="0" maxOccurs="1" />
</xsd:choice>
</xsd:complexType>

Expand All @@ -94,4 +95,8 @@

<xsd:attribute name="enabled" type="xsd:boolean" />
</xsd:complexType>

<xsd:complexType name="tracing-twig">
<xsd:attribute name="enabled" type="xsd:boolean" />
</xsd:complexType>
</xsd:schema>
6 changes: 6 additions & 0 deletions src/Resources/config/services.xml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@
<argument type="service" id="Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware" />
</service>

<service id="Sentry\SentryBundle\Tracing\Twig\TwigTracingExtension" class="Sentry\SentryBundle\Tracing\Twig\TwigTracingExtension">
<argument type="service" id="Sentry\State\HubInterface" />

<tag name="twig.extension" />
</service>

<service id="Sentry\Integration\RequestFetcherInterface" class="Sentry\SentryBundle\Integration\RequestFetcher">
<argument type="service" id="Symfony\Component\HttpFoundation\RequestStack" />
<argument type="service" id="Symfony\Bridge\PsrHttpMessage\HttpMessageFactoryInterface" on-invalid="null" />
Expand Down
102 changes: 102 additions & 0 deletions src/Tracing/Twig/TwigTracingExtension.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
<?php

declare(strict_types=1);

namespace Sentry\SentryBundle\Tracing\Twig;

use Sentry\State\HubInterface;
use Sentry\Tracing\Span;
use Sentry\Tracing\SpanContext;
use SplObjectStorage;
use Twig\Extension\AbstractExtension;
use Twig\Profiler\NodeVisitor\ProfilerNodeVisitor;
use Twig\Profiler\Profile;

final class TwigTracingExtension extends AbstractExtension
{
/**
* @var HubInterface The current hub
*/
private $hub;

/**
* @var SplObjectStorage<object, Span> The currently active spans
*/
private $spans;

/**
* @param HubInterface $hub The current hub
*/
public function __construct(HubInterface $hub)
{
$this->hub = $hub;
$this->spans = new SplObjectStorage();
}

/**
* This method is called before the execution of a block, a macro or a
* template.
*
* @param Profile $profile The profiling data
*/
public function enter(Profile $profile): void
{
$transaction = $this->hub->getTransaction();

if (null === $transaction) {
return;
}

$spanContext = new SpanContext();
$spanContext->setOp('view.render');
$spanContext->setDescription($this->getSpanDescription($profile));

$this->spans[$profile] = $transaction->startChild($spanContext);
}

/**
* This method is called when the execution of a block, a macro or a
* template is finished.
*
* @param Profile $profile The profiling data
*/
public function leave(Profile $profile): void
{
if (!isset($this->spans[$profile])) {
return;
}

$this->spans[$profile]->finish();

unset($this->spans[$profile]);
}

/**
* {@inheritdoc}
*/
public function getNodeVisitors(): array
{
return [
new ProfilerNodeVisitor(self::class),
];
}

/**
* Gets a short description for the span.
*
* @param Profile $profile The profiling data
*/
private function getSpanDescription(Profile $profile): string
{
switch (true) {
case $profile->isRoot():
return $profile->getName();

case $profile->isTemplate():
return $profile->getTemplate();

default:
return sprintf('%s::%s(%s)', $profile->getTemplate(), $profile->getType(), $profile->getName());
}
}
}
3 changes: 3 additions & 0 deletions tests/DependencyInjection/ConfigurationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ public function testProcessConfigurationWithDefaultConfiguration(): void
'enabled' => false,
'connections' => class_exists(DoctrineBundle::class) ? ['%doctrine.default_connection%'] : [],
],
'twig' => [
'enabled' => false,
],
],
];

Expand Down
3 changes: 3 additions & 0 deletions tests/DependencyInjection/Fixtures/php/full.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,8 @@
'enabled' => false,
'connections' => ['default'],
],
'twig' => [
'enabled' => false,
],
],
]);
14 changes: 14 additions & 0 deletions tests/DependencyInjection/Fixtures/php/twig_tracing_enabled.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

declare(strict_types=1);

use Symfony\Component\DependencyInjection\ContainerBuilder;

/** @var ContainerBuilder $container */
$container->loadFromExtension('sentry', [
'tracing' => [
'twig' => [
'enabled' => true,
],
],
]);
1 change: 1 addition & 0 deletions tests/DependencyInjection/Fixtures/xml/full.xml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
<sentry:dbal enabled="false">
<sentry:connection>default</sentry:connection>
</sentry:dbal>
<sentry:twig enabled="false" />
</sentry:tracing>
</sentry:config>
</container>
14 changes: 14 additions & 0 deletions tests/DependencyInjection/Fixtures/xml/twig_tracing_enabled.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?xml version="1.0" ?>

<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:sentry="https://sentry.io/schema/dic/sentry-symfony"
xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd
https://sentry.io/schema/dic/sentry-symfony https://sentry.io/schema/dic/sentry-symfony/sentry-1.0.xsd">

<sentry:config>
<sentry:tracing>
<sentry:twig enabled="true" />
</sentry:tracing>
</sentry:config>
</container>
2 changes: 2 additions & 0 deletions tests/DependencyInjection/Fixtures/yml/full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,5 @@ sentry:
enabled: false
connections:
- enabled
twig:
enabled: false
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
sentry:
tracing:
twig:
enabled: true
15 changes: 15 additions & 0 deletions tests/DependencyInjection/SentryExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use Sentry\SentryBundle\SentryBundle;
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\ConnectionConfigurator;
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware;
use Sentry\SentryBundle\Tracing\Twig\TwigTracingExtension;
use Sentry\Serializer\RepresentationSerializer;
use Sentry\Serializer\Serializer;
use Sentry\Transport\TransportFactoryInterface;
Expand Down Expand Up @@ -286,6 +287,20 @@ public function testTracingDriverMiddlewareIsRemovedWhenDbalTracingIsDisabled():
$this->assertEmpty($container->getParameter('sentry.tracing.dbal.connections'));
}

public function testTwigTracingExtensionIsConfiguredWhenTwigTracingIsEnabled(): void
{
$container = $this->createContainerFromFixture('twig_tracing_enabled');

$this->assertTrue($container->hasDefinition(TwigTracingExtension::class));
}

public function testTwigTracingExtensionIsRemovedWhenTwigTracingIsDisabled(): void
{
$container = $this->createContainerFromFixture('full');

$this->assertFalse($container->hasDefinition(TwigTracingExtension::class));
}

private function createContainerFromFixture(string $fixtureFile): ContainerBuilder
{
$container = new ContainerBuilder(new EnvPlaceholderParameterBag([
Expand Down
10 changes: 10 additions & 0 deletions tests/Stubs/Profile.phpstub
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

namespace Twig\Profiler;

/**
* @template-implements \IteratorAggregate<Profile, Profile>
*/
final class Profile implements \IteratorAggregate, \Serializable
{
}