From a48cde0cb09a258f77e4967d787e5f49a69eb05f Mon Sep 17 00:00:00 2001 From: Alexander Pekhota Date: Mon, 27 Sep 2021 17:19:18 +0300 Subject: [PATCH] Fix for sub-span overflow --- .../JaegerHttpReporterFactory.php | 1 + .../ReporterFactory/JaegerReporterFactory.php | 1 + src/Jaeger/Sender/JaegerSender.php | 65 +++++++++++++++++++ .../Jaeger/Sender/JaegerThriftSenderTest.php | 41 ++++++++++++ 4 files changed, 108 insertions(+) diff --git a/src/Jaeger/ReporterFactory/JaegerHttpReporterFactory.php b/src/Jaeger/ReporterFactory/JaegerHttpReporterFactory.php index 6694aaa..038e811 100644 --- a/src/Jaeger/ReporterFactory/JaegerHttpReporterFactory.php +++ b/src/Jaeger/ReporterFactory/JaegerHttpReporterFactory.php @@ -29,6 +29,7 @@ public function createReporter() : ReporterInterface $client = new HttpAgentClient($protocol); $this->config->getLogger()->debug('Initializing HTTP Jaeger Tracer with Jaeger.Thrift over Binary protocol'); $sender = new JaegerSender($client, $this->config->getLogger()); + $sender->setMaxBufferLength($this->config->getMaxBufferLength()); return new JaegerReporter($sender); } } diff --git a/src/Jaeger/ReporterFactory/JaegerReporterFactory.php b/src/Jaeger/ReporterFactory/JaegerReporterFactory.php index 139254c..1f030f7 100644 --- a/src/Jaeger/ReporterFactory/JaegerReporterFactory.php +++ b/src/Jaeger/ReporterFactory/JaegerReporterFactory.php @@ -37,6 +37,7 @@ public function createReporter() : ReporterInterface $client = new AgentClient($protocol); $this->config->getLogger()->debug('Initializing UDP Jaeger Tracer with Jaeger.Thrift over Binary protocol'); $sender = new JaegerSender($client, $this->config->getLogger()); + $sender->setMaxBufferLength($this->config->getMaxBufferLength()); return new JaegerReporter($sender); } } diff --git a/src/Jaeger/Sender/JaegerSender.php b/src/Jaeger/Sender/JaegerSender.php index a525b4a..dfce510 100644 --- a/src/Jaeger/Sender/JaegerSender.php +++ b/src/Jaeger/Sender/JaegerSender.php @@ -15,6 +15,9 @@ use Jaeger\Tracer; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; +use Thrift\Protocol\TBinaryProtocol; +use Thrift\Protocol\TCompactProtocol; +use Thrift\Transport\TMemoryBuffer; use const Jaeger\JAEGER_HOSTNAME_TAG_KEY; class JaegerSender implements SenderInterface @@ -44,6 +47,17 @@ class JaegerSender implements SenderInterface */ private $mapper; + /** + * @var int + */ + private $jaegerBatchOverheadLength = 512; + + /** + * The maximum length of the thrift-objects for a jaeger-batch. + * + * @var int + */ + private $maxBufferLength = 64000; /** * @param AgentIf $agentClient @@ -81,6 +95,11 @@ public function flush(): int return $count; } + public function setMaxBufferLength($maxBufferLength) + { + $this->maxBufferLength = $maxBufferLength; + } + /** * @param JaegerSpan[] $spans * @return array @@ -110,6 +129,52 @@ private function send(array $spans) return ; } + $chunks = $this->chunkSplit($spans); + foreach ($chunks as $chunk) { + /** @var JaegerThriftSpan[] $chunk */ + $this->emitJaegerBatch($chunk); + } + } + + /** + * @param JaegerThriftSpan $span + */ + private function getBufferLength($span) + { + $memoryBuffer = new TMemoryBuffer(); + $span->write(new TBinaryProtocol($memoryBuffer)); + return $memoryBuffer->available(); + } + + private function chunkSplit(array $spans): array + { + $actualBufferSize = $this->jaegerBatchOverheadLength; + $chunkId = 0; + $chunks = []; + + foreach ($spans as $span) { + $spanBufferLength = $this->getBufferLength($span); + if (!empty($chunks[$chunkId]) && ($actualBufferSize + $spanBufferLength) > $this->maxBufferLength) { + // point to next chunk + ++$chunkId; + + // reset buffer size + $actualBufferSize = $this->jaegerBatchOverheadLength; + } + + if (!isset($chunks[$chunkId])) { + $chunks[$chunkId] = []; + } + + $chunks[$chunkId][] = $span; + $actualBufferSize += $spanBufferLength; + } + + return $chunks; + } + + protected function emitJaegerBatch(array $spans) + { /** @var Tag[] $tags */ $tags = []; diff --git a/tests/Jaeger/Sender/JaegerThriftSenderTest.php b/tests/Jaeger/Sender/JaegerThriftSenderTest.php index 0c83477..5bd6d0a 100644 --- a/tests/Jaeger/Sender/JaegerThriftSenderTest.php +++ b/tests/Jaeger/Sender/JaegerThriftSenderTest.php @@ -4,6 +4,7 @@ namespace Jaeger\Tests\Sender; use Jaeger\Sender\JaegerSender; +use Jaeger\Sender\UdpSender; use Jaeger\Span; use Jaeger\SpanContext; use Jaeger\Thrift\Agent\AgentClient; @@ -41,6 +42,7 @@ public function testFlush(): void $client = $this->createMock(AgentClient::class); $sender = new JaegerSender($client); + $sender->setMaxBufferLength(64000); $client ->expects(self::exactly(1)) @@ -56,6 +58,7 @@ public function testFlush(): void public function testEmitBatch() { $client = $this->createMock(AgentClient::class); $sender = new JaegerSender($client); + $sender->setMaxBufferLength(64000); $span = $this->createMock(Span::class); $span->method('getOperationName')->willReturn('dummy-operation'); @@ -82,4 +85,42 @@ public function testEmitBatch() { $sender->append($span); $this->assertEquals(1, $sender->flush()); } + + public function testMaxBufferLength() { + $tracer = $this->createMock(Tracer::class); + $tracer->method('getIpAddress')->willReturn(''); + $tracer->method('getServiceName')->willReturn(''); + + $context = $this->createMock(SpanContext::class); + + $span = $this->createMock(Span::class); + $span->method('getOperationName')->willReturn('dummy-operation'); + $span->method('getTracer')->willReturn($tracer); + $span->method('getContext')->willReturn($context); + + $client = $this->createMock(AgentClient::class); + + $mockBuilder = $this->getMockBuilder(JaegerSender::class); + $mockMethods = ['emitJaegerBatch']; + if (method_exists($mockBuilder, "onlyMethods")) { + $mockBuilder = $mockBuilder->onlyMethods($mockMethods); + } else { + $mockBuilder = $mockBuilder->setMethods($mockMethods); + } + $sender = $mockBuilder->setConstructorArgs([$client])->getMock(); + $sender->setMaxBufferLength(800); + $sender->expects(self::exactly(2)) + ->method('emitJaegerBatch') + ->withConsecutive( + [self::countOf(2)], + [self::countOf(1)] + ); + + // jaeger batch overhead parameter = 512 + $sender->append($span); // 512 + 143 < 800 - chunk 1 + $sender->append($span); // 512 + 143*2 => 798 < 800 - chunk 1 + $sender->append($span); // 512 + 143*3 > 800 - chunk 2 + + self::assertEquals(3, $sender->flush()); + } }