Skip to content

Commit a8a1645

Browse files
Fix/shutdown (#21)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 406041f commit a8a1645

7 files changed

Lines changed: 70 additions & 16 deletions

File tree

src/Factory/SDKBuilder.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public function build(): void
3535
->setMeterProvider($meterProvider)
3636
->setLoggerProvider($loggerProvider)
3737
->setPropagator(TraceContextPropagator::getInstance())
38-
->setAutoShutdown(true)
38+
->setAutoShutdown(false)
3939
->buildAndRegisterGlobal();
4040
}
4141
}

src/Listener/OtelShutdownListener.php

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace Hyperf\OpenTelemetry\Listener;
66

77
use Hyperf\Contract\StdoutLoggerInterface;
8+
use Hyperf\Coroutine\Coroutine;
89
use Hyperf\Event\Contract\ListenerInterface;
910
use Hyperf\Framework\Event\OnWorkerExit;
1011
use OpenTelemetry\SDK\Metrics\MeterProviderInterface;
@@ -29,16 +30,18 @@ public function listen(): array
2930

3031
public function process(object $event): void
3132
{
32-
try {
33-
$this->tracerProvider->shutdown();
34-
} catch (Throwable $e) {
35-
$this->logger->warning(sprintf('[OTel] tracer shutdown failed: %s', $e->getMessage()));
36-
}
33+
Coroutine::create(function () {
34+
try {
35+
$this->tracerProvider->shutdown();
36+
} catch (Throwable $e) {
37+
$this->logger->warning(sprintf('[OTel] tracer shutdown failed: %s', $e->getMessage()));
38+
}
3739

38-
try {
39-
$this->meterProvider->shutdown();
40-
} catch (Throwable $e) {
41-
$this->logger->warning(sprintf('[OTel] meter shutdown failed: %s', $e->getMessage()));
42-
}
40+
try {
41+
$this->meterProvider->shutdown();
42+
} catch (Throwable $e) {
43+
$this->logger->warning(sprintf('[OTel] meter shutdown failed: %s', $e->getMessage()));
44+
}
45+
});
4346
}
4447
}

src/Support/Uri.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ class Uri
88
{
99
public static function sanitize(string $uri, array $uriMask = []): string
1010
{
11-
$uuid = '/\/(?<=\/)([0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12})(?=\/|$)/i';
11+
$uuid = '/\/(?<=\/)([0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12})(?=\/|$)/i';
1212

1313
$defaultPatterns = [
1414
$uuid => '/{uuid}',

src/Transport/SwooleGrpcTransportFactory.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ public function create(
3737
if ($compression !== null) {
3838
$compressionType = is_array($compression) ? ($compression[0] ?? null) : $compression;
3939
}
40+
if ($compressionType === 'none') {
41+
$compressionType = null;
42+
}
4043

4144
return new SwooleGrpcTransport(
4245
host: $parsed['host'],

tests/Unit/Listener/OtelShutdownListenerTest.php

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,28 +29,51 @@ public function testImplementsListenerInterface(): void
2929
$this->assertInstanceOf(ListenerInterface::class, $listener);
3030
}
3131

32-
public function testProcessCallsShutdown(): void
32+
public function testListensToOnWorkerExit(): void
33+
{
34+
$listener = new OtelShutdownListener(
35+
$this->createMock(MeterProviderInterface::class),
36+
$this->createMock(TracerProviderInterface::class),
37+
$this->createMock(StdoutLoggerInterface::class)
38+
);
39+
$this->assertSame([OnWorkerExit::class], $listener->listen());
40+
}
41+
42+
public function testProcessCallsShutdownInsideCoroutine(): void
3343
{
3444
$meterProvider = $this->createMock(MeterProviderInterface::class);
3545
$tracerProvider = $this->createMock(TracerProviderInterface::class);
3646
$logger = $this->createMock(StdoutLoggerInterface::class);
3747

38-
$meterProvider->expects($this->once())->method('shutdown');
3948
$tracerProvider->expects($this->once())->method('shutdown');
49+
$meterProvider->expects($this->once())->method('shutdown');
4050
$logger->expects($this->never())->method('warning');
4151

4252
$listener = new OtelShutdownListener($meterProvider, $tracerProvider, $logger);
4353
$listener->process(new OnWorkerExit($this->createMock(Server::class), 0));
4454
}
4555

46-
public function testProcessLogsWarningOnException(): void
56+
public function testProcessLogsWarningOnTracerException(): void
57+
{
58+
$meterProvider = $this->createMock(MeterProviderInterface::class);
59+
$tracerProvider = $this->createMock(TracerProviderInterface::class);
60+
$logger = $this->createMock(StdoutLoggerInterface::class);
61+
62+
$tracerProvider->method('shutdown')->willThrowException(new RuntimeException('tracer error'));
63+
$logger->expects($this->atLeastOnce())->method('warning')->with($this->stringContains('tracer'));
64+
65+
$listener = new OtelShutdownListener($meterProvider, $tracerProvider, $logger);
66+
$listener->process(new OnWorkerExit($this->createMock(Server::class), 0));
67+
}
68+
69+
public function testProcessLogsWarningOnMeterException(): void
4770
{
4871
$meterProvider = $this->createMock(MeterProviderInterface::class);
4972
$tracerProvider = $this->createMock(TracerProviderInterface::class);
5073
$logger = $this->createMock(StdoutLoggerInterface::class);
5174

5275
$meterProvider->method('shutdown')->willThrowException(new RuntimeException('meter error'));
53-
$logger->expects($this->once())->method('warning')->with($this->stringContains('meter error'));
76+
$logger->expects($this->atLeastOnce())->method('warning')->with($this->stringContains('meter'));
5477

5578
$listener = new OtelShutdownListener($meterProvider, $tracerProvider, $logger);
5679
$listener->process(new OnWorkerExit($this->createMock(Server::class), 0));

tests/Unit/Support/UriTest.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@ public function testSanitizeUuid()
3333
$this->assertStringContainsString('/{uuid}', $result);
3434
}
3535

36+
public function testSanitizeUuidV7()
37+
{
38+
$uri = '/019363e0-7be6-7f00-8a7d-d6fb379a33a1/';
39+
$result = Uri::sanitize($uri);
40+
$this->assertStringContainsString('/{uuid}', $result);
41+
}
42+
3643
public function testSanitizeLicensePlate()
3744
{
3845
$uri = '/ABC1A23/';

tests/Unit/Transport/SwooleGrpcTransportFactoryTest.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,4 +100,22 @@ public function testCreateWithGrpcsScheme(): void
100100

101101
$this->assertSame(ContentTypes::PROTOBUF, $transport->contentType());
102102
}
103+
104+
public function testCreateWithCompressionNoneTreatedAsNull(): void
105+
{
106+
$factory = new SwooleGrpcTransportFactory();
107+
108+
$transport = $factory->create(
109+
endpoint: 'http://localhost:4317/opentelemetry.proto.collector.trace.v1.TraceService/Export',
110+
contentType: ContentTypes::PROTOBUF,
111+
compression: 'none',
112+
);
113+
114+
$this->assertSame(ContentTypes::PROTOBUF, $transport->contentType());
115+
116+
// Use reflection to verify compression was normalized to null
117+
$reflection = new \ReflectionClass($transport);
118+
$compressionProp = $reflection->getProperty('compression');
119+
$this->assertNull($compressionProp->getValue($transport));
120+
}
103121
}

0 commit comments

Comments
 (0)