Skip to content

Commit

Permalink
revert pass-by-reference to LogRecordProcessor::onEmit
Browse files Browse the repository at this point in the history
add a test to demonstrate that mutations are still seen by later processors without by-reference
  • Loading branch information
brettmc committed Jan 28, 2025
1 parent 27dfc42 commit 25498e7
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/SDK/Logs/LogRecordProcessorInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

interface LogRecordProcessorInterface
{
public function onEmit(ReadWriteLogRecord &$record, ?ContextInterface $context = null): void;
public function onEmit(ReadWriteLogRecord $record, ?ContextInterface $context = null): void;
public function shutdown(?CancellationInterface $cancellation = null): bool;
public function forceFlush(?CancellationInterface $cancellation = null): bool;
}
2 changes: 1 addition & 1 deletion src/SDK/Logs/Processor/BatchLogRecordProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public function __construct(
});
}

public function onEmit(ReadWriteLogRecord &$record, ?ContextInterface $context = null): void
public function onEmit(ReadWriteLogRecord $record, ?ContextInterface $context = null): void
{
if ($this->closed) {
return;
Expand Down
2 changes: 1 addition & 1 deletion src/SDK/Logs/Processor/MultiLogRecordProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public function __construct(array $processors)
}
}

public function onEmit(ReadWriteLogRecord &$record, ?ContextInterface $context = null): void
public function onEmit(ReadWriteLogRecord $record, ?ContextInterface $context = null): void
{
foreach ($this->processors as $processor) {
$processor->onEmit($record, $context);
Expand Down
2 changes: 1 addition & 1 deletion src/SDK/Logs/Processor/NoopLogRecordProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public static function getInstance(): self
/**
* @codeCoverageIgnore
*/
public function onEmit(ReadWriteLogRecord &$record, ?ContextInterface $context = null): void
public function onEmit(ReadWriteLogRecord $record, ?ContextInterface $context = null): void
{
}

Expand Down
2 changes: 1 addition & 1 deletion src/SDK/Logs/Processor/SimpleLogRecordProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public function __construct(private readonly LogRecordExporterInterface $exporte
/**
* @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/sdk.md#onemit
*/
public function onEmit(ReadWriteLogRecord &$record, ?ContextInterface $context = null): void
public function onEmit(ReadWriteLogRecord $record, ?ContextInterface $context = null): void
{
$this->exporter->export([$record]);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/Integration/SDK/Logs/LoggerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public function __construct(private readonly InMemoryExporter $exporter)
{
}

public function onEmit(ReadWriteLogRecord &$record, ?ContextInterface $context = null): void
public function onEmit(ReadWriteLogRecord $record, ?ContextInterface $context = null): void
{
$record->setAttributes(['baz' => 'bat']);
$this->exporter->export([$record]);
Expand Down
80 changes: 80 additions & 0 deletions tests/Integration/SDK/Logs/test_mutate_log_record_in_onemit.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
--TEST--
Test mutating a LogRecord during SpanProcessor::onEmit
--FILE--
<?php
require_once 'vendor/autoload.php';

use OpenTelemetry\SDK\Trace\TracerProviderBuilder;
use OpenTelemetry\SDK\Trace\ReadWriteSpanInterface;
use OpenTelemetry\SDK\Trace\ReadableSpanInterface;
use OpenTelemetry\SDK\Common\Future\CancellationInterface;
use OpenTelemetry\SDK\Trace\SpanProcessorInterface;
use OpenTelemetry\Context\ContextInterface;

$exporter = (new \OpenTelemetry\SDK\Logs\Exporter\ConsoleExporterFactory())->create();

$one = new class implements \OpenTelemetry\SDK\Logs\LogRecordProcessorInterface {
public function onEmit(\OpenTelemetry\SDK\Logs\ReadWriteLogRecord $record, ?ContextInterface $context = null): void
{
$record->setBody('updated');
$record->setAttribute('new-key', 'new-value');
}

public function shutdown(?CancellationInterface $cancellation = null): bool
{
return true;
}

public function forceFlush(?CancellationInterface $cancellation = null): bool
{
return true;
}
};
$two = new \OpenTelemetry\SDK\Logs\Processor\SimpleLogRecordProcessor($exporter);

$loggerProvider = (new \OpenTelemetry\SDK\Logs\LoggerProviderBuilder())
->addLogRecordProcessor(new \OpenTelemetry\SDK\Logs\Processor\MultiLogRecordProcessor([$one, $two]))
->build();

$logger = $loggerProvider->getLogger('test');
$logger->emit(new \OpenTelemetry\API\Logs\LogRecord('body'));
?>
--EXPECTF--
{
"resource": {
"attributes": {
"telemetry.sdk.name": "opentelemetry",
"telemetry.sdk.language": "php",
"telemetry.sdk.version": "%s",
"telemetry.distro.name": "%s",
"telemetry.distro.version": "%s",
"service.name": "%s"
},
"dropped_attributes_count": 0
},
"scopes": [
{
"name": "test",
"version": null,
"attributes": [],
"dropped_attributes_count": 0,
"schema_url": null,
"logs": [
{
"timestamp": null,
"observed_timestamp": %d,
"severity_number": 0,
"severity_text": null,
"body": "updated",
"trace_id": "%s",
"span_id": "%s",
"trace_flags": 0,
"attributes": {
"new-key": "new-value"
},
"dropped_attributes_count": 0
}
]
}
]
}

0 comments on commit 25498e7

Please sign in to comment.