Skip to content

Commit 0c38ff9

Browse files
committed
fix: guard against a mixed buffer
1 parent c395db8 commit 0c38ff9

File tree

2 files changed

+123
-12
lines changed

2 files changed

+123
-12
lines changed

src/Utils/Test/src/TraceStructureAssertionTrait.php

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -56,23 +56,29 @@ public function assertTraceStructure($spans, array $expectedStructure, bool $str
5656
}
5757

5858
/**
59-
* Converts spans to an array if they are in a different format.
59+
* Converts spans to an array if they are in a different format and filters out non-span items.
6060
*
61+
* @psalm-suppress UnusedVariable
6162
* @param array|ArrayObject|Traversable $spans
6263
* @return array
6364
*/
6465
private function convertSpansToArray($spans): array
6566
{
66-
if (is_array($spans)) {
67-
return $spans;
68-
}
67+
$array = [];
6968

70-
if ($spans instanceof ArrayObject || $spans instanceof Traversable) {
71-
return iterator_to_array($spans);
69+
if (is_array($spans)) {
70+
$array = $spans;
71+
} elseif ($spans instanceof ArrayObject || $spans instanceof Traversable) {
72+
$array = iterator_to_array($spans);
73+
} else {
74+
/** @phpstan-ignore deadCode.unreachable */
75+
throw new \InvalidArgumentException('Spans must be an array, ArrayObject, or Traversable');
7276
}
7377

74-
/** @phpstan-ignore deadCode.unreachable */
75-
throw new \InvalidArgumentException('Spans must be an array, ArrayObject, or Traversable');
78+
// Filter out non-span items
79+
return array_filter($array, function ($item) {
80+
return $item instanceof ImmutableSpan;
81+
});
7682
}
7783

7884
/**
@@ -86,10 +92,7 @@ private function buildSpanMap(array $spans): array
8692
$spanMap = [];
8793

8894
foreach ($spans as $span) {
89-
if (!$span instanceof ImmutableSpan) {
90-
throw new \InvalidArgumentException('Each span must be an instance of ImmutableSpan');
91-
}
92-
95+
// All non-span items should have been filtered out in convertSpansToArray
9396
$spanMap[$span->getSpanId()] = [
9497
'span' => $span,
9598
'children' => [],
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace OpenTelemetry\TestUtils\Tests\Unit;
6+
7+
use ArrayObject;
8+
use OpenTelemetry\API\Logs\LogRecord;
9+
use OpenTelemetry\API\Trace\SpanKind;
10+
use OpenTelemetry\SDK\Trace\ImmutableSpan;
11+
use OpenTelemetry\SDK\Trace\SpanExporter\InMemoryExporter;
12+
use OpenTelemetry\SDK\Trace\SpanProcessor\SimpleSpanProcessor;
13+
use OpenTelemetry\SDK\Trace\TracerProvider;
14+
use OpenTelemetry\TestUtils\TraceStructureAssertionTrait;
15+
use PHPUnit\Framework\TestCase;
16+
17+
/**
18+
* Test to demonstrate that TraceStructureAssertionTrait only works with spans.
19+
*/
20+
class MixedBufferTest extends TestCase
21+
{
22+
use TraceStructureAssertionTrait;
23+
24+
private ArrayObject $sharedBuffer;
25+
private TracerProvider $tracerProvider;
26+
27+
protected function setUp(): void
28+
{
29+
// Create a shared buffer for both spans and logrecords
30+
$this->sharedBuffer = new ArrayObject();
31+
32+
// Create a TracerProvider with an InMemoryExporter using the shared buffer
33+
$this->tracerProvider = new TracerProvider(
34+
new SimpleSpanProcessor(
35+
new InMemoryExporter($this->sharedBuffer)
36+
)
37+
);
38+
}
39+
40+
/**
41+
* Test that demonstrates the trait now automatically filters out logrecords.
42+
*/
43+
public function test_trait_automatically_filters_logrecords(): void
44+
{
45+
// Create a span
46+
$tracer = $this->tracerProvider->getTracer('test-tracer');
47+
$span = $tracer->spanBuilder('test-span')
48+
->setSpanKind(SpanKind::KIND_SERVER)
49+
->startSpan();
50+
$span->setAttribute('attribute.one', 'value1');
51+
$span->end();
52+
53+
// Manually add a logrecord to the shared buffer
54+
$logRecord = new LogRecord('Test log message');
55+
$this->sharedBuffer->append($logRecord);
56+
57+
// Define the expected structure
58+
$expectedStructure = [
59+
[
60+
'name' => 'test-span',
61+
'kind' => SpanKind::KIND_SERVER,
62+
'attributes' => [
63+
'attribute.one' => 'value1',
64+
],
65+
],
66+
];
67+
68+
// This should now pass because the trait automatically filters out logrecords
69+
$this->assertTraceStructure($this->sharedBuffer, $expectedStructure);
70+
}
71+
72+
/**
73+
* Test that demonstrates the manual solution to filter out logrecords still works.
74+
*/
75+
public function test_manual_filtering_of_logrecords_still_works(): void
76+
{
77+
// Create a span
78+
$tracer = $this->tracerProvider->getTracer('test-tracer');
79+
$span = $tracer->spanBuilder('test-span')
80+
->setSpanKind(SpanKind::KIND_SERVER)
81+
->startSpan();
82+
$span->setAttribute('attribute.one', 'value1');
83+
$span->end();
84+
85+
// Manually add a logrecord to the shared buffer
86+
$logRecord = new LogRecord('Test log message');
87+
$this->sharedBuffer->append($logRecord);
88+
89+
// Define the expected structure
90+
$expectedStructure = [
91+
[
92+
'name' => 'test-span',
93+
'kind' => SpanKind::KIND_SERVER,
94+
'attributes' => [
95+
'attribute.one' => 'value1',
96+
],
97+
],
98+
];
99+
100+
// Filter the buffer to only include spans
101+
$spansOnly = array_filter(iterator_to_array($this->sharedBuffer), function ($item) {
102+
return $item instanceof ImmutableSpan;
103+
});
104+
105+
// This should pass because we've filtered out the logrecords
106+
$this->assertTraceStructure($spansOnly, $expectedStructure);
107+
}
108+
}

0 commit comments

Comments
 (0)