diff --git a/README.md b/README.md index 69f089b..857324d 100644 --- a/README.md +++ b/README.md @@ -2,10 +2,10 @@ [![Build Status](https://travis-ci.org/jcchavezs/zipkin-instrumentation-symfony.svg?branch=master)](https://travis-ci.org/jcchavezs/zipkin-instrumentation-symfony) [![CircleCI](https://circleci.com/gh/jcchavezs/zipkin-instrumentation-symfony/tree/master.svg?style=svg)](https://circleci.com/gh/jcchavezs/zipkin-instrumentation-symfony/tree/master) -[![Latest Stable Version](https://poser.pugx.org/jcchavezs/zipkin-symfony/v/stable)](https://packagist.org/packages/jcchavezs/zipkin-instrumentation-symfony) +[![Latest Stable Version](https://poser.pugx.org/jcchavezs/zipkin-instrumentation-symfony/v/stable)](https://packagist.org/packages/jcchavezs/zipkin-instrumentation-symfony) [![Minimum PHP Version](https://img.shields.io/badge/php-%3E%3D%205.6-8892BF.svg)](https://php.net/) [![Total Downloads](https://poser.pugx.org/jcchavezs/zipkin-instrumentation-symfony/downloads)](https://packagist.org/packages/jcchavezs/zipkin-instrumentation-symfony) -[![License](https://poser.pugx.org/jcchavezs/zipkin-symfony/license)](https://packagist.org/packages/jcchavezs/zipkin-instrumentation-symfony) +[![License](https://poser.pugx.org/jcchavezs/zipkin-instrumentation-symfony/license)](https://packagist.org/packages/jcchavezs/zipkin-instrumentation-symfony) A Zipkin integration for Symfony applications @@ -212,7 +212,7 @@ services: ## Contributing -All contribution and feedback are welcome. +All contributions and feedback are welcome. ### Unit testing @@ -228,7 +228,7 @@ On every build we run a end to end (E2E) test against a symfony application. This test run in our CI tests but it can be also reproduced in local by: 1. Go to `tests/Integration` -2. Run `make SYMFONY_VERSION={{SYMFONY_VERSION}} build` to build the test application (by default newest version) +2. Run `SYMFONY_VERSION={{SYMFONY_VERSION}} make build` to build the test application (by default newest version) 3. Run `make run-zipkin` to start zipkin sever 4. Run `make run-app` to start the test application 5. Hit the application `curl -i http://localhost:8002/_health` diff --git a/src/ZipkinBundle/Middleware.php b/src/ZipkinBundle/Middleware.php index 4206a39..bce942b 100644 --- a/src/ZipkinBundle/Middleware.php +++ b/src/ZipkinBundle/Middleware.php @@ -15,11 +15,10 @@ use Symfony\Component\HttpKernel\Kernel; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\KernelEvent; -use Symfony\Component\HttpKernel\Event\ResponseEvent; final class Middleware { - const SPAN_CLOSER_KEY = 'zipkin_bundle_span_closer'; + const SCOPE_CLOSER_KEY = 'zipkin_bundle_scope_closer'; /** * @var Tracer @@ -95,7 +94,7 @@ public function onKernelRequest(KernelEvent $event) $span->tag($key, $value); } - $request->attributes->set(self::SPAN_CLOSER_KEY, $this->tracer->openScope($span)); + $request->attributes->set(self::SCOPE_CLOSER_KEY, $this->tracer->openScope($span)); } /** @@ -167,14 +166,14 @@ public function onKernelResponse(KernelEvent $event) public function onKernelTerminate(KernelEvent $event) { // Previously, the onKernelResponse listener did not exist in this class - // and hence we finished the span and closed the scope on terminate. - // However, terminate happens after the response have been sent and it could - // happen that span is being finished after some other processing attached by - // the user. onKernelResponse is the right place to finish the span but in order - // to not to break existing user relaying on the onKernelTerminate to finish - // the span we add this temporary fix. - - $scopeCloser = $event->getRequest()->attributes->get(self::SPAN_CLOSER_KEY); + // and hence we finished the span and closed the scope on the onKernelTerminate. + // However, terminate occurs after the response has been sent and it could + // happen that span is being finished after some other processing (potentially + // expensive) attached by the user. onKernelResponse is the right place to finish + // the span but in order to not to break existing user relaying on the + // onKernelTerminate to finish the span we add this temporary fix. + + $scopeCloser = $event->getRequest()->attributes->get(self::SCOPE_CLOSER_KEY); if ($scopeCloser !== null) { if ($this->usesDeprecatedEvents) { /** @@ -217,9 +216,11 @@ private function finishSpan(Request $request, $response) } $span->finish(); - $scopeCloser = $request->attributes->get(self::SPAN_CLOSER_KEY); + $scopeCloser = $request->attributes->get(self::SCOPE_CLOSER_KEY); if ($scopeCloser !== null) { $scopeCloser(); + // We reset the scope closer as it did its job + $request->attributes->remove(self::SCOPE_CLOSER_KEY); } }