Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apply logging spec updates #977

Merged
merged 1 commit into from
Apr 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/logs/features/monolog-otel-integration.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions src/API/Common/Instrumentation/CachedInstrumentation.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
2 changes: 1 addition & 1 deletion src/API/Logs/EventLogger.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
2 changes: 1 addition & 1 deletion src/API/Logs/LoggerInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@

interface LoggerInterface
{
public function logRecord(LogRecord $logRecord): void;
public function emit(LogRecord $logRecord): void;
}
1 change: 0 additions & 1 deletion src/API/Logs/LoggerProviderInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ public function getLogger(
string $name,
?string $version = null,
?string $schemaUrl = null,
bool $includeTraceContext = true,
iterable $attributes = [] //instrumentation scope attributes
): LoggerInterface;
}
2 changes: 1 addition & 1 deletion src/API/Logs/NoopLogger.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public static function getInstance(): self
/**
* @codeCoverageIgnore
*/
public function logRecord(LogRecord $logRecord): void
public function emit(LogRecord $logRecord): void
{
}

Expand Down
2 changes: 1 addition & 1 deletion src/API/Logs/NoopLoggerProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
8 changes: 3 additions & 5 deletions src/SDK/Logs/Logger.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/SDK/Logs/LoggerProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/SDK/Logs/NoopLoggerProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
11 changes: 5 additions & 6 deletions src/SDK/Logs/ReadableLogRecord.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it not part of the spec?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was removed in the last couple of days. The changes in this PR are referenced in open-telemetry/opentelemetry-specification#2911

$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;

Expand Down
2 changes: 1 addition & 1 deletion tests/Unit/API/Logs/EventLoggerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
1 change: 0 additions & 1 deletion tests/Unit/SDK/Logs/Exporter/ConsoleExporterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ public function test_export(): void
$this->createMock(InstrumentationScopeInterface::class),
$this->createMock(LoggerSharedState::class),
(new LogRecord('foo')),
true,
)),
];

Expand Down
24 changes: 22 additions & 2 deletions tests/Unit/SDK/Logs/LoggerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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);
}
}
2 changes: 1 addition & 1 deletion tests/Unit/SDK/Logs/ReadableLogRecordTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down