Skip to content

Commit

Permalink
span status compliance (#884)
Browse files Browse the repository at this point in the history
implement spec-compliance for span status:
- Ok is final
- Unset should be ignored
- Error can be updated to Ok

ref: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status
  • Loading branch information
brettmc authored Dec 7, 2022
1 parent 311afa2 commit 02d9421
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/SDK/Trace/Span.php
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,15 @@ public function setStatus(string $code, string $description = null): self
return $this;
}

// An attempt to set value Unset SHOULD be ignored.
if ($code === API\StatusCode::STATUS_UNSET) {
return $this;
}

// When span status is set to Ok it SHOULD be considered final and any further attempts to change it SHOULD be ignored.
if ($this->status->getCode() === API\StatusCode::STATUS_OK) {
return $this;
}
$this->status = StatusData::create($code, $description);

return $this;
Expand Down
37 changes: 37 additions & 0 deletions tests/Unit/SDK/Trace/SpanTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use OpenTelemetry\API\Trace\NonRecordingSpan;
use OpenTelemetry\API\Trace\SpanContext;
use OpenTelemetry\API\Trace\SpanContextValidator;
use OpenTelemetry\API\Trace\StatusCode;
use OpenTelemetry\Context\Context;
use OpenTelemetry\SDK\Common\Attribute\Attributes;
use OpenTelemetry\SDK\Common\Attribute\AttributesInterface;
Expand Down Expand Up @@ -747,6 +748,42 @@ public function test_add_event_order_preserved(): void
$this->assertEvent($events[2], 'c', Attributes::create(['key' => 2]), self::START_EPOCH);
}

/**
* @dataProvider statusCodeProvider
* @group trace-compliance
* @psalm-param StatusCode::STATUS_* $code
*
* When span status is set to Ok it SHOULD be considered final and any further attempts to change it SHOULD be ignored.
*/
public function test_set_status_after_ok_is_ignored(string $code): void
{
$span = $this->createTestRootSpan();
$span->setStatus(API\StatusCode::STATUS_OK);
$this->assertSame(API\StatusCode::STATUS_OK, $span->toSpanData()->getStatus()->getCode());
$span->setStatus($code);
$this->assertNotSame($code, $span->toSpanData()->getStatus()->getCode(), 'update after Ok was ignored');
$span->setStatus(API\StatusCode::STATUS_OK);
$this->assertSame(API\StatusCode::STATUS_OK, $span->toSpanData()->getStatus()->getCode());
}

public function statusCodeProvider(): array
{
return [
[API\StatusCode::STATUS_UNSET],
[API\StatusCode::STATUS_ERROR],
];
}

/**
* @group trace-compliance
*/
public function test_can_set_status_to_ok_after_error(): void
{
$span = $this->createTestRootSpan();
$span->setStatus(API\StatusCode::STATUS_ERROR);
$this->assertSame(API\StatusCode::STATUS_ERROR, $span->toSpanData()->getStatus()->getCode());
}

private function createTestRootSpan(): Span
{
return $this
Expand Down

0 comments on commit 02d9421

Please sign in to comment.