diff --git a/README.md b/README.md index ec47257..673cc58 100644 --- a/README.md +++ b/README.md @@ -60,30 +60,40 @@ Use static methods anywhere in your codebase without passing the span around: // Set attribute on current span Span::add('db.query_count', 5); -// Capture an exception -Span::error($exception); +// Add warning details as attributes +Span::add('warning', 'retry scheduled'); ``` ### Error Handling -The `setError()` method captures the exception for exporters to process: +Pass the exception to `finish()` when the span ends because of an error: ```php +$span = Span::init('api.request'); + try { // ... } catch (Throwable $e) { - $span->setError($e); + $span->finish(error: $e); throw $e; } ``` Exporters access the exception via `$span->getError()` and extract what they need (message, trace, etc.). -The `level` attribute is automatically set to `error` when an error is captured. You can override it: +Use `setError()` when you need to record the error before the span ends, such as before cleanup work that should still be included in the same span. + +The `level` attribute is set when the span finishes. It defaults to `error` when an error is captured and `info` otherwise. Pass a level to `finish()` to override it: + +```php +$span->finish(level: 'warning', error: $e); +``` + +Use attributes for warning details that do not end the span: ```php -$span->setError($e); -$span->set('level', 'warning'); // override auto-detected level +Span::add('warning', 'retry scheduled'); +Span::add('warning.message', $message); ``` ### Distributed Tracing @@ -226,7 +236,6 @@ $this->assertEquals('http.request', $spans[0]->get('action')); | `init(string $action, ?string $traceparent): Span` | Create and store a new span | | `current(): ?Span` | Get the current span | | `add(string $key, scalar $value)` | Set attribute on current span | -| `error(Throwable $e)` | Capture exception on current span | | `traceparent(): ?string` | Get traceparent header from current span | ### Span (instance) @@ -240,7 +249,7 @@ $this->assertEquals('http.request', $spans[0]->get('action')); | `setError(Throwable $e): self` | Capture exception | | `getError(): ?Throwable` | Get captured exception | | `getTraceparent(): string` | Get W3C traceparent header value | -| `finish(): void` | End span and export | +| `finish(?string $level = null, ?Throwable $error = null): void` | End span and export | ### Attribute Conventions diff --git a/src/Span/Exporter/Exporter.php b/src/Span/Exporter/Exporter.php index 0d7d00c..7f387ef 100644 --- a/src/Span/Exporter/Exporter.php +++ b/src/Span/Exporter/Exporter.php @@ -1,5 +1,7 @@ set($key, $value); } - /** - * Capture an exception on the current span. - * - * Convenience method to record errors without holding a span reference. - * Does nothing if no span is active. - * - * @param Throwable $error The exception to capture - */ - public static function error(Throwable $error): void - { - self::current()?->setError($error); - } - /** * Get the traceparent header value from the current span. * @@ -256,9 +245,16 @@ public function getAttributes(): array * * Sets span.finished_at and span.duration, then sends to all exporters * that pass their sampler (if any). Clears the current span from storage. + * + * @param string|null $level Level to export for this span + * @param Throwable|null $error Exception that caused the span to fail */ - public function finish(): void + public function finish(?string $level = null, ?Throwable $error = null): void { + if ($error instanceof \Throwable) { + $this->setError($error); + } + $finishedAt = microtime(true); /** @var float $startedAt */ $startedAt = $this->attributes['span.started_at']; @@ -266,9 +262,7 @@ public function finish(): void $this->attributes['span.finished_at'] = $finishedAt; $this->attributes['span.duration'] = $finishedAt - $startedAt; - if (!isset($this->attributes['level'])) { - $this->attributes['level'] = $this->error instanceof \Throwable ? 'error' : 'info'; - } + $this->attributes['level'] = $level ?? ($this->error instanceof \Throwable ? 'error' : 'info'); foreach (self::$exporters as $config) { try { diff --git a/src/Span/Storage/Coroutine.php b/src/Span/Storage/Coroutine.php index 7d67877..0899a4d 100644 --- a/src/Span/Storage/Coroutine.php +++ b/src/Span/Storage/Coroutine.php @@ -1,5 +1,7 @@ assertNull(Span::current()); } - public function testErrorSetsErrorOnCurrentSpan(): void + public function testFinishAcceptsError(): void { - $span = Span::init('test'); + $span = new Span(); $error = new RuntimeException('Test'); - Span::error($error); + $span->finish(error: $error); $this->assertSame($error, $span->getError()); } - public function testErrorDoesNothingWhenNoCurrentSpan(): void + public function testFinishWithErrorSetsLevelError(): void { - // Should not throw - Span::error(new RuntimeException('Test')); + $span = new Span(); - $this->assertNull(Span::current()); + $span->finish(error: new RuntimeException('Test')); + + $this->assertSame('error', $span->get('level')); + } + + public function testFinishWithErrorExportsErrorSpan(): void + { + $exported = []; + $exporter = $this->createExporter($exported); + $error = new RuntimeException('Test'); + + Span::addExporter($exporter); + + $span = Span::init('test'); + $span->finish(error: $error); + + $this->assertCount(1, $exported); + $this->assertSame($error, $exported[0]->getError()); } public function testSamplerFiltersExport(): void @@ -575,14 +591,21 @@ public function testFinishSetsLevelErrorWhenErrorSet(): void $this->assertSame('error', $span->get('level')); } - public function testFinishDoesNotOverrideExplicitLevel(): void + public function testFinishAcceptsLevelOverride(): void + { + $span = new Span(); + $span->finish(level: 'warning', error: new RuntimeException('Test')); + + $this->assertSame('warning', $span->get('level')); + } + + public function testFinishOwnsLevelAttribute(): void { $span = new Span(); $span->set('level', 'warning'); - $span->setError(new RuntimeException('Test')); $span->finish(); - $this->assertSame('warning', $span->get('level')); + $this->assertSame('info', $span->get('level')); } public function testLevelNotSetBeforeFinish(): void diff --git a/tests/Storage/AutoTest.php b/tests/Storage/AutoTest.php index 4001af2..97093fe 100644 --- a/tests/Storage/AutoTest.php +++ b/tests/Storage/AutoTest.php @@ -1,5 +1,7 @@