From 12ed8fe470282c234b5365e9673e7df2ad5788af Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 18 Apr 2023 13:58:19 +1000 Subject: [PATCH] apply logging spec updates Removing include_trace_context parameter Set ObservedTimestamp on emit if not already set Rename logRecord to emit --- .../features/monolog-otel-integration.php | 2 +- .../Instrumentation/CachedInstrumentation.php | 5 ++-- src/API/Logs/EventLogger.php | 2 +- src/API/Logs/LoggerInterface.php | 2 +- src/API/Logs/LoggerProviderInterface.php | 1 - src/API/Logs/NoopLogger.php | 2 +- src/API/Logs/NoopLoggerProvider.php | 2 +- src/SDK/Logs/Logger.php | 8 +++---- src/SDK/Logs/LoggerProvider.php | 4 ++-- src/SDK/Logs/NoopLoggerProvider.php | 2 +- src/SDK/Logs/ReadableLogRecord.php | 11 ++++----- tests/Unit/API/Logs/EventLoggerTest.php | 2 +- .../SDK/Logs/Exporter/ConsoleExporterTest.php | 1 - tests/Unit/SDK/Logs/LoggerTest.php | 24 +++++++++++++++++-- tests/Unit/SDK/Logs/ReadableLogRecordTest.php | 2 +- 15 files changed, 42 insertions(+), 28 deletions(-) diff --git a/examples/logs/features/monolog-otel-integration.php b/examples/logs/features/monolog-otel-integration.php index 33b3e9fe0..5852e155e 100644 --- a/examples/logs/features/monolog-otel-integration.php +++ b/examples/logs/features/monolog-otel-integration.php @@ -45,7 +45,7 @@ public function __construct(string $level, bool $bubble = true, ?LoggerProviderI protected function write(array $record): void { - $this->logger->logRecord($this->convert($record)); + $this->logger->emit($this->convert($record)); } private function convert(array $record): LogRecord diff --git a/src/API/Common/Instrumentation/CachedInstrumentation.php b/src/API/Common/Instrumentation/CachedInstrumentation.php index 6cba1a4b3..c7969511f 100644 --- a/src/API/Common/Instrumentation/CachedInstrumentation.php +++ b/src/API/Common/Instrumentation/CachedInstrumentation.php @@ -88,10 +88,9 @@ public function logger(): LoggerInterface $loggerProvider = Globals::loggerProvider(); if ($this->loggers === null) { - //@todo configurable includeTraceContext? - return $loggerProvider->getLogger($this->name, $this->version, $this->schemaUrl, true, $this->attributes); + return $loggerProvider->getLogger($this->name, $this->version, $this->schemaUrl, $this->attributes); } - return $this->loggers[$loggerProvider] ??= $loggerProvider->getLogger($this->name, $this->version, $this->schemaUrl, true, $this->attributes); + return $this->loggers[$loggerProvider] ??= $loggerProvider->getLogger($this->name, $this->version, $this->schemaUrl, $this->attributes); } } diff --git a/src/API/Logs/EventLogger.php b/src/API/Logs/EventLogger.php index 75bc6ed4a..68deb865c 100644 --- a/src/API/Logs/EventLogger.php +++ b/src/API/Logs/EventLogger.php @@ -21,6 +21,6 @@ public function logEvent(string $eventName, LogRecord $logRecord): void 'event.name' => $eventName, 'event.domain' => $this->domain, ]); - $this->logger->logRecord($logRecord); + $this->logger->emit($logRecord); } } diff --git a/src/API/Logs/LoggerInterface.php b/src/API/Logs/LoggerInterface.php index ab89a8cec..89477c8d2 100644 --- a/src/API/Logs/LoggerInterface.php +++ b/src/API/Logs/LoggerInterface.php @@ -6,5 +6,5 @@ interface LoggerInterface { - public function logRecord(LogRecord $logRecord): void; + public function emit(LogRecord $logRecord): void; } diff --git a/src/API/Logs/LoggerProviderInterface.php b/src/API/Logs/LoggerProviderInterface.php index 2222a900d..e60353de2 100644 --- a/src/API/Logs/LoggerProviderInterface.php +++ b/src/API/Logs/LoggerProviderInterface.php @@ -13,7 +13,6 @@ public function getLogger( string $name, ?string $version = null, ?string $schemaUrl = null, - bool $includeTraceContext = true, iterable $attributes = [] //instrumentation scope attributes ): LoggerInterface; } diff --git a/src/API/Logs/NoopLogger.php b/src/API/Logs/NoopLogger.php index 0062a5884..faacd5e10 100644 --- a/src/API/Logs/NoopLogger.php +++ b/src/API/Logs/NoopLogger.php @@ -20,7 +20,7 @@ public static function getInstance(): self /** * @codeCoverageIgnore */ - public function logRecord(LogRecord $logRecord): void + public function emit(LogRecord $logRecord): void { } diff --git a/src/API/Logs/NoopLoggerProvider.php b/src/API/Logs/NoopLoggerProvider.php index bb0c5c6de..8b00b6637 100644 --- a/src/API/Logs/NoopLoggerProvider.php +++ b/src/API/Logs/NoopLoggerProvider.php @@ -13,7 +13,7 @@ public static function getInstance(): self return $instance ??= new self(); } - public function getLogger(string $name, ?string $version = null, ?string $schemaUrl = null, bool $includeTraceContext = true, iterable $attributes = []): LoggerInterface + public function getLogger(string $name, ?string $version = null, ?string $schemaUrl = null, iterable $attributes = []): LoggerInterface { return NoopLogger::getInstance(); } diff --git a/src/SDK/Logs/Logger.php b/src/SDK/Logs/Logger.php index 9058a7f26..0b8db152d 100644 --- a/src/SDK/Logs/Logger.php +++ b/src/SDK/Logs/Logger.php @@ -18,18 +18,16 @@ class Logger implements LoggerInterface { private InstrumentationScopeInterface $scope; private LoggerSharedState $loggerSharedState; - private bool $includeTraceContext; - public function __construct(LoggerSharedState $loggerSharedState, InstrumentationScopeInterface $scope, bool $includeTraceContext) + public function __construct(LoggerSharedState $loggerSharedState, InstrumentationScopeInterface $scope) { $this->loggerSharedState = $loggerSharedState; $this->scope = $scope; - $this->includeTraceContext = $includeTraceContext; } - public function logRecord(LogRecord $logRecord): void + public function emit(LogRecord $logRecord): void { - $readWriteLogRecord = new ReadWriteLogRecord($this->scope, $this->loggerSharedState, $logRecord, $this->includeTraceContext); + $readWriteLogRecord = new ReadWriteLogRecord($this->scope, $this->loggerSharedState, $logRecord); // @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/sdk.md#onemit $this->loggerSharedState->getProcessor()->onEmit( $readWriteLogRecord, diff --git a/src/SDK/Logs/LoggerProvider.php b/src/SDK/Logs/LoggerProvider.php index a4c2023ec..f0a8266c1 100644 --- a/src/SDK/Logs/LoggerProvider.php +++ b/src/SDK/Logs/LoggerProvider.php @@ -29,14 +29,14 @@ public function __construct(LogRecordProcessorInterface $processor, Instrumentat /** * @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/sdk.md#logger-creation */ - public function getLogger(string $name, ?string $version = null, ?string $schemaUrl = null, bool $includeTraceContext = true, iterable $attributes = []): LoggerInterface + public function getLogger(string $name, ?string $version = null, ?string $schemaUrl = null, iterable $attributes = []): LoggerInterface { if ($this->loggerSharedState->hasShutdown()) { return NoopLogger::getInstance(); } $scope = $this->instrumentationScopeFactory->create($name, $version, $schemaUrl, $attributes); - return new Logger($this->loggerSharedState, $scope, $includeTraceContext); + return new Logger($this->loggerSharedState, $scope); } public function shutdown(CancellationInterface $cancellation = null): bool diff --git a/src/SDK/Logs/NoopLoggerProvider.php b/src/SDK/Logs/NoopLoggerProvider.php index 724b40bba..819e02ee5 100644 --- a/src/SDK/Logs/NoopLoggerProvider.php +++ b/src/SDK/Logs/NoopLoggerProvider.php @@ -16,7 +16,7 @@ public static function getInstance(): self return $instance ??= new self(); } - public function getLogger(string $name, ?string $version = null, ?string $schemaUrl = null, bool $includeTraceContext = true, iterable $attributes = []): LoggerInterface + public function getLogger(string $name, ?string $version = null, ?string $schemaUrl = null, iterable $attributes = []): LoggerInterface { return NoopLogger::getInstance(); } diff --git a/src/SDK/Logs/ReadableLogRecord.php b/src/SDK/Logs/ReadableLogRecord.php index 04e116bef..74666b30d 100644 --- a/src/SDK/Logs/ReadableLogRecord.php +++ b/src/SDK/Logs/ReadableLogRecord.php @@ -24,19 +24,18 @@ class ReadableLogRecord extends LogRecord protected AttributesInterface $convertedAttributes; protected SpanContextInterface $spanContext; - public function __construct(InstrumentationScopeInterface $scope, LoggerSharedState $loggerSharedState, LogRecord $logRecord, bool $includeTraceContext) + public function __construct(InstrumentationScopeInterface $scope, LoggerSharedState $loggerSharedState, LogRecord $logRecord) { $this->scope = $scope; $this->loggerSharedState = $loggerSharedState; parent::__construct($logRecord->body); $this->timestamp = $logRecord->timestamp; - $this->observedTimestamp = $logRecord->observedTimestamp; + $this->observedTimestamp = $logRecord->observedTimestamp + ?? (int) (microtime(true) * LogRecord::NANOS_PER_SECOND); $this->context = $logRecord->context; - if ($includeTraceContext) { - $context = $this->context ?? Context::getCurrent(); - $this->spanContext = Span::fromContext($context)->getContext(); - }; + $context = $this->context ?? Context::getCurrent(); + $this->spanContext = Span::fromContext($context)->getContext(); $this->severityNumber = $logRecord->severityNumber; $this->severityText = $logRecord->severityText; diff --git a/tests/Unit/API/Logs/EventLoggerTest.php b/tests/Unit/API/Logs/EventLoggerTest.php index 9227b674e..f06625fc2 100644 --- a/tests/Unit/API/Logs/EventLoggerTest.php +++ b/tests/Unit/API/Logs/EventLoggerTest.php @@ -21,7 +21,7 @@ public function test_log_event(): void $logRecord = $this->createMock(LogRecord::class); $eventLogger = new EventLogger($logger, $domain); $logRecord->expects($this->once())->method('setAttributes'); - $logger->expects($this->once())->method('logRecord')->with($this->equalTo($logRecord)); + $logger->expects($this->once())->method('emit')->with($this->equalTo($logRecord)); $eventLogger->logEvent('some.event', $logRecord); } diff --git a/tests/Unit/SDK/Logs/Exporter/ConsoleExporterTest.php b/tests/Unit/SDK/Logs/Exporter/ConsoleExporterTest.php index c97913ffd..0c580dc0e 100644 --- a/tests/Unit/SDK/Logs/Exporter/ConsoleExporterTest.php +++ b/tests/Unit/SDK/Logs/Exporter/ConsoleExporterTest.php @@ -34,7 +34,6 @@ public function test_export(): void $this->createMock(InstrumentationScopeInterface::class), $this->createMock(LoggerSharedState::class), (new LogRecord('foo')), - true, )), ]; diff --git a/tests/Unit/SDK/Logs/LoggerTest.php b/tests/Unit/SDK/Logs/LoggerTest.php index 08b57601b..50be2f38b 100644 --- a/tests/Unit/SDK/Logs/LoggerTest.php +++ b/tests/Unit/SDK/Logs/LoggerTest.php @@ -34,7 +34,7 @@ public function setUp(): void public function test_log_record(): void { - $logger = new Logger($this->sharedState, $this->scope, true); + $logger = new Logger($this->sharedState, $this->scope); $record = (new LogRecord())->setContext($this->createMock(ContextInterface::class)); $this->processor->expects($this->once())->method('onEmit') @@ -43,6 +43,26 @@ public function test_log_record(): void $this->isInstanceOf(ContextInterface::class) ); - $logger->logRecord($record); + $logger->emit($record); + } + + public function test_sets_observed_timestamp_on_emit(): void + { + $logger = new Logger($this->sharedState, $this->scope); + $record = new LogRecord(); + $time = microtime(true) * LogRecord::NANOS_PER_SECOND; + + $this->processor->expects($this->once())->method('onEmit') + ->with( + $this->callback(function (ReadWriteLogRecord $record) use ($time) { + $this->assertNotNull($record->getObservedTimestamp()); + $this->assertGreaterThan($time, $record->getObservedTimestamp()); + + return true; + }), + $this->anything(), + ); + + $logger->emit($record); } } diff --git a/tests/Unit/SDK/Logs/ReadableLogRecordTest.php b/tests/Unit/SDK/Logs/ReadableLogRecordTest.php index e5c50873d..ac409a377 100644 --- a/tests/Unit/SDK/Logs/ReadableLogRecordTest.php +++ b/tests/Unit/SDK/Logs/ReadableLogRecordTest.php @@ -37,7 +37,7 @@ public function test_getters(): void ->setObservedTimestamp(22) ->setAttributes(['foo' => 'bar']) ->setContext($context); - $record = new ReadableLogRecord($scope, $sharedState, $logRecord, true); + $record = new ReadableLogRecord($scope, $sharedState, $logRecord); $this->assertSame($scope, $record->getInstrumentationScope()); $this->assertSame($resource, $record->getResource());