From eb2901b166f9e7fbbc2be226ecf39935cd0bf756 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 29 Aug 2023 11:00:07 +0200 Subject: [PATCH 1/5] Add starfish v1 attributes to span data/breadcrumbs --- src/Tracing/HttpClient/AbstractTraceableHttpClient.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Tracing/HttpClient/AbstractTraceableHttpClient.php b/src/Tracing/HttpClient/AbstractTraceableHttpClient.php index 8dc86020..8b72835e 100644 --- a/src/Tracing/HttpClient/AbstractTraceableHttpClient.php +++ b/src/Tracing/HttpClient/AbstractTraceableHttpClient.php @@ -80,6 +80,8 @@ public function request(string $method, string $url, array $options = []): Respo 'http.url' => (string) $partialUri, ]); $context->setData([ + 'http.url' => (string) $partialUri, + 'http.request.method' => $method, 'http.query' => $uri->getQuery(), 'http.fragment' => $uri->getFragment(), ]); From 9dd3bd7ce6f01e5a4ea8a3577af005235c29e75c Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 29 Aug 2023 11:00:21 +0200 Subject: [PATCH 2/5] Rename getSpanTags to getSpanData --- src/Tracing/Doctrine/DBAL/TracingDriverConnection.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php index 6233cd17..d8a13da6 100644 --- a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php +++ b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php @@ -90,7 +90,7 @@ public function __construct( ) { $this->hub = $hub; $this->decoratedConnection = $decoratedConnection; - $this->spanData = $this->getSpanTags($databasePlatform, $params); + $this->spanData = $this->getSpanData($databasePlatform, $params); } /** @@ -258,7 +258,7 @@ private function traceFunction(string $spanOperation, string $spanDescription, \ * * @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/database.md */ - private function getSpanTags(string $databasePlatform, array $params): array + private function getSpanData(string $databasePlatform, array $params): array { $data = ['db.system' => $databasePlatform]; From 2de76d50a03514f6699fad17670a919c9ac0038e Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 29 Aug 2023 12:48:35 +0200 Subject: [PATCH 3/5] Update DB spans --- src/Tracing/Doctrine/DBAL/TracingDriverConnection.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php index d8a13da6..b23dff90 100644 --- a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php +++ b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php @@ -272,20 +272,20 @@ private function getSpanData(string $databasePlatform, array $params): array if (isset($params['host']) && !empty($params['host']) && !isset($params['memory'])) { if (false === filter_var($params['host'], \FILTER_VALIDATE_IP)) { - $data['net.peer.name'] = $params['host']; + $data['server.address'] = $params['host']; } else { - $data['net.peer.ip'] = $params['host']; + $data['server.address'] = $params['host']; } } if (isset($params['port'])) { - $data['net.peer.port'] = (string) $params['port']; + $data['server.port'] = (string) $params['port']; } if (isset($params['unix_socket'])) { - $data['net.transport'] = 'Unix'; + $data['server.socket.address'] = 'Unix'; } elseif (isset($params['memory'])) { - $data['net.transport'] = 'inproc'; + $data['server.socket.address'] = 'inproc'; } return $data; From 087c53a1c887acdd6bdb546771fa46d5430989c2 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 29 Aug 2023 12:51:53 +0200 Subject: [PATCH 4/5] Update tests --- .../Doctrine/DBAL/TracingDriverConnectionTest.php | 12 ++++++------ tests/Tracing/HttpClient/TraceableHttpClientTest.php | 2 ++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/Tracing/Doctrine/DBAL/TracingDriverConnectionTest.php b/tests/Tracing/Doctrine/DBAL/TracingDriverConnectionTest.php index b8dae823..43f96214 100644 --- a/tests/Tracing/Doctrine/DBAL/TracingDriverConnectionTest.php +++ b/tests/Tracing/Doctrine/DBAL/TracingDriverConnectionTest.php @@ -466,8 +466,8 @@ public function spanDataDataProvider(): \Generator 'db.system' => 'foo_platform', 'db.user' => 'root', 'db.name' => 'INFORMATION_SCHEMA', - 'net.peer.port' => '3306', - 'net.transport' => 'Unix', + 'server.port' => '3306', + 'server.socket.address' => 'Unix', ], ]; @@ -482,8 +482,8 @@ public function spanDataDataProvider(): \Generator 'db.system' => 'foo_platform', 'db.user' => 'root', 'db.name' => 'INFORMATION_SCHEMA', - 'net.peer.port' => '3306', - 'net.transport' => 'inproc', + 'server.port' => '3306', + 'server.socket.address' => 'inproc', ], ]; @@ -493,7 +493,7 @@ public function spanDataDataProvider(): \Generator ], [ 'db.system' => 'foo_platform', - 'net.peer.name' => 'localhost', + 'server.address' => 'localhost', ], ]; @@ -503,7 +503,7 @@ public function spanDataDataProvider(): \Generator ], [ 'db.system' => 'foo_platform', - 'net.peer.ip' => '127.0.0.1', + 'server.address' => '127.0.0.1', ], ]; } diff --git a/tests/Tracing/HttpClient/TraceableHttpClientTest.php b/tests/Tracing/HttpClient/TraceableHttpClientTest.php index 5778a31f..1c1b1e88 100644 --- a/tests/Tracing/HttpClient/TraceableHttpClientTest.php +++ b/tests/Tracing/HttpClient/TraceableHttpClientTest.php @@ -104,6 +104,8 @@ public function testRequest(): void 'http.url' => 'https://www.example.com/test-page', ]; $expectedData = [ + 'http.url' => 'https://www.example.com/test-page', + 'http.request.method' => 'GET', 'http.query' => 'foo=bar', 'http.fragment' => 'baz', ]; From 34d181a8902cd3a43fa21cdb945fea0131d61059 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 29 Aug 2023 13:04:27 +0200 Subject: [PATCH 5/5] Migrate tags to data --- src/EventListener/TracingRequestListener.php | 8 ++-- .../TracingSubRequestListener.php | 4 +- .../AbstractTraceableHttpClient.php | 18 +++---- .../TracingRequestListenerTest.php | 48 +++++++++---------- .../TracingSubRequestListenerTest.php | 16 +++---- .../HttpClient/TraceableHttpClientTest.php | 17 +++---- 6 files changed, 54 insertions(+), 57 deletions(-) diff --git a/src/EventListener/TracingRequestListener.php b/src/EventListener/TracingRequestListener.php index 56782fc5..377269c2 100644 --- a/src/EventListener/TracingRequestListener.php +++ b/src/EventListener/TracingRequestListener.php @@ -53,7 +53,7 @@ public function handleKernelRequestEvent(RequestEvent $event): void } $context->setStartTimestamp($requestStartTime); - $context->setTags($this->getTags($request)); + $context->setData($this->getData($request)); $this->hub->setSpan($this->hub->startTransaction($context)); } @@ -76,19 +76,19 @@ public function handleKernelTerminateEvent(TerminateEvent $event): void } /** - * Gets the tags to attach to the transaction. + * Gets the data to attach to the transaction. * * @param Request $request The HTTP request * * @return array */ - private function getTags(Request $request): array + private function getData(Request $request): array { $client = $this->hub->getClient(); $httpFlavor = $this->getHttpFlavor($request); $tags = [ 'net.host.port' => (string) $request->getPort(), - 'http.method' => $request->getMethod(), + 'http.request.method' => $request->getMethod(), 'http.url' => $request->getUri(), 'route' => $this->getRouteName($request), ]; diff --git a/src/EventListener/TracingSubRequestListener.php b/src/EventListener/TracingSubRequestListener.php index b61b15b4..6791d548 100644 --- a/src/EventListener/TracingSubRequestListener.php +++ b/src/EventListener/TracingSubRequestListener.php @@ -37,8 +37,8 @@ public function handleKernelRequestEvent(RequestEvent $event): void $spanContext = new SpanContext(); $spanContext->setOp('http.server'); $spanContext->setDescription(sprintf('%s %s%s%s', $request->getMethod(), $request->getSchemeAndHttpHost(), $request->getBaseUrl(), $request->getPathInfo())); - $spanContext->setTags([ - 'http.method' => $request->getMethod(), + $spanContext->setData([ + 'http.request.method' => $request->getMethod(), 'http.url' => $request->getUri(), 'route' => $this->getRouteName($request), ]); diff --git a/src/Tracing/HttpClient/AbstractTraceableHttpClient.php b/src/Tracing/HttpClient/AbstractTraceableHttpClient.php index 8b72835e..fb5e922b 100644 --- a/src/Tracing/HttpClient/AbstractTraceableHttpClient.php +++ b/src/Tracing/HttpClient/AbstractTraceableHttpClient.php @@ -75,16 +75,18 @@ public function request(string $method, string $url, array $options = []): Respo $context = new SpanContext(); $context->setOp('http.client'); $context->setDescription($method . ' ' . (string) $partialUri); - $context->setTags([ - 'http.method' => $method, - 'http.url' => (string) $partialUri, - ]); - $context->setData([ + + $contextData = [ 'http.url' => (string) $partialUri, 'http.request.method' => $method, - 'http.query' => $uri->getQuery(), - 'http.fragment' => $uri->getFragment(), - ]); + ]; + if ('' !== $uri->getQuery()) { + $contextData['http.query'] = $uri->getQuery(); + } + if ('' !== $uri->getFragment()) { + $contextData['http.fragment'] = $uri->getFragment(); + } + $context->setData($contextData); $childSpan = $span->startChild($context); diff --git a/tests/EventListener/TracingRequestListenerTest.php b/tests/EventListener/TracingRequestListenerTest.php index 5dfa1ff4..1fb74bae 100644 --- a/tests/EventListener/TracingRequestListenerTest.php +++ b/tests/EventListener/TracingRequestListenerTest.php @@ -98,9 +98,9 @@ public function handleKernelRequestEventDataProvider(): \Generator $transactionContext->setSource(TransactionSource::url()); $transactionContext->setOp('http.server'); $transactionContext->setStartTimestamp(1613493597.010275); - $transactionContext->setTags([ + $transactionContext->setData([ 'net.host.port' => '80', - 'http.method' => 'GET', + 'http.request.method' => 'GET', 'http.url' => 'http://www.example.com/', 'http.flavor' => '1.1', 'route' => '', @@ -135,9 +135,9 @@ public function handleKernelRequestEventDataProvider(): \Generator $transactionContext->setSource(TransactionSource::url()); $transactionContext->setOp('http.server'); $transactionContext->setStartTimestamp(1613493597.010275); - $transactionContext->setTags([ + $transactionContext->setData([ 'net.host.port' => '80', - 'http.method' => 'GET', + 'http.request.method' => 'GET', 'http.url' => 'http://www.example.com/', 'http.flavor' => '1.1', 'route' => '', @@ -167,9 +167,9 @@ public function handleKernelRequestEventDataProvider(): \Generator $transactionContext->setSource(TransactionSource::url()); $transactionContext->setOp('http.server'); $transactionContext->setStartTimestamp(1613493597.010275); - $transactionContext->setTags([ + $transactionContext->setData([ 'net.host.port' => '80', - 'http.method' => 'GET', + 'http.request.method' => 'GET', 'http.url' => 'http://www.example.com/', 'http.flavor' => '1.1', 'route' => '', @@ -190,9 +190,9 @@ public function handleKernelRequestEventDataProvider(): \Generator $transactionContext->setSource(TransactionSource::url()); $transactionContext->setOp('http.server'); $transactionContext->setStartTimestamp(1613493597.010275); - $transactionContext->setTags([ + $transactionContext->setData([ 'net.host.port' => '80', - 'http.method' => 'GET', + 'http.request.method' => 'GET', 'http.url' => 'http://127.0.0.1/', 'http.flavor' => '1.1', 'route' => '', @@ -221,9 +221,9 @@ public function handleKernelRequestEventDataProvider(): \Generator $transactionContext->setSource(TransactionSource::route()); $transactionContext->setOp('http.server'); $transactionContext->setStartTimestamp(1613493597.010275); - $transactionContext->setTags([ + $transactionContext->setData([ 'net.host.port' => '80', - 'http.method' => 'GET', + 'http.request.method' => 'GET', 'http.url' => 'http://www.example.com/path', 'http.flavor' => '1.1', 'route' => 'app_homepage', @@ -245,9 +245,9 @@ public function handleKernelRequestEventDataProvider(): \Generator $transactionContext->setSource(TransactionSource::url()); $transactionContext->setOp('http.server'); $transactionContext->setStartTimestamp(1613493597.010275); - $transactionContext->setTags([ + $transactionContext->setData([ 'net.host.port' => '80', - 'http.method' => 'GET', + 'http.request.method' => 'GET', 'http.url' => 'http://www.example.com/path', 'http.flavor' => '1.1', 'route' => '/path', @@ -269,9 +269,9 @@ public function handleKernelRequestEventDataProvider(): \Generator $transactionContext->setSource(TransactionSource::url()); $transactionContext->setOp('http.server'); $transactionContext->setStartTimestamp(1613493597.010275); - $transactionContext->setTags([ + $transactionContext->setData([ 'net.host.port' => '80', - 'http.method' => 'GET', + 'http.request.method' => 'GET', 'http.url' => 'http://www.example.com/', 'http.flavor' => '1.1', 'route' => 'App\\Controller::indexAction', @@ -293,9 +293,9 @@ public function handleKernelRequestEventDataProvider(): \Generator $transactionContext->setSource(TransactionSource::url()); $transactionContext->setOp('http.server'); $transactionContext->setStartTimestamp(1613493597.010275); - $transactionContext->setTags([ + $transactionContext->setData([ 'net.host.port' => '80', - 'http.method' => 'GET', + 'http.request.method' => 'GET', 'http.url' => 'http://www.example.com/', 'http.flavor' => '1.1', 'route' => 'App\\Controller::indexAction', @@ -317,9 +317,9 @@ public function handleKernelRequestEventDataProvider(): \Generator $transactionContext->setSource(TransactionSource::url()); $transactionContext->setOp('http.server'); $transactionContext->setStartTimestamp(1613493597.010275); - $transactionContext->setTags([ + $transactionContext->setData([ 'net.host.port' => '80', - 'http.method' => 'GET', + 'http.request.method' => 'GET', 'http.url' => 'http://www.example.com/', 'http.flavor' => '1.1', 'route' => 'class@anonymous::indexAction', @@ -341,9 +341,9 @@ public function handleKernelRequestEventDataProvider(): \Generator $transactionContext->setSource(TransactionSource::url()); $transactionContext->setOp('http.server'); $transactionContext->setStartTimestamp(1613493597.010275); - $transactionContext->setTags([ + $transactionContext->setData([ 'net.host.port' => '80', - 'http.method' => 'GET', + 'http.request.method' => 'GET', 'http.url' => 'http://www.example.com/', 'http.flavor' => '1.1', 'route' => '', @@ -365,9 +365,9 @@ public function handleKernelRequestEventDataProvider(): \Generator $transactionContext->setSource(TransactionSource::url()); $transactionContext->setOp('http.server'); $transactionContext->setStartTimestamp(1613493597.010275); - $transactionContext->setTags([ + $transactionContext->setData([ 'net.host.port' => '80', - 'http.method' => 'GET', + 'http.request.method' => 'GET', 'http.url' => 'http://www.example.com/', 'http.flavor' => '1.1', 'route' => '', @@ -389,9 +389,9 @@ public function handleKernelRequestEventDataProvider(): \Generator $transactionContext->setSource(TransactionSource::url()); $transactionContext->setOp('http.server'); $transactionContext->setStartTimestamp(1613493597.010275); - $transactionContext->setTags([ + $transactionContext->setData([ 'net.host.port' => '', - 'http.method' => 'GET', + 'http.request.method' => 'GET', 'http.url' => 'http://:/', 'route' => '', 'net.host.name' => '', diff --git a/tests/EventListener/TracingSubRequestListenerTest.php b/tests/EventListener/TracingSubRequestListenerTest.php index 9503e2a3..ef9da8d1 100644 --- a/tests/EventListener/TracingSubRequestListenerTest.php +++ b/tests/EventListener/TracingSubRequestListenerTest.php @@ -75,8 +75,8 @@ public function handleKernelRequestEventDataProvider(): \Generator $span = new Span(); $span->setOp('http.server'); $span->setDescription('GET http://www.example.com/path'); - $span->setTags([ - 'http.method' => 'GET', + $span->setData([ + 'http.request.method' => 'GET', 'http.url' => 'http://www.example.com/path', 'route' => 'App\\Controller::indexAction', ]); @@ -92,8 +92,8 @@ public function handleKernelRequestEventDataProvider(): \Generator $span = new Span(); $span->setOp('http.server'); $span->setDescription('GET http://www.example.com/'); - $span->setTags([ - 'http.method' => 'GET', + $span->setData([ + 'http.request.method' => 'GET', 'http.url' => 'http://www.example.com/', 'route' => 'App\\Controller::indexAction', ]); @@ -109,8 +109,8 @@ public function handleKernelRequestEventDataProvider(): \Generator $span = new Span(); $span->setOp('http.server'); $span->setDescription('GET http://www.example.com/'); - $span->setTags([ - 'http.method' => 'GET', + $span->setData([ + 'http.request.method' => 'GET', 'http.url' => 'http://www.example.com/', 'route' => 'class@anonymous::indexAction', ]); @@ -126,8 +126,8 @@ public function handleKernelRequestEventDataProvider(): \Generator $span = new Span(); $span->setOp('http.server'); $span->setDescription('GET http://www.example.com/'); - $span->setTags([ - 'http.method' => 'GET', + $span->setData([ + 'http.request.method' => 'GET', 'http.url' => 'http://www.example.com/', 'route' => '', ]); diff --git a/tests/Tracing/HttpClient/TraceableHttpClientTest.php b/tests/Tracing/HttpClient/TraceableHttpClientTest.php index 1c1b1e88..af4b6bdc 100644 --- a/tests/Tracing/HttpClient/TraceableHttpClientTest.php +++ b/tests/Tracing/HttpClient/TraceableHttpClientTest.php @@ -99,10 +99,6 @@ public function testRequest(): void $this->assertNotNull($transaction->getSpanRecorder()); $spans = $transaction->getSpanRecorder()->getSpans(); - $expectedTags = [ - 'http.method' => 'GET', - 'http.url' => 'https://www.example.com/test-page', - ]; $expectedData = [ 'http.url' => 'https://www.example.com/test-page', 'http.request.method' => 'GET', @@ -121,7 +117,6 @@ public function testRequest(): void $this->assertSame('http.client', $spans[1]->getOp()); $this->assertSame('GET https://www.example.com/test-page', $spans[1]->getDescription()); $this->assertSame(SpanStatus::ok(), $spans[1]->getStatus()); - $this->assertSame($expectedTags, $spans[1]->getTags()); $this->assertSame($expectedData, $spans[1]->getData()); } @@ -160,16 +155,16 @@ public function testRequestDoesNotContainTracingHeaders(): void $this->assertNotNull($transaction->getSpanRecorder()); $spans = $transaction->getSpanRecorder()->getSpans(); - $expectedTags = [ - 'http.method' => 'PUT', + $expectedData = [ 'http.url' => 'https://www.example.com/test-page', + 'http.request.method' => 'PUT', ]; $this->assertCount(2, $spans); $this->assertNull($spans[1]->getEndTimestamp()); $this->assertSame('http.client', $spans[1]->getOp()); $this->assertSame('PUT https://www.example.com/test-page', $spans[1]->getDescription()); - $this->assertSame($expectedTags, $spans[1]->getTags()); + $this->assertSame($expectedData, $spans[1]->getData()); } public function testRequestDoesContainsTracingHeadersWithoutTransaction(): void @@ -271,9 +266,9 @@ public function testStream(): void $this->assertNotNull($transaction->getSpanRecorder()); $spans = $transaction->getSpanRecorder()->getSpans(); - $expectedTags = [ - 'http.method' => 'GET', + $expectedData = [ 'http.url' => 'https://www.example.com/test-page', + 'http.request.method' => 'GET', ]; $this->assertSame('foobar', implode('', $chunks)); @@ -282,7 +277,7 @@ public function testStream(): void $this->assertNotNull($spans[1]->getEndTimestamp()); $this->assertSame('http.client', $spans[1]->getOp()); $this->assertSame('GET https://www.example.com/test-page', $spans[1]->getDescription()); - $this->assertSame($expectedTags, $spans[1]->getTags()); + $this->assertSame($expectedData, $spans[1]->getData()); $loopIndex = 0;