From 4623de889f0faf87eac4934a5bec85e99db125eb Mon Sep 17 00:00:00 2001 From: Chirag Aggarwal Date: Mon, 27 Apr 2026 11:39:40 +0530 Subject: [PATCH 1/6] Move span errors to finish --- README.md | 22 +++++++++++++++------- src/Span/Span.php | 21 +++++++-------------- tests/SpanTest.php | 30 +++++++++++++++++++++++------- 3 files changed, 45 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index ec47257..04f072a 100644 --- a/README.md +++ b/README.md @@ -60,19 +60,21 @@ 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 error details as attributes +Span::add('error', $details); ``` ### 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($e); throw $e; } ``` @@ -82,8 +84,15 @@ Exporters access the exception via `$span->getError()` and extract what they nee The `level` attribute is automatically set to `error` when an error is captured. You can override it: ```php -$span->setError($e); $span->set('level', 'warning'); // override auto-detected level +$span->finish($e); +``` + +Use attributes for error-like details that do not end the span: + +```php +Span::add('error', $details); +Span::add('error.message', $message); ``` ### Distributed Tracing @@ -226,7 +235,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 +248,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(?Throwable $e = null): void` | End span and export | ### Attribute Conventions diff --git a/src/Span/Span.php b/src/Span/Span.php index b519b64..28bb8c9 100644 --- a/src/Span/Span.php +++ b/src/Span/Span.php @@ -143,19 +143,6 @@ public static function add(string $key, string|int|float|bool|null $value): void self::current()?->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 +243,15 @@ 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 Throwable|null $error Exception that caused the span to fail */ - public function finish(): void + public function finish(?Throwable $error = null): void { + if ($error instanceof Throwable) { + $this->error = $error; + } + $finishedAt = microtime(true); /** @var float $startedAt */ $startedAt = $this->attributes['span.started_at']; diff --git a/tests/SpanTest.php b/tests/SpanTest.php index bdee001..8390a85 100644 --- a/tests/SpanTest.php +++ b/tests/SpanTest.php @@ -215,22 +215,38 @@ public function testAddDoesNothingWhenNoCurrentSpan(): void $this->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); $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(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); + + $this->assertCount(1, $exported); + $this->assertSame($error, $exported[0]->getError()); } public function testSamplerFiltersExport(): void From 78f27533ab22f37a69dc3cb40e9d35352f8b0f96 Mon Sep 17 00:00:00 2001 From: Chirag Aggarwal Date: Mon, 27 Apr 2026 12:17:02 +0530 Subject: [PATCH 2/6] Address PR feedback for span error finish --- README.md | 2 ++ src/Span/Exporter/Exporter.php | 2 ++ src/Span/Exporter/None.php | 2 ++ src/Span/Exporter/SentryField.php | 2 ++ src/Span/Exporter/Stdout.php | 2 ++ src/Span/Span.php | 4 ++-- src/Span/Storage/Coroutine.php | 2 ++ src/Span/Storage/Memory.php | 2 ++ src/Span/Storage/Storage.php | 2 ++ tests/Exporter/PrettyTest.php | 2 ++ tests/Exporter/SentryTest.php | 2 ++ tests/Exporter/StdoutTest.php | 2 ++ tests/Storage/AutoTest.php | 2 ++ 13 files changed, 26 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 04f072a..08be01d 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,8 @@ try { Exporters access the exception via `$span->getError()` and extract what they need (message, trace, etc.). +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 automatically set to `error` when an error is captured. You can override it: ```php 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 @@ error = $error; + if ($error instanceof \Throwable) { + $this->setError($error); } $finishedAt = microtime(true); 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 @@ Date: Mon, 27 Apr 2026 12:29:29 +0530 Subject: [PATCH 3/6] Add strict types to span --- src/Span/Span.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Span/Span.php b/src/Span/Span.php index 34fda61..f31ef8e 100644 --- a/src/Span/Span.php +++ b/src/Span/Span.php @@ -1,5 +1,7 @@ Date: Mon, 27 Apr 2026 13:02:43 +0530 Subject: [PATCH 4/6] Move level override to finish --- README.md | 7 +++---- src/Span/Span.php | 7 +++---- tests/SpanTest.php | 13 ++++++++++--- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 08be01d..a810ddf 100644 --- a/README.md +++ b/README.md @@ -83,11 +83,10 @@ Exporters access the exception via `$span->getError()` and extract what they nee 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 automatically set to `error` when an error is captured. You can override it: +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->set('level', 'warning'); // override auto-detected level -$span->finish($e); +$span->finish($e, level: 'warning'); ``` Use attributes for error-like details that do not end the span: @@ -250,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(?Throwable $e = null): void` | End span and export | +| `finish(?Throwable $e = null, ?string $level = null): void` | End span and export | ### Attribute Conventions diff --git a/src/Span/Span.php b/src/Span/Span.php index f31ef8e..405ba0b 100644 --- a/src/Span/Span.php +++ b/src/Span/Span.php @@ -247,8 +247,9 @@ public function getAttributes(): array * that pass their sampler (if any). Clears the current span from storage. * * @param Throwable|null $error Exception that caused the span to fail + * @param string|null $level Level to export for this span */ - public function finish(?Throwable $error = null): void + public function finish(?Throwable $error = null, ?string $level = null): void { if ($error instanceof \Throwable) { $this->setError($error); @@ -261,9 +262,7 @@ public function finish(?Throwable $error = null): 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/tests/SpanTest.php b/tests/SpanTest.php index 8390a85..56de095 100644 --- a/tests/SpanTest.php +++ b/tests/SpanTest.php @@ -591,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(new RuntimeException('Test'), level: 'warning'); + + $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 From 34a63b5cfa05c83fdc76fea87416807d24ba162a Mon Sep 17 00:00:00 2001 From: Chirag Aggarwal Date: Mon, 27 Apr 2026 13:15:26 +0530 Subject: [PATCH 5/6] Address maintainer finish API feedback --- README.md | 16 ++++++++-------- src/Span/Span.php | 4 ++-- tests/SpanTest.php | 8 ++++---- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index a810ddf..286215b 100644 --- a/README.md +++ b/README.md @@ -60,8 +60,8 @@ Use static methods anywhere in your codebase without passing the span around: // Set attribute on current span Span::add('db.query_count', 5); -// Add error details as attributes -Span::add('error', $details); +// Add warning details as attributes +Span::add('warning', 'retry scheduled'); ``` ### Error Handling @@ -74,7 +74,7 @@ $span = Span::init('api.request'); try { // ... } catch (Throwable $e) { - $span->finish($e); + $span->finish(error: $e); throw $e; } ``` @@ -86,14 +86,14 @@ Use `setError()` when you need to record the error before the span ends, such as 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($e, level: 'warning'); +$span->finish(level: 'warning', error: $e); ``` -Use attributes for error-like details that do not end the span: +Use attributes for warning details that do not end the span: ```php -Span::add('error', $details); -Span::add('error.message', $message); +Span::add('warning', 'retry scheduled'); +Span::add('warning.message', $message); ``` ### Distributed Tracing @@ -249,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(?Throwable $e = null, ?string $level = null): void` | End span and export | +| `finish(?string $level = null, ?Throwable $e = null): void` | End span and export | ### Attribute Conventions diff --git a/src/Span/Span.php b/src/Span/Span.php index 405ba0b..6ad7f0b 100644 --- a/src/Span/Span.php +++ b/src/Span/Span.php @@ -246,10 +246,10 @@ 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 Throwable|null $error Exception that caused the span to fail * @param string|null $level Level to export for this span + * @param Throwable|null $error Exception that caused the span to fail */ - public function finish(?Throwable $error = null, ?string $level = null): void + public function finish(?string $level = null, ?Throwable $error = null): void { if ($error instanceof \Throwable) { $this->setError($error); diff --git a/tests/SpanTest.php b/tests/SpanTest.php index 56de095..43baba2 100644 --- a/tests/SpanTest.php +++ b/tests/SpanTest.php @@ -220,7 +220,7 @@ public function testFinishAcceptsError(): void $span = new Span(); $error = new RuntimeException('Test'); - $span->finish($error); + $span->finish(error: $error); $this->assertSame($error, $span->getError()); } @@ -229,7 +229,7 @@ public function testFinishWithErrorSetsLevelError(): void { $span = new Span(); - $span->finish(new RuntimeException('Test')); + $span->finish(error: new RuntimeException('Test')); $this->assertSame('error', $span->get('level')); } @@ -243,7 +243,7 @@ public function testFinishWithErrorExportsErrorSpan(): void Span::addExporter($exporter); $span = Span::init('test'); - $span->finish($error); + $span->finish(error: $error); $this->assertCount(1, $exported); $this->assertSame($error, $exported[0]->getError()); @@ -594,7 +594,7 @@ public function testFinishSetsLevelErrorWhenErrorSet(): void public function testFinishAcceptsLevelOverride(): void { $span = new Span(); - $span->finish(new RuntimeException('Test'), level: 'warning'); + $span->finish(level: 'warning', error: new RuntimeException('Test')); $this->assertSame('warning', $span->get('level')); } From 09f14bd68d98a169e07af40d9815c489b878bf5e Mon Sep 17 00:00:00 2001 From: Chirag Aggarwal Date: Mon, 27 Apr 2026 13:18:27 +0530 Subject: [PATCH 6/6] Fix finish error parameter docs --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 286215b..673cc58 100644 --- a/README.md +++ b/README.md @@ -249,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(?string $level = null, ?Throwable $e = null): void` | End span and export | +| `finish(?string $level = null, ?Throwable $error = null): void` | End span and export | ### Attribute Conventions