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

Add conditional traceresponse propagation to Symfony auto-instrumentation #146

Merged
merged 2 commits into from
Apr 12, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/Instrumentation/Symfony/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
"symfony/http-kernel": "*",
"symfony/http-client-contracts": "*"
},
"suggest": {
"open-telemetry/opentelemetry-propagation-traceresponse": "Automatically propagate the context to the client."
},
"require-dev": {
"friendsofphp/php-cs-fixer": "^3",
"phan/phan": "^5.0",
Expand All @@ -24,7 +27,8 @@
"open-telemetry/sdk": "^1.0",
"phpunit/phpunit": "^9.5",
"vimeo/psalm": "^4.0",
"symfony/http-client": "^5.4||^6.0"
"symfony/http-client": "^5.4||^6.0",
"open-telemetry/opentelemetry-propagation-traceresponse": "*"
},
"autoload": {
"psr-4": {
Expand All @@ -38,5 +42,10 @@
"psr-4": {
"OpenTelemetry\\Tests\\Instrumentation\\Symfony\\": "tests/"
}
},
"config": {
"allow-plugins": {
"php-http/discovery": false
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
/**
* @internal
*/
final class HeadersPropagator implements PropagationGetterInterface
final class RequestPropagationGetter implements PropagationGetterInterface
{
public static function instance(): self
{
Expand Down
37 changes: 37 additions & 0 deletions src/Instrumentation/Symfony/src/ResponsePropagationSetter.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\Contrib\Instrumentation\Symfony;

use function assert;
use OpenTelemetry\Context\Propagation\PropagationSetterInterface;
use Symfony\Component\HttpFoundation\Response;

/**
* @internal
*/
final class ResponsePropagationSetter implements PropagationSetterInterface
{
public static function instance(): self
{
static $instance;

return $instance ??= new self();
}

/** @psalm-suppress InvalidReturnType */
public function keys($carrier): array
{
assert($carrier instanceof Response);
/** @psalm-suppress InvalidReturnStatement */
return $carrier->headers->keys();
}

public function set(&$carrier, string $key, string $value): void
{
assert($carrier instanceof Response);

$carrier->headers->set($key, $value);
}
}
36 changes: 23 additions & 13 deletions src/Instrumentation/Symfony/src/SymfonyInstrumentation.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public static function register(): void

$parent = Context::getCurrent();
if ($request) {
$parent = Globals::propagator()->extract($request, HeadersPropagator::instance());
$parent = Globals::propagator()->extract($request, RequestPropagationGetter::instance());
$span = $builder
->setParent($parent)
->setAttribute(TraceAttributes::HTTP_URL, $request->getUri())
Expand Down Expand Up @@ -94,19 +94,29 @@ public static function register(): void
$span->setStatus(StatusCode::STATUS_ERROR, $exception->getMessage());
}

if ($response) {
if ($response->getStatusCode() >= Response::HTTP_BAD_REQUEST) {
$span->setStatus(StatusCode::STATUS_ERROR);
}
$span->setAttribute(TraceAttributes::HTTP_STATUS_CODE, $response->getStatusCode());
$span->setAttribute(TraceAttributes::HTTP_FLAVOR, $response->getProtocolVersion());
$contentLength = $response->headers->get('Content-Length');
/** @psalm-suppress PossiblyFalseArgument */
if (null === $contentLength && is_string($response->getContent())) {
$contentLength = \strlen($response->getContent());
}
if (null === $response) {
$span->end();

return;
}

if ($response->getStatusCode() >= Response::HTTP_BAD_REQUEST) {
$span->setStatus(StatusCode::STATUS_ERROR);
}
$span->setAttribute(TraceAttributes::HTTP_STATUS_CODE, $response->getStatusCode());
$span->setAttribute(TraceAttributes::HTTP_FLAVOR, $response->getProtocolVersion());
$contentLength = $response->headers->get('Content-Length');
/** @psalm-suppress PossiblyFalseArgument */
if (null === $contentLength && is_string($response->getContent())) {
$contentLength = \strlen($response->getContent());
}

$span->setAttribute(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH, $contentLength);

$span->setAttribute(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH, $contentLength);
// Propagate traceresponse header to response, if TraceResponsePropagator is present
if (class_exists('OpenTelemetry\Contrib\Propagation\TraceResponse\TraceResponsePropagator')) {
$prop = new \OpenTelemetry\Contrib\Propagation\TraceResponse\TraceResponsePropagator();
$prop->inject($response, ResponsePropagationSetter::instance(), $scope->context());
}

$span->end();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace OpenTelemetry\Tests\Instrumentation\Symfony\tests\Integration;

use OpenTelemetry\Contrib\Propagation\TraceResponse\TraceResponsePropagator;
use OpenTelemetry\SemConv\TraceAttributes;
use Symfony\Component\EventDispatcher\EventDispatcher;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
Expand All @@ -27,7 +28,13 @@ public function test_http_kernel_handle_exception(): void
});
$this->assertCount(0, $this->storage);

$kernel->handle(new Request());
$response = $kernel->handle(new Request());

$this->assertArrayHasKey(
TraceResponsePropagator::TRACERESPONSE,
$response->headers->all(),
'traceresponse header is present if TraceResponsePropagator is present'
);
}

public function test_http_kernel_handle_attributes(): void
Expand All @@ -37,7 +44,7 @@ public function test_http_kernel_handle_attributes(): void
$request = new Request();
$request->attributes->set('_route', 'test_route');

$kernel->handle($request);
$response = $kernel->handle($request);

$attributes = $this->storage[0]->getAttributes();
$this->assertCount(1, $this->storage);
Expand All @@ -49,6 +56,12 @@ public function test_http_kernel_handle_attributes(): void
$this->assertEquals(200, $attributes->get(TraceAttributes::HTTP_STATUS_CODE));
$this->assertEquals('1.0', $attributes->get(TraceAttributes::HTTP_FLAVOR));
$this->assertEquals(5, $attributes->get(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH));

$this->assertArrayHasKey(
TraceResponsePropagator::TRACERESPONSE,
$response->headers->all(),
'traceresponse header is present if TraceResponsePropagator is present'
);
}

public function test_http_kernel_handle_stream_response(): void
Expand All @@ -59,19 +72,31 @@ public function test_http_kernel_handle_stream_response(): void
}));
$this->assertCount(0, $this->storage);

