Skip to content

Commit 3fd0cfe

Browse files
allflameallflame
committed
Fixed broken logic in the RequestSpanListener.php
1 parent 30fdef2 commit 3fd0cfe

12 files changed

+135
-44
lines changed

‎src/Bridge/AppStartSpanListener.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public function __construct(TracerInterface $tracer)
2222

2323
public static function getSubscribedEvents(): array
2424
{
25-
return [RequestEvent::class => ['onRequest', -1023],];
25+
return [RequestEvent::class => ['onRequest', -1],];
2626
}
2727

2828
public function onRequest(RequestEvent $event)

‎src/Bridge/HandlerFlushListener.php‎ renamed to ‎src/Bridge/MainSpanFlushListener.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@
66
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
77
use Symfony\Component\HttpKernel\Event\TerminateEvent;
88

9-
class HandlerFlushListener implements EventSubscriberInterface
9+
class MainSpanFlushListener implements EventSubscriberInterface
1010
{
1111
private $backgroundHandler;
1212

1313
private $globalHandler;
1414

15-
public function __construct(BackgroundSpanHandler $backgroundHandler, GlobalSpanHandler $globalHandler)
15+
public function __construct(BackgroundSpanHandler $backgroundHandler, MainSpanHandler $globalHandler)
1616
{
1717
$this->backgroundHandler = $backgroundHandler;
1818
$this->globalHandler = $globalHandler;

‎src/Bridge/GlobalSpanHandler.php‎ renamed to ‎src/Bridge/MainSpanHandler.php‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
use Jaeger\Tracer\TracerInterface;
1414
use Symfony\Component\HttpFoundation\Request;
1515

16-
class GlobalSpanHandler
16+
class MainSpanHandler
1717
{
1818
/**
1919
* @var Span
@@ -32,10 +32,10 @@ public function __construct(TracerInterface $tracer, NameGeneratorInterface $nam
3232
$this->nameGenerator = $nameGenerator;
3333
}
3434

35-
public function start(Request $request): GlobalSpanHandler
35+
public function start(Request $request): MainSpanHandler
3636
{
3737
$this->span = $this->tracer->start(
38-
$this->nameGenerator->generate(),
38+
'main.start',
3939
[
4040
new HttpMethodTag($request->getMethod()),
4141
new HttpUriTag($request->getRequestUri()),
@@ -48,6 +48,16 @@ public function start(Request $request): GlobalSpanHandler
4848
return $this;
4949
}
5050

51+
public function name(): MainSpanHandler
52+
{
53+
if (null === $this->span) {
54+
return $this;
55+
}
56+
$this->span->operationName = $this->nameGenerator->generate();
57+
58+
return $this;
59+
}
60+
5161
public function finish(): void
5262
{
5363
if (null === $this->span) {

‎src/Bridge/GlobalSpanListener.php‎ renamed to ‎src/Bridge/MainSpanLifecycleListener.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,31 +8,31 @@
88
use Symfony\Component\HttpKernel\Event\RequestEvent;
99
use Symfony\Component\HttpKernel\Event\TerminateEvent;
1010

11-
class GlobalSpanListener implements EventSubscriberInterface
11+
class MainSpanLifecycleListener implements EventSubscriberInterface
1212
{
13-
private GlobalSpanHandler $handler;
13+
private MainSpanHandler $handler;
1414

15-
public function __construct(GlobalSpanHandler $handler)
15+
public function __construct(MainSpanHandler $handler)
1616
{
1717
$this->handler = $handler;
1818
}
1919

2020
public static function getSubscribedEvents(): array
2121
{
2222
return [
23-
RequestEvent::class => ['onRequest', 25],
23+
RequestEvent::class => ['onRequest', 1024],
2424
TerminateEvent::class => ['onTerminate', 4096],
2525
];
2626
}
2727

28-
public function onTerminate(): GlobalSpanListener
28+
public function onTerminate(): MainSpanLifecycleListener
2929
{
3030
$this->handler->finish();
3131

3232
return $this;
3333
}
3434

35-
public function onRequest(RequestEvent $event): GlobalSpanListener
35+
public function onRequest(RequestEvent $event): MainSpanLifecycleListener
3636
{
3737
if (false === $this->isMainRequestEvent($event)) {
3838
return $this;

‎src/Bridge/MainSpanNameListener.php‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace Jaeger\Symfony\Bridge;
5+
6+
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
7+
use Symfony\Component\HttpFoundation\Request;
8+
use Symfony\Component\HttpKernel\Event\KernelEvent;
9+
use Symfony\Component\HttpKernel\Event\RequestEvent;
10+
use Symfony\Component\HttpKernel\Event\TerminateEvent;
11+
12+
class MainSpanNameListener implements EventSubscriberInterface
13+
{
14+
private MainSpanHandler $handler;
15+
16+
public function __construct(MainSpanHandler $handler)
17+
{
18+
$this->handler = $handler;
19+
}
20+
21+
public static function getSubscribedEvents(): array
22+
{
23+
return [
24+
RequestEvent::class => ['onRequest', 30],
25+
];
26+
}
27+
28+
public function onRequest(Request $request): MainSpanNameListener
29+
{
30+
$this->handler->name();
31+
32+
return $this;
33+
}
34+
35+
/**
36+
* Use non-deprecated check method if availble
37+
*
38+
* @param KernelEvent $event
39+
*
40+
* @return bool
41+
*/
42+
private function isMainRequestEvent(KernelEvent $event): bool
43+
{
44+
if (\method_exists($event, 'isMainRequest')) {
45+
return $event->isMainRequest();
46+
}
47+
48+
return $event->isMasterRequest();
49+
}
50+
}

‎src/Bridge/RequestSpanListener.php‎

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
use Jaeger\Log\ErrorLog;
1010
use Jaeger\Symfony\Name\Generator\NameGeneratorInterface;
1111
use Jaeger\Symfony\Tag\SymfonyComponentTag;
12+
use Jaeger\Symfony\Tag\SymfonyMainRequestTag;
13+
use Jaeger\Symfony\Tag\SymfonySubRequestTag;
1214
use Jaeger\Symfony\Tag\SymfonyVersionTag;
1315
use Jaeger\Tag\ErrorTag;
1416
use Jaeger\Tag\SpanKindServerTag;
@@ -37,17 +39,14 @@ public function __construct(\SplStack $stack, NameGeneratorInterface $nameGenera
3739
public static function getSubscribedEvents(): array
3840
{
3941
return [
40-
RequestEvent::class => ['onRequest', -1024],
42+
RequestEvent::class => ['onRequest', 29],
4143
ResponseEvent::class => ['onResponse', -1024],
4244
ExceptionEvent::class => ['onKernelException', 0],
4345
];
4446
}
4547

4648
public function onResponse(ResponseEvent $event): void
4749
{
48-
if ($this->isMainRequestEvent($event)) {
49-
return;
50-
}
5150
if ($this->spans->isEmpty()) {
5251
return;
5352
}
@@ -56,36 +55,34 @@ public function onResponse(ResponseEvent $event): void
5655

5756
public function onRequest(RequestEvent $event): void
5857
{
59-
if ($this->isMainRequestEvent($event)) {
60-
return;
61-
}
6258
$request = $event->getRequest();
63-
$this->spans->push(
64-
$this->tracer->start(
65-
$this->nameGenerator->generate(),
66-
[
67-
new HttpMethodTag($request->getMethod()),
68-
new HttpUriTag($request->getRequestUri()),
69-
new SpanKindServerTag(),
70-
new SymfonyComponentTag(),
71-
new SymfonyVersionTag(),
72-
]
73-
)
59+
$span = $this->tracer->start(
60+
$this->nameGenerator->generate(),
61+
[
62+
new HttpMethodTag($request->getMethod()),
63+
new HttpUriTag($request->getRequestUri()),
64+
new SpanKindServerTag(),
65+
new SymfonyComponentTag(),
66+
new SymfonyVersionTag(),
67+
]
7468
);
69+
if ($this->isMainRequestEvent($event)) {
70+
$span->addTag(new SymfonyMainRequestTag());
71+
} else {
72+
$span->addTag(new SymfonySubRequestTag());
73+
}
74+
$this->spans->push($span);
7575
}
7676

7777
public function onKernelException(ExceptionEvent $event): void
7878
{
7979
if ($this->spans->isEmpty()) {
8080
return;
8181
}
82-
8382
$exception = $event->getThrowable();
84-
8583
$this->spans->top()
8684
->addTag(new ErrorTag())
87-
->addLog(new ErrorLog($exception->getMessage(), $exception->getTraceAsString()))
88-
;
85+
->addLog(new ErrorLog($exception->getMessage(), $exception->getTraceAsString()));
8986
}
9087

9188
/**

‎src/Name/Generator/ControllerNameGenerator.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public static function getSubscribedEvents(): array
1515
{
1616
return [
1717
// Subscribe after route was resolved and request attributes were set
18-
RequestEvent::class => ['onRequest', 30],
18+
RequestEvent::class => ['onRequest', 31],
1919
TerminateEvent::class => ['onTerminate', -16384],
2020
];
2121
}

‎src/Name/Generator/DefaultNameGenerator.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public static function getSubscribedEvents(): array
1717
{
1818
return [
1919
// Subscribe after route was resolved and request attributes were set
20-
RequestEvent::class => ['onRequest', 30],
20+
RequestEvent::class => ['onRequest', 31],
2121
ConsoleCommandEvent::class => ['onCommand', 31],
2222
TerminateEvent::class => ['onTerminate', -16384],
2323
ConsoleTerminateEvent::class => ['onTerminate'],

‎src/Name/Generator/RequestNameGenerator.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public static function getSubscribedEvents(): array
3535
{
3636
return [
3737
// Subscribe after route was resolved and request attributes were set
38-
RequestEvent::class => ['onRequest', 30],
38+
RequestEvent::class => ['onRequest', 31],
3939
TerminateEvent::class => ['onTerminate', -16384],
4040
];
4141
}

‎src/Resources/services.yml‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,10 @@ services:
155155
jaeger.span.handler.background:
156156
class: Jaeger\Symfony\Bridge\BackgroundSpanHandler
157157
arguments: ['@jaeger.tracer']
158-
jaeger.span.handler.global:
159-
class: Jaeger\Symfony\Bridge\GlobalSpanHandler
158+
jaeger.span.handler.main:
159+
class: Jaeger\Symfony\Bridge\MainSpanHandler
160160
arguments: ['@jaeger.tracer', '@jaeger.name.generator']
161+
jaeger.span.handler.global: '@jaeger.span.handler.main'
161162
jaeger.debug.listener:
162163
class: Jaeger\Symfony\Bridge\DebugListener
163164
arguments:
@@ -180,9 +181,14 @@ services:
180181
- '@jaeger.tracer'
181182
tags:
182183
- {name: 'kernel.event_subscriber' }
183-
jaeger.global.span.listener:
184-
class: Jaeger\Symfony\Bridge\GlobalSpanListener
185-
arguments: ['@jaeger.span.handler.global']
184+
jaeger.global.span.listener.lifecycle:
185+
class: Jaeger\Symfony\Bridge\MainSpanLifecycleListener
186+
arguments: ['@jaeger.span.handler.main']
187+
tags:
188+
- {name: 'kernel.event_subscriber' }
189+
jaeger.global.span.listener.name:
190+
class: Jaeger\Symfony\Bridge\MainSpanNameListener
191+
arguments: ['@jaeger.span.handler.main']
186192
tags:
187193
- {name: 'kernel.event_subscriber' }
188194
jaeger.app.start.listener:
@@ -201,8 +207,8 @@ services:
201207
tags:
202208
- {name: 'kernel.event_subscriber' }
203209
jaeger.span.handler.listener.flush:
204-
class: Jaeger\Symfony\Bridge\HandlerFlushListener
205-
arguments: ['@jaeger.span.handler.background', '@jaeger.span.handler.global']
210+
class: Jaeger\Symfony\Bridge\MainSpanFlushListener
211+
arguments: ['@jaeger.span.handler.background', '@jaeger.span.handler.main']
206212
tags:
207213
- {name: 'kernel.event_subscriber' }
208214
jaeger.exception.listener:

0 commit comments

Comments
 (0)