General improvements

composer.json

@@ -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",
@@ -46,18 +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/twig-bundle": "^3.4.44||^4.4.12||^5.0.11",
-        "symfony/yaml": "^3.4.44||^4.4.11||^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.",
-        "symfony/twig-bundle": "Allow distributed tracing of twig template rendering using Sentry."
+        "symfony/twig-bundle": "Allow distributed tracing of Twig template rendering using Sentry."
     },
     "autoload": {
         "files": [

psalm.xml

@@ -13,6 +13,7 @@
             <directory name="vendor" />
         </ignoreFiles>
     </projectFiles>
+
     <stubs>
         <file name="tests/Stubs/Profile.phpstub"/>
     </stubs>

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

@@ -95,6 +95,7 @@
 
         <xsd:attribute name="enabled" type="xsd:boolean" />
     </xsd:complexType>
+
     <xsd:complexType name="tracing-twig">
         <xsd:attribute name="enabled" type="xsd:boolean" />
     </xsd:complexType>

src/Resources/config/services.xml

@@ -75,6 +75,7 @@
 
         <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>
 

src/Tracing/Twig/TwigTracingExtension.php

@@ -5,6 +5,7 @@
 namespace Sentry\SentryBundle\Tracing\Twig;
 
 use Sentry\State\HubInterface;
+use Sentry\Tracing\Span;
 use Sentry\Tracing\SpanContext;
 use SplObjectStorage;
 use Twig\Extension\AbstractExtension;
@@ -19,46 +20,83 @@ final class TwigTracingExtension extends AbstractExtension
     private $hub;
 
     /**
-     * @var SplObjectStorage<object, \Sentry\Tracing\Span>
+     * @var SplObjectStorage<object, Span> The currently active spans
      */
-    private $events;
+    private $spans;
 
     /**
      * @param HubInterface $hub The current hub
      */
     public function __construct(HubInterface $hub)
     {
         $this->hub = $hub;
-        $this->events = new SplObjectStorage();
+        $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 || !$profile->isTemplate()) {
+        if (null === $transaction) {
             return;
         }
 
         $spanContext = new SpanContext();
-        $spanContext->setOp('twig.template');
-        $spanContext->setDescription($profile->getName());
+        $spanContext->setOp('view.render');
+        $spanContext->setDescription($this->getSpanDescription($profile));
 
-        $this->events[$profile] = $transaction->startChild($spanContext);
+        $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 (empty($this->events[$profile]) || !$profile->isTemplate()) {
+        if (!isset($this->spans[$profile])) {
             return;
         }
 
-        $this->events[$profile]->finish();
-        unset($this->events[$profile]);
+        $this->spans[$profile]->finish();
+
+        unset($this->spans[$profile]);
     }
 
+    /**
+     * {@inheritdoc}
+     */
     public function getNodeVisitors(): array
     {
-        return [new ProfilerNodeVisitor(static::class)];
+        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());
+        }
     }
 }

tests/Stubs/Profile.phpstub

@@ -6,4 +6,5 @@ namespace Twig\Profiler;
  * @template-implements \IteratorAggregate<Profile, Profile>
  */
 final class Profile implements \IteratorAggregate, \Serializable
-{}
+{
+}

tests/Tracing/Twig/TwigTracingExtensionTest.php

@@ -30,66 +30,64 @@ protected function setUp(): void
         $this->listener = new TwigTracingExtension($this->hub);
     }
 
-    public function testThatTwigEnterProfileIgnoresTracingWhenTransactionIsNotStarted(): void
+    /**
+     * @dataProvider enterDataProvider
+     */
+    public function testEnter(Profile $profile, string $spanDescription): void
     {
-        $this->hub->expects($this->once())
-            ->method('getTransaction')
-            ->willReturn(null);
-
-        $this->listener->enter(new Profile('main', Profile::TEMPLATE));
-    }
+        $transaction = new Transaction(new TransactionContext());
+        $transaction->initSpanRecorder();
 
-    public function testThatTwigEnterProfileIgnoresTracingWhenNotATemplate(): void
-    {
         $this->hub->expects($this->once())
             ->method('getTransaction')
-            ->willReturn(new Transaction(new TransactionContext()));
-
-        $this->listener->enter(new Profile('main', Profile::ROOT));
-    }
+            ->willReturn($transaction);
 
-    public function testThatTwigLeaveProfileIgnoresTracingWhenTransactionIsNotStarted(): void
-    {
-        $this->hub->expects($this->once())
-            ->method('getTransaction')
-            ->willReturn(null);
+        $this->listener->enter($profile);
 
-        $profile = new Profile('main', Profile::TEMPLATE);
+        $spans = $transaction->getSpanRecorder()->getSpans();
 
-        $this->listener->enter($profile);
-        $this->listener->leave($profile);
+        $this->assertCount(2, $spans);
+        $this->assertNull($spans[1]->getEndTimestamp());
+        $this->assertSame('view.render', $spans[1]->getOp());
+        $this->assertSame($spanDescription, $spans[1]->getDescription());
     }
 
-    public function testThatTwigLeaveProfileIgnoresTracingWhenNotATemplate(): void
+    /**
+     * @return \Generator<mixed>
+     */
+    public function enterDataProvider(): \Generator
     {
-        $this->hub->expects($this->once())
-            ->method('getTransaction')
-            ->willReturn(new Transaction(new TransactionContext()));
-
-        $profile = new Profile('main', Profile::ROOT);
-
-        $this->listener->enter($profile);
-        $this->listener->leave($profile);
+        yield [
+            new Profile('main.twig', Profile::ROOT),
+            'main',
+        ];
+
+        yield [
+            new Profile('main.twig', Profile::TEMPLATE),
+            'main.twig',
+        ];
+
+        yield [
+            new Profile('main.twig', Profile::BLOCK, 'content'),
+            'main.twig::block(content)',
+        ];
+
+        yield [
+            new Profile('main.twig', Profile::MACRO, 'input'),
+            'main.twig::macro(input)',
+        ];
     }
 
-    public function testThatTwigEnterProfileAttachesAChildSpanWhenTransactionStarted(): void
+    public function testEnterDoesNothingIfNoSpanIsSetOnHub(): void
     {
-        $transaction = new Transaction(new TransactionContext());
-        $transaction->initSpanRecorder();
-
         $this->hub->expects($this->once())
             ->method('getTransaction')
-            ->willReturn($transaction);
+            ->willReturn(null);
 
         $this->listener->enter(new Profile('main', Profile::TEMPLATE));
-
-        $spans = $transaction->getSpanRecorder()->getSpans();
-
-        $this->assertCount(2, $spans);
-        $this->assertNull($spans[1]->getEndTimestamp());
     }
 
-    public function testThatTwigLeaveProfileFinishesTheChildSpanWhenChildSpanStarted(): void
+    public function testLeave(): void
     {
         $transaction = new Transaction(new TransactionContext());
         $transaction->initSpanRecorder();
@@ -108,4 +106,11 @@ public function testThatTwigLeaveProfileFinishesTheChildSpanWhenChildSpanStarted
         $this->assertCount(2, $spans);
         $this->assertNotNull($spans[1]->getEndTimestamp());
     }
+
+    public function testLeaveDoesNothingIfSpanDoesNotExistsForProfile(): void
+    {
+        $this->expectNotToPerformAssertions();
+
+        $this->listener->leave(new Profile('main', Profile::TEMPLATE));
+    }
 }