$kernel->handle(new Request());
$response = $kernel->handle(new Request());
$this->assertCount(1, $this->storage);
$this->assertNull($this->storage[0]->getAttributes()->get(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH));

$this->assertArrayHasKey(
TraceResponsePropagator::TRACERESPONSE,
$response->headers->all(),
'traceresponse header is present if TraceResponsePropagator is present'
);
}

public function test_http_kernel_handle_binary_file_response(): void
{
$kernel = $this->getHttpKernel(new EventDispatcher(), fn () => new BinaryFileResponse(__FILE__));
$this->assertCount(0, $this->storage);

$kernel->handle(new Request());
$response = $kernel->handle(new Request());
$this->assertCount(1, $this->storage);
$this->assertNull($this->storage[0]->getAttributes()->get(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH));

$this->assertArrayHasKey(
TraceResponsePropagator::TRACERESPONSE,
$response->headers->all(),
'traceresponse header is present if TraceResponsePropagator is present'
);
}

public function test_http_kernel_handle_with_empty_route(): void
Expand All @@ -81,19 +106,31 @@ public function test_http_kernel_handle_with_empty_route(): void
$request = new Request();
$request->attributes->set('_route', '');

$kernel->handle($request, HttpKernelInterface::MAIN_REQUEST, true);
$response = $kernel->handle($request, HttpKernelInterface::MAIN_REQUEST, true);
$this->assertCount(1, $this->storage);
$this->assertFalse($this->storage[0]->getAttributes()->has(TraceAttributes::HTTP_ROUTE));

$this->assertArrayHasKey(
TraceResponsePropagator::TRACERESPONSE,
$response->headers->all(),
'traceresponse header is present if TraceResponsePropagator is present'
);
}

public function test_http_kernel_handle_without_route(): void
{
$kernel = $this->getHttpKernel(new EventDispatcher());
$this->assertCount(0, $this->storage);

$kernel->handle(new Request(), HttpKernelInterface::MAIN_REQUEST, true);
$response = $kernel->handle(new Request(), HttpKernelInterface::MAIN_REQUEST, true);
$this->assertCount(1, $this->storage);
$this->assertFalse($this->storage[0]->getAttributes()->has(TraceAttributes::HTTP_ROUTE));

$this->assertArrayHasKey(
TraceResponsePropagator::TRACERESPONSE,
$response->headers->all(),
'traceresponse header is present if TraceResponsePropagator is present'
);
}

private function getHttpKernel(EventDispatcherInterface $eventDispatcher, $controller = null, RequestStack $requestStack = null, array $arguments = []): HttpKernel
Expand Down