Set the span status when tracing an HTTP client request (#748)

src/Tracing/HttpClient/AbstractTraceableResponse.php

@@ -5,6 +5,7 @@
 namespace Sentry\SentryBundle\Tracing\HttpClient;
 
 use Sentry\Tracing\Span;
+use Sentry\Tracing\SpanStatus;
 use Symfony\Contracts\HttpClient\ChunkInterface;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
 use Symfony\Contracts\HttpClient\ResponseInterface;
@@ -123,9 +124,27 @@ public static function stream(HttpClientInterface $client, iterable $responses,
 
  private function finishSpan(): void
  {
- if (null !== $this->span) {
- $this->span->finish();
- $this->span = null;
+ if (null === $this->span) {
+ return;
  }
+
+ // We finish the span (which means setting the span end timestamp) first
+ // to ensure the measured time is as close as possible to the duration of
+ // the HTTP request
+ $this->span->finish();
+
+ /** @var int $statusCode */
+ $statusCode = $this->response->getInfo('http_code');
+
+ // If the returned status code is 0, it means that this info isn't available
+ // yet (e.g. an error happened before the request was sent), hence we cannot
+ // determine what happened.
+ if (0 === $statusCode) {
+ $this->span->setStatus(SpanStatus::unknownError());
+ } else {
+ $this->span->setStatus(SpanStatus::createFromHttpStatusCode($statusCode));
+ }
+
+ $this->span = null;
  }
 }

tests/Tracing/HttpClient/TraceableHttpClientTest.php

@@ -19,6 +19,7 @@
 use Sentry\State\Scope;
 use Sentry\Tracing\PropagationContext;
 use Sentry\Tracing\SpanId;
+use Sentry\Tracing\SpanStatus;
 use Sentry\Tracing\TraceId;
 use Sentry\Tracing\Transaction;
 use Sentry\Tracing\TransactionContext;
@@ -107,10 +108,17 @@ public function testRequest(): void
  'http.fragment' => 'baz',
  ];
 
+ // Call gc to invoke destructors at the right time.
+ unset($response);
+
+ gc_mem_caches();
+ gc_collect_cycles();
+
  $this->assertCount(2, $spans);
- $this->assertNull($spans[1]->getEndTimestamp());
+ $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(SpanStatus::ok(), $spans[1]->getStatus());
  $this->assertSame($expectedTags, $spans[1]->getTags());
  $this->assertSame($expectedData, $spans[1]->getData());
  }
@@ -194,6 +202,50 @@ public function testRequestDoesContainsTracingHeadersWithoutTransaction(): void
  $this->assertSame(['baggage: sentry-trace_id=566e3688a61d4bc888951642d6f14a19,sentry-public_key=public,sentry-release=1.0.0,sentry-environment=test'], $mockResponse->getRequestOptions()['normalized_headers']['baggage']);
  }
 
+ public function testRequestSetsUnknownErrorAsSpanStatusIfResponseStatusCodeIsUnavailable(): void
+ {
+ $client = $this->createMock(ClientInterface::class);
+ $client->expects($this->once())
+ ->method('getOptions')
+ ->willReturn(new Options(['dsn' => 'https://public:[email protected]/sentry/1']));
+
+ $transaction = new Transaction(new TransactionContext());
+ $transaction->initSpanRecorder();
+
+ $this->hub->expects($this->once())
+ ->method('getSpan')
+ ->willReturn($transaction);
+
+ $this->hub->expects($this->once())
+ ->method('getClient')
+ ->willReturn($client);
+
+ $decoratedHttpClient = new MockHttpClient(new MockResponse());
+ $httpClient = new TraceableHttpClient($decoratedHttpClient, $this->hub);
+
+ // Cancelling the response is the only way that does not override in any
+ // way the status code and leave it set to 0. This is a required precondition
+ // for the span status to be set to the expected value.
+ $response = $httpClient->request('GET', 'https://www.example.com/test-page');
+ $response->cancel();
+
+ $this->assertNotNull($transaction->getSpanRecorder());
+ $this->assertInstanceOf(AbstractTraceableResponse::class, $response);
+
+ // Call gc to invoke destructors at the right time.
+ unset($response);
+
+ gc_mem_caches();
+ gc_collect_cycles();
+
+ $spans = $transaction->getSpanRecorder()->getSpans();
+
+ $this->assertCount(2, $spans);
+ $this->assertNotNull($spans[1]->getEndTimestamp());
+ $this->assertSame('GET https://www.example.com/test-page', $spans[1]->getDescription());
+ $this->assertSame(SpanStatus::unknownError(), $spans[1]->getStatus());
+ }
+
  public function testStream(): void
  {
  $transaction = new Transaction(new TransactionContext());