From c3066fad5398deb598f4a8d467cd4b4eb58b6743 Mon Sep 17 00:00:00 2001 From: ambkhand Date: Wed, 1 Jun 2022 16:26:29 +0530 Subject: [PATCH 01/10] Otel-php:632 Move stack trace formatting out of Span class --- src/SDK/Common/Util/TracingUtil.php | 79 +++++++++++++++++++++++++++++ src/SDK/Trace/Span.php | 79 ++++------------------------- tests/Unit/SDK/Trace/SpanTest.php | 6 ++- 3 files changed, 94 insertions(+), 70 deletions(-) create mode 100644 src/SDK/Common/Util/TracingUtil.php diff --git a/src/SDK/Common/Util/TracingUtil.php b/src/SDK/Common/Util/TracingUtil.php new file mode 100644 index 000000000..5011b86be --- /dev/null +++ b/src/SDK/Common/Util/TracingUtil.php @@ -0,0 +1,79 @@ +getTrace(); + $prev = $e->getPrevious(); + $result[] = sprintf('%s%s: %s', $starter, get_class($e), $e->getMessage()); + $file = $e->getFile(); + $line = $e->getLine(); + while (true) { + $current = "$file:$line"; + if (in_array($current, $seen, true)) { + $result[] = sprintf(' ... %d more', count($trace)+1); + + break; + } + + // Lambda to format traces -- we want to format the trace with '.' separators + $slashToDot = function ($str) { + return str_replace('\\', '.', $str); + }; + + $traceHasKey = array_key_exists(0, $trace); + $traceKeyHasClass = $traceHasKey && array_key_exists('class', $trace[0]); + $traceKeyHasFunction = $traceKeyHasClass && array_key_exists('function', $trace[0]); + + $result[] = sprintf( + ' at %s%s%s(%s%s%s)', + $traceKeyHasClass ? $slashToDot($trace[0]['class']) : '', + $traceKeyHasFunction ? '.' : '', + $traceKeyHasFunction ? $slashToDot($trace[0]['function']) : 'main', + $line === null ? $file : basename($file), + $line === null ? '' : ':', + $line ?? '' + ); + $seen[] = "$file:$line"; + if (!count($trace)) { + break; + } + $file = array_key_exists('file', $trace[0]) ? $trace[0]['file'] : 'Unknown Source'; + $line = array_key_exists('file', $trace[0]) && array_key_exists('line', $trace[0]) && $trace[0]['line'] ? $trace[0]['line'] : null; + array_shift($trace); + } + $result = implode("\n", $result); + if ($prev) { + $result .= "\n" . self::formatStackTrace($prev, $seen); + } + + return $result; + } +} diff --git a/src/SDK/Trace/Span.php b/src/SDK/Trace/Span.php index 9e5d3ea59..e1a609afc 100644 --- a/src/SDK/Trace/Span.php +++ b/src/SDK/Trace/Span.php @@ -4,23 +4,19 @@ namespace OpenTelemetry\SDK\Trace; -use function array_key_exists; -use function array_shift; -use function basename; use function count; use function ctype_space; use function get_class; -use function in_array; use OpenTelemetry\API\Trace as API; use OpenTelemetry\Context\Context; use OpenTelemetry\SDK\Common\Attribute\AttributeLimits; use OpenTelemetry\SDK\Common\Attribute\Attributes; use OpenTelemetry\SDK\Common\Attribute\AttributesInterface; +use OpenTelemetry\SDK\Common\Dev\Compatibility\Util as BcUtil; use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeInterface; use OpenTelemetry\SDK\Common\Time\ClockFactory; +use OpenTelemetry\SDK\Common\Util\TracingUtil; use OpenTelemetry\SDK\Resource\ResourceInfo; -use function sprintf; -use function str_replace; use Throwable; final class Span extends API\AbstractSpan implements ReadWriteSpanInterface @@ -153,72 +149,19 @@ public static function startSpan( } /** - * This function provides a more java-like stacktrace - * that supports exception chaining and provides exact - * lines of where exceptions are thrown - * - * Example: - * Exception: Thrown from grandparent - * at grandparent_func(test.php:56) - * at parent_func(test.php:51) - * at child_func(test.php:44) - * at (main)(test.php:62) - * - * Credit: https://www.php.net/manual/en/exception.gettraceasstring.php#114980 + * Backward compatibility methods * + * @codeCoverageIgnore */ public static function formatStackTrace(Throwable $e, array &$seen = null): string { - $starter = $seen ? 'Caused by: ' : ''; - $result = []; - if (!$seen) { - $seen = []; - } - $trace = $e->getTrace(); - $prev = $e->getPrevious(); - $result[] = sprintf('%s%s: %s', $starter, get_class($e), $e->getMessage()); - $file = $e->getFile(); - $line = $e->getLine(); - while (true) { - $current = "$file:$line"; - if (in_array($current, $seen, true)) { - $result[] = sprintf(' ... %d more', count($trace)+1); - - break; - } - - // Lambda to format traces -- we want to format the trace with '.' separators - $slashToDot = function ($str) { - return str_replace('\\', '.', $str); - }; - - $traceHasKey = array_key_exists(0, $trace); - $traceKeyHasClass = $traceHasKey && array_key_exists('class', $trace[0]); - $traceKeyHasFunction = $traceKeyHasClass && array_key_exists('function', $trace[0]); - - $result[] = sprintf( - ' at %s%s%s(%s%s%s)', - $traceKeyHasClass ? $slashToDot($trace[0]['class']) : '', - $traceKeyHasFunction ? '.' : '', - $traceKeyHasFunction ? $slashToDot($trace[0]['function']) : 'main', - $line === null ? $file : basename($file), - $line === null ? '' : ':', - $line ?? '' - ); - $seen[] = "$file:$line"; - if (!count($trace)) { - break; - } - $file = array_key_exists('file', $trace[0]) ? $trace[0]['file'] : 'Unknown Source'; - $line = array_key_exists('file', $trace[0]) && array_key_exists('line', $trace[0]) && $trace[0]['line'] ? $trace[0]['line'] : null; - array_shift($trace); - } - $result = implode("\n", $result); - if ($prev) { - $result .= "\n" . self::formatStackTrace($prev, $seen); - } + BcUtil::triggerMethodDeprecationNotice( + __METHOD__, + 'formatStackTrace', + TracingUtil::class + ); - return $result; + return TracingUtil::formatStackTrace($e, $seen); } /** @inheritDoc */ @@ -292,7 +235,7 @@ public function recordException(Throwable $exception, iterable $attributes = []) $eventAttributes = new Attributes([ 'exception.type' => get_class($exception), 'exception.message' => $exception->getMessage(), - 'exception.stacktrace' => self::formatStackTrace($exception), + 'exception.stacktrace' => TracingUtil::formatStackTrace($exception), ]); foreach ($attributes as $key => $value) { diff --git a/tests/Unit/SDK/Trace/SpanTest.php b/tests/Unit/SDK/Trace/SpanTest.php index 21f5e7001..148e56370 100644 --- a/tests/Unit/SDK/Trace/SpanTest.php +++ b/tests/Unit/SDK/Trace/SpanTest.php @@ -19,6 +19,7 @@ use OpenTelemetry\SDK\Common\Time\ClockFactory; use OpenTelemetry\SDK\Common\Time\ClockInterface; use OpenTelemetry\SDK\Common\Time\Util as TimeUtil; +use OpenTelemetry\SDK\Common\Util\TracingUtil; use OpenTelemetry\SDK\Resource\ResourceInfo; use OpenTelemetry\SDK\Resource\ResourceInfoFactory; use OpenTelemetry\SDK\Trace\Event; @@ -34,6 +35,7 @@ use OpenTelemetry\SDK\Trace\SpanProcessorInterface; use OpenTelemetry\SDK\Trace\StatusData; use OpenTelemetry\Tests\Unit\SDK\Util\TestClock; + use function range; use function str_repeat; @@ -569,7 +571,7 @@ public function test_record_exception(): void new Attributes([ 'exception.type' => 'Exception', 'exception.message' => 'ERR', - 'exception.stacktrace' => Span::formatStackTrace($exception), + 'exception.stacktrace' => TracingUtil::formatStackTrace($exception), ]), $event->getAttributes() ); @@ -595,7 +597,7 @@ public function test_record_exception_additional_attributes(): void new Attributes([ 'exception.type' => 'Exception', 'exception.message' => 'ERR', - 'exception.stacktrace' => Span::formatStackTrace($exception), + 'exception.stacktrace' => TracingUtil::formatStackTrace($exception), 'foo' => 'bar', ]), $event->getAttributes() From 99a42734dc819e99604132a247250ab6d464e8f3 Mon Sep 17 00:00:00 2001 From: ambkhand Date: Wed, 1 Jun 2022 18:46:18 +0530 Subject: [PATCH 02/10] added function usages --- src/SDK/Common/Util/TracingUtil.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/SDK/Common/Util/TracingUtil.php b/src/SDK/Common/Util/TracingUtil.php index 5011b86be..5e0e7dc09 100644 --- a/src/SDK/Common/Util/TracingUtil.php +++ b/src/SDK/Common/Util/TracingUtil.php @@ -4,6 +4,13 @@ namespace OpenTelemetry\SDK\Common\Util; +use function array_key_exists; +use function array_shift; +use function basename; +use function sprintf; +use function str_replace; +use function in_array; + use Throwable; class TracingUtil From c8365d1f0dad093364805943b944e5e424e67bf9 Mon Sep 17 00:00:00 2001 From: ambkhand Date: Thu, 2 Jun 2022 13:03:33 +0530 Subject: [PATCH 03/10] fix linting errors --- src/SDK/Common/Util/TracingUtil.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SDK/Common/Util/TracingUtil.php b/src/SDK/Common/Util/TracingUtil.php index 5e0e7dc09..068a555d2 100644 --- a/src/SDK/Common/Util/TracingUtil.php +++ b/src/SDK/Common/Util/TracingUtil.php @@ -7,9 +7,9 @@ use function array_key_exists; use function array_shift; use function basename; +use function in_array; use function sprintf; use function str_replace; -use function in_array; use Throwable; From b02b7ea99e28ee99e928e8389bd86176096b1f7d Mon Sep 17 00:00:00 2001 From: ambkhand Date: Sat, 25 Jun 2022 12:09:03 +0530 Subject: [PATCH 04/10] Added documentation for adding Span Attributes --- src/API/Trace/SpanInterface.php | 3 ++- src/SDK/Trace/Span.php | 2 +- tests/Unit/SDK/Trace/SpanTest.php | 1 - 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/API/Trace/SpanInterface.php b/src/API/Trace/SpanInterface.php index 8c9c06898..d82a9b5c2 100644 --- a/src/API/Trace/SpanInterface.php +++ b/src/API/Trace/SpanInterface.php @@ -50,7 +50,8 @@ public function isRecording(): bool; /** * @see https://github.com/open-telemetry/opentelemetry-specification/blob/v1.6.1/specification/trace/api.md#set-attributes - * + * Adding attributes at span creation is preferred to calling SetAttribute later, as samplers can only consider information + * already present during span creation * @param non-empty-string $key * @param bool|int|float|string|array|null $value Note: the array MUST be homogeneous, i.e. it MUST NOT contain values of different types. */ diff --git a/src/SDK/Trace/Span.php b/src/SDK/Trace/Span.php index 7e965c9a9..8d9ca09eb 100644 --- a/src/SDK/Trace/Span.php +++ b/src/SDK/Trace/Span.php @@ -9,9 +9,9 @@ use OpenTelemetry\Context\Context; use OpenTelemetry\SDK\Common\Attribute\AttributesBuilderInterface; use OpenTelemetry\SDK\Common\Dev\Compatibility\Util as BcUtil; +use OpenTelemetry\SDK\Common\Exception\StackTraceFormatter; use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeInterface; use OpenTelemetry\SDK\Common\Time\ClockFactory; -use OpenTelemetry\SDK\Common\Util\TracingUtil; use OpenTelemetry\SDK\Resource\ResourceInfo; use Throwable; diff --git a/tests/Unit/SDK/Trace/SpanTest.php b/tests/Unit/SDK/Trace/SpanTest.php index 05f73f36f..ccaac32a3 100644 --- a/tests/Unit/SDK/Trace/SpanTest.php +++ b/tests/Unit/SDK/Trace/SpanTest.php @@ -20,7 +20,6 @@ use OpenTelemetry\SDK\Common\Time\ClockFactory; use OpenTelemetry\SDK\Common\Time\ClockInterface; use OpenTelemetry\SDK\Common\Time\Util as TimeUtil; -use OpenTelemetry\SDK\Common\Util\TracingUtil; use OpenTelemetry\SDK\Resource\ResourceInfo; use OpenTelemetry\SDK\Resource\ResourceInfoFactory; use OpenTelemetry\SDK\Trace\Event; From 296a486d70c3fc3a0304502d11cec8b6d7e72044 Mon Sep 17 00:00:00 2001 From: ambkhand Date: Sat, 25 Jun 2022 12:14:43 +0530 Subject: [PATCH 05/10] removing TracingUtl class --- src/SDK/Common/Util/TracingUtil.php | 86 ----------------------------- 1 file changed, 86 deletions(-) delete mode 100644 src/SDK/Common/Util/TracingUtil.php diff --git a/src/SDK/Common/Util/TracingUtil.php b/src/SDK/Common/Util/TracingUtil.php deleted file mode 100644 index 068a555d2..000000000 --- a/src/SDK/Common/Util/TracingUtil.php +++ /dev/null @@ -1,86 +0,0 @@ -getTrace(); - $prev = $e->getPrevious(); - $result[] = sprintf('%s%s: %s', $starter, get_class($e), $e->getMessage()); - $file = $e->getFile(); - $line = $e->getLine(); - while (true) { - $current = "$file:$line"; - if (in_array($current, $seen, true)) { - $result[] = sprintf(' ... %d more', count($trace)+1); - - break; - } - - // Lambda to format traces -- we want to format the trace with '.' separators - $slashToDot = function ($str) { - return str_replace('\\', '.', $str); - }; - - $traceHasKey = array_key_exists(0, $trace); - $traceKeyHasClass = $traceHasKey && array_key_exists('class', $trace[0]); - $traceKeyHasFunction = $traceKeyHasClass && array_key_exists('function', $trace[0]); - - $result[] = sprintf( - ' at %s%s%s(%s%s%s)', - $traceKeyHasClass ? $slashToDot($trace[0]['class']) : '', - $traceKeyHasFunction ? '.' : '', - $traceKeyHasFunction ? $slashToDot($trace[0]['function']) : 'main', - $line === null ? $file : basename($file), - $line === null ? '' : ':', - $line ?? '' - ); - $seen[] = "$file:$line"; - if (!count($trace)) { - break; - } - $file = array_key_exists('file', $trace[0]) ? $trace[0]['file'] : 'Unknown Source'; - $line = array_key_exists('file', $trace[0]) && array_key_exists('line', $trace[0]) && $trace[0]['line'] ? $trace[0]['line'] : null; - array_shift($trace); - } - $result = implode("\n", $result); - if ($prev) { - $result .= "\n" . self::formatStackTrace($prev, $seen); - } - - return $result; - } -} From 90865ed9822c6283332153398842970164a8427f Mon Sep 17 00:00:00 2001 From: ambkhand Date: Mon, 27 Jun 2022 17:18:07 +0530 Subject: [PATCH 06/10] Refactor TraceState's __toString method --- src/API/Trace/TraceState.php | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/src/API/Trace/TraceState.php b/src/API/Trace/TraceState.php index 06c82f949..80cf42fca 100644 --- a/src/API/Trace/TraceState.php +++ b/src/API/Trace/TraceState.php @@ -5,8 +5,6 @@ namespace OpenTelemetry\API\Trace; use function array_reverse; -use function array_walk; -use function implode; use function strlen; class TraceState implements TraceStateInterface @@ -97,23 +95,15 @@ public function getListMemberCount(): int */ public function __toString(): string { - if (!empty($this->traceState)) { - $clonedTracestate = clone $this; - - // Reverse the order back to the original to ensure new entries are at the beginning. - $clonedTracestate->traceState = array_reverse($clonedTracestate->traceState); - - array_walk( - $clonedTracestate->traceState, - static function (&$v, $k) { - $v = $k . self::LIST_MEMBER_KEY_VALUE_SPLITTER . $v; - } - ); - - return implode(self::LIST_MEMBERS_SEPARATOR, $clonedTracestate->traceState); + if (empty($this->traceState)) { + return ''; + } + $traceStateString=''; + foreach (array_reverse($this->traceState) as $k => $v) { + $traceStateString .=$k . self::LIST_MEMBER_KEY_VALUE_SPLITTER . $v . self::LIST_MEMBERS_SEPARATOR; } - return ''; + return rtrim($traceStateString, ','); } /** From 1ba59c9e8146f45377c3a0d8ab166a963893ddc0 Mon Sep 17 00:00:00 2001 From: ambkhand Date: Tue, 5 Jul 2022 17:27:41 +0530 Subject: [PATCH 07/10] Added path with @covers to remove warnings --- tests/Unit/SDK/Trace/SpanTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Unit/SDK/Trace/SpanTest.php b/tests/Unit/SDK/Trace/SpanTest.php index ccaac32a3..072abfd5c 100644 --- a/tests/Unit/SDK/Trace/SpanTest.php +++ b/tests/Unit/SDK/Trace/SpanTest.php @@ -247,7 +247,7 @@ public function test_end_twice(): void /** * @group trace-compliance - * @covers ::isRecording + * @covers OpenTelemetry\SDK\Trace\Span::isRecording */ public function test_to_span_data_active_span(): void { @@ -446,7 +446,7 @@ public function test_get_duration_ended_span(): void /** * @group trace-compliance - * @covers ::setAttributes + * @covers OpenTelemetry\SDK\Trace\Span::setAttributes */ public function test_set_attributes(): void { @@ -486,7 +486,7 @@ public function test_set_attributes_empty(): void /** * @group trace-compliance - * @covers ::addEvent + * @covers OpenTelemetry\SDK\Trace\Span::addEvent */ public function test_add_event(): void { From 947b2de61ac8bcad16781d02c0d19906af1cc25a Mon Sep 17 00:00:00 2001 From: ambkhand Date: Wed, 13 Jul 2022 13:56:19 +0530 Subject: [PATCH 08/10] Added @coversDefaultClass annotation --- tests/Unit/SDK/Trace/SpanTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/SDK/Trace/SpanTest.php b/tests/Unit/SDK/Trace/SpanTest.php index 8a25567fc..6a92f237d 100644 --- a/tests/Unit/SDK/Trace/SpanTest.php +++ b/tests/Unit/SDK/Trace/SpanTest.php @@ -40,7 +40,7 @@ use function str_repeat; /** - * @covers OpenTelemetry\SDK\Trace\Span + * @coversDefaultClass OpenTelemetry\SDK\Trace\Span */ class SpanTest extends MockeryTestCase { From cd3e1353dd2c3bb3bfc373f97c45eae5ddd95b5b Mon Sep 17 00:00:00 2001 From: ambkhand Date: Wed, 20 Jul 2022 14:39:31 +0530 Subject: [PATCH 09/10] fix to check if iterable is empty --- src/SDK/Common/Util/functions.php | 19 +++++++++++++++++++ .../Metrics/Exporters/AbstractExporter.php | 3 ++- src/SDK/Trace/Behavior/SpanExporterTrait.php | 3 ++- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/SDK/Common/Util/functions.php b/src/SDK/Common/Util/functions.php index eff0328e1..e74666e5c 100644 --- a/src/SDK/Common/Util/functions.php +++ b/src/SDK/Common/Util/functions.php @@ -5,6 +5,7 @@ namespace OpenTelemetry\SDK\Common\Util; use Closure; +use Exception; use function get_class; use ReflectionFunction; use stdClass; @@ -50,3 +51,21 @@ function weaken(Closure $closure, ?object &$target = null): Closure ? static fn (...$args) => ($obj = $ref->get()) ? $closure->call($obj, ...$args) : null : static fn (...$args) => ($obj = $ref->get()) ? $closure->bindTo($obj)(...$args) : null; } + +/** + * Returns whether an iterable is empty or not + */ +function isEmpty(iterable $list) +{ + try { + foreach ($list as $value) { + break; + } + } + //catch exception + catch (Exception $e) { + return true; + } + + return false; +} diff --git a/src/SDK/Metrics/Exporters/AbstractExporter.php b/src/SDK/Metrics/Exporters/AbstractExporter.php index 2897a4b59..8b777a516 100644 --- a/src/SDK/Metrics/Exporters/AbstractExporter.php +++ b/src/SDK/Metrics/Exporters/AbstractExporter.php @@ -7,6 +7,7 @@ use Exception; use InvalidArgumentException; use OpenTelemetry\API\Metrics as API; +use function OpenTelemetry\SDK\Common\Util\isEmpty; use OpenTelemetry\SDK\Metrics\Exceptions\RetryableExportException; abstract class AbstractExporter implements API\ExporterInterface @@ -16,7 +17,7 @@ abstract class AbstractExporter implements API\ExporterInterface */ public function export(iterable $metrics): int { - if (empty($metrics)) { + if (isEmpty($metrics)) { return API\ExporterInterface::SUCCESS; } diff --git a/src/SDK/Trace/Behavior/SpanExporterTrait.php b/src/SDK/Trace/Behavior/SpanExporterTrait.php index 226469a2d..641ff6fe9 100644 --- a/src/SDK/Trace/Behavior/SpanExporterTrait.php +++ b/src/SDK/Trace/Behavior/SpanExporterTrait.php @@ -4,6 +4,7 @@ namespace OpenTelemetry\SDK\Trace\Behavior; +use function OpenTelemetry\SDK\Common\Util\isEmpty; use OpenTelemetry\SDK\Trace\SpanDataInterface; use OpenTelemetry\SDK\Trace\SpanExporterInterface; @@ -40,7 +41,7 @@ public function export(iterable $spans): int return SpanExporterInterface::STATUS_FAILED_NOT_RETRYABLE; } - if (empty($spans)) { + if (isEmpty($spans)) { return SpanExporterInterface::STATUS_SUCCESS; } From d0b2dc5c3d7f6729d9d93c7c9d6f9ab878b90c43 Mon Sep 17 00:00:00 2001 From: ambkhand Date: Wed, 20 Jul 2022 15:05:12 +0530 Subject: [PATCH 10/10] fixed for faling test case --- src/SDK/Common/Util/functions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SDK/Common/Util/functions.php b/src/SDK/Common/Util/functions.php index e74666e5c..4120dada1 100644 --- a/src/SDK/Common/Util/functions.php +++ b/src/SDK/Common/Util/functions.php @@ -67,5 +67,5 @@ function isEmpty(iterable $list) return true; } - return false; + return empty($list); }