From 9f9f62889b1db98ecc0d97b0e9eda643595bd87d Mon Sep 17 00:00:00 2001 From: Steve Oliver Date: Sun, 30 Jul 2023 10:06:35 -0700 Subject: [PATCH 1/3] Tag span with laravel_action only when route exists This prevents `TypeError: method_exists(): Argument #1 ($object_or_class) must be of type object|string, null given` when trying to tag a request for a route that does not exist. --- src/Middleware/TraceRequests.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Middleware/TraceRequests.php b/src/Middleware/TraceRequests.php index fc2597e..b99d5f3 100644 --- a/src/Middleware/TraceRequests.php +++ b/src/Middleware/TraceRequests.php @@ -106,8 +106,10 @@ protected function tagRequestData(Span $span, Request $request): void */ protected function tagResponseData(Span $span, Request $request, $response): void { - if (method_exists($request->route(), 'getActionName')) { - $span->tag('laravel_action', $request->route()->getActionName()); + if ($route = $request->route()) { + if (method_exists($route, 'getActionName')) { + $span->tag('laravel_action', $route->getActionName()); + } } $span->tag('response_status', strval($response->getStatusCode())); From 0fcefc67bdf458d724323e1ad9d641849bea9c9d Mon Sep 17 00:00:00 2001 From: Steve Oliver Date: Sun, 30 Jul 2023 10:16:45 -0700 Subject: [PATCH 2/3] Check $route before method_exists($route, 'uri') --- src/Middleware/TraceRequests.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Middleware/TraceRequests.php b/src/Middleware/TraceRequests.php index b99d5f3..2928ec5 100644 --- a/src/Middleware/TraceRequests.php +++ b/src/Middleware/TraceRequests.php @@ -237,7 +237,7 @@ protected function hideSensitiveInput(Collection $input): Collection */ protected function isLaravelRoute($route): bool { - return method_exists($route, 'uri'); + return $route && method_exists($route, 'uri'); } /** From 12d87947217d43c873252850dc9715150bc18c0e Mon Sep 17 00:00:00 2001 From: Summer Date: Fri, 18 Aug 2023 17:12:11 +0800 Subject: [PATCH 3/3] feat: zipkin support PercentageSampler --- config/tracing.php | 2 + src/Drivers/Zipkin/ZipkinTracer.php | 18 +++++-- src/TracingDriverManager.php | 33 ++++++++++++- tests/Zipkin/InteractsWithZipkin.php | 7 ++- tests/Zipkin/TracerTest.php | 74 ++++++++++++++++++++++++++++ 5 files changed, 128 insertions(+), 6 deletions(-) diff --git a/config/tracing.php b/config/tracing.php index e2b235a..9a365e4 100644 --- a/config/tracing.php +++ b/config/tracing.php @@ -97,6 +97,8 @@ 'max_tag_len' => 1048576, 'request_timeout' => 5, ], + 'sampler_class' => \Zipkin\Samplers\BinarySampler::class, + 'percentage_sampler_rate' => env('ZIPKIN_PERCENTAGE_SAMPLER_RATE', 0.2), ], ]; diff --git a/src/Drivers/Zipkin/ZipkinTracer.php b/src/Drivers/Zipkin/ZipkinTracer.php index e09dcd3..527882f 100644 --- a/src/Drivers/Zipkin/ZipkinTracer.php +++ b/src/Drivers/Zipkin/ZipkinTracer.php @@ -69,6 +69,11 @@ class ZipkinTracer implements Tracer */ protected $reporter; + /** + * @var Sampler|null + */ + protected $sampler; + /** * @var \Zipkin\DefaultTracing|\Zipkin\Tracing */ @@ -107,6 +112,7 @@ class ZipkinTracer implements Tracer * @param bool|null $usesTraceId128bits * @param int|null $requestTimeout * @param Reporter|null $reporter + * @param Sampler|null $sampler */ public function __construct( string $serviceName, @@ -114,7 +120,8 @@ public function __construct( int $port, ?bool $usesTraceId128bits = false, ?int $requestTimeout = 5, - ?Reporter $reporter = null + ?Reporter $reporter = null, + ?Sampler $sampler = null ) { $this->serviceName = $serviceName; $this->host = $host; @@ -122,6 +129,7 @@ public function __construct( $this->usesTraceId128bits = $usesTraceId128bits; $this->requestTimeout = $requestTimeout; $this->reporter = $reporter; + $this->sampler = $sampler; } /** @@ -320,7 +328,7 @@ public function flush(): void protected function createReporter(): Reporter { if (!$this->reporter) { - return new HttpReporter([ + $this->reporter = new HttpReporter([ 'endpoint_url' => sprintf('http://%s:%s/api/v2/spans', $this->host, $this->port), 'timeout' => $this->requestTimeout, ]); @@ -348,7 +356,11 @@ protected function createEndpoint(): Endpoint */ protected function createSampler(): Sampler { - return BinarySampler::createAsAlwaysSample(); + if (!$this->sampler) { + $this->sampler = BinarySampler::createAsAlwaysSample(); + } + + return $this->sampler; } protected function registerDefaultExtractionFormats(): void diff --git a/src/TracingDriverManager.php b/src/TracingDriverManager.php index ba22900..88dd2e9 100644 --- a/src/TracingDriverManager.php +++ b/src/TracingDriverManager.php @@ -4,9 +4,12 @@ use Illuminate\Contracts\Config\Repository; use Illuminate\Support\Manager; +use InvalidArgumentException; use Vinelab\Tracing\Contracts\Tracer; use Vinelab\Tracing\Drivers\Null\NullTracer; use Vinelab\Tracing\Drivers\Zipkin\ZipkinTracer; +use Zipkin\Samplers\BinarySampler; +use Zipkin\Samplers\PercentageSampler; class TracingDriverManager extends Manager { @@ -43,6 +46,7 @@ public function getDefaultDriver() * Create an instance of Zipkin tracing engine * * @return ZipkinTracer|Tracer + * @throws InvalidArgumentException */ public function createZipkinDriver() { @@ -51,7 +55,9 @@ public function createZipkinDriver() $this->config->get('tracing.zipkin.host'), $this->config->get('tracing.zipkin.port'), $this->config->get('tracing.zipkin.options.128bit'), - $this->config->get('tracing.zipkin.options.request_timeout', 5) + $this->config->get('tracing.zipkin.options.request_timeout', 5), + null, + $this->getZipkinSampler() ); ZipkinTracer::setMaxTagLen( @@ -65,4 +71,29 @@ public function createNullDriver() { return new NullTracer(); } + + /** + * @return BinarySampler|PercentageSampler + * @throws InvalidArgumentException + */ + protected function getZipkinSampler() + { + $samplerClassName = $this->config->get('tracing.zipkin.sampler_class'); + if (!class_exists($samplerClassName)) { + throw new InvalidArgumentException( + \sprintf('Invalid sampler class. Expected `BinarySampler` or `PercentageSampler`, got %f', $samplerClassName) + ); + } + + switch ($samplerClassName) { + case BinarySampler::class: + $sampler = BinarySampler::createAsAlwaysSample(); + break; + case PercentageSampler::class: + $sampler = PercentageSampler::create($this->config->get('tracing.zipkin.percentage_sampler_rate')); + break; + } + + return $sampler; + } } diff --git a/tests/Zipkin/InteractsWithZipkin.php b/tests/Zipkin/InteractsWithZipkin.php index 8af6c5a..94bda75 100644 --- a/tests/Zipkin/InteractsWithZipkin.php +++ b/tests/Zipkin/InteractsWithZipkin.php @@ -5,6 +5,7 @@ use Vinelab\Tracing\Drivers\Zipkin\ZipkinTracer; use Zipkin\Recording\Span; use Zipkin\Reporter; +use Zipkin\Sampler; trait InteractsWithZipkin { @@ -15,6 +16,7 @@ trait InteractsWithZipkin * @param int $port * @param int $requestTimeout * @param bool $usesTraceId128bits + * @param Sampler|null $sampler * @return ZipkinTracer */ protected function createTracer( @@ -23,10 +25,11 @@ protected function createTracer( string $host = 'localhost', int $port = 9411, int $requestTimeout = 5, - bool $usesTraceId128bits = false + bool $usesTraceId128bits = false, + ?Sampler $sampler = null ): ZipkinTracer { - $tracer = new ZipkinTracer($serviceName, $host, $port, $usesTraceId128bits, $requestTimeout, $reporter); + $tracer = new ZipkinTracer($serviceName, $host, $port, $usesTraceId128bits, $requestTimeout, $reporter, $sampler); $tracer->init(); return $tracer; diff --git a/tests/Zipkin/TracerTest.php b/tests/Zipkin/TracerTest.php index d4024c1..6c4c7f0 100644 --- a/tests/Zipkin/TracerTest.php +++ b/tests/Zipkin/TracerTest.php @@ -3,6 +3,7 @@ namespace Vinelab\Tracing\Tests\Zipkin; use Carbon\Carbon; +use Closure; use Google\Cloud\PubSub\Message; use GuzzleHttp\Psr7\Request as PsrRequest; use Illuminate\Http\Request; @@ -16,11 +17,52 @@ use Vinelab\Tracing\Propagation\Formats; use Vinelab\Tracing\Tests\Fixtures\NoopReporter; use Zipkin\Propagation\TraceContext; +use Zipkin\Recording\Span; +use Zipkin\Samplers\BinarySampler; +use Zipkin\Samplers\PercentageSampler; class TracerTest extends TestCase { use InteractsWithZipkin; + /** @test */ + public function create_tracer_with_BinarySampler() + { + $reporter = Mockery::spy(NoopReporter::class); + + $sampler = BinarySampler::createAsAlwaysSample(); + + $tracer = $this->createTracer($reporter, 'orders', '192.88.105.145', 9444, 5, false, $sampler); + + $this->assertSpan($reporter, $tracer); + } + + /** @test */ + public function create_tracer_with_PercentageSampler_1_rate() + { + $reporter = Mockery::spy(NoopReporter::class); + + $sampler = PercentageSampler::create(1); + + $tracer = $this->createTracer($reporter, 'orders', '192.88.105.145', 9444, 5, false, $sampler); + + $this->assertSpan($reporter, $tracer); + } + + /** @test */ + public function create_tracer_with_PercentageSampler_0_rate() + { + $reporter = Mockery::spy(NoopReporter::class); + + $sampler = PercentageSampler::create(0); + + $tracer = $this->createTracer($reporter, 'orders', '192.88.105.145', 9444, 5, false, $sampler); + + $this->assertSpan($reporter, $tracer,function ($spans){ + $this->assertEmpty($spans); + }); + } + /** @test */ public function configure_reporter() { @@ -362,4 +404,36 @@ protected function assertValidTraceContext(SpanContext $spanContext): void $this->assertNotNull($spanContext); $this->assertInstanceOf(TraceContext::class, $spanContext->getRawContext()); } + + protected function assertSpan($reporter, ZipkinTracer $tracer, Closure $assert = null) + { + $startTimestamp = intval(microtime(true) * 1000000); + $span = $tracer->startSpan('Http Request', null, $startTimestamp); + $span->setName('Create Order'); + $span->tag('request_path', 'api/orders'); + $span->annotate('Create Payment'); + $span->annotate('Update Order Status'); + $finishTimestamp = intval(microtime(true) * 1000000); + $span->finish($finishTimestamp); + $tracer->flush(); + + if (!$assert) { + $assert = function ($spans) use ($startTimestamp, $finishTimestamp) { + /** @var Span $span */ + $span = $this->shiftSpan($spans); + $this->assertEquals('Create Order', $span->getName()); + $this->assertEquals($finishTimestamp - $startTimestamp, $span->getDuration()); + $this->assertEquals('api/orders', Arr::get($span->getTags(), 'request_path')); + $this->assertEquals('Create Payment', Arr::get($span->getAnnotations(), '0.value')); + $this->assertEquals('Update Order Status', Arr::get($span->getAnnotations(), '1.value')); + }; + } + + $reporter->shouldHaveReceived('report')->with( + Mockery::on(function ($spans) use ($assert) { + $assert($spans); + return true; + }) + ); + } }