From f8549d1380096ac8c3163dc389789c8b8f765ed3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Carlos=20Ch=C3=A1vez?= Date: Wed, 21 Oct 2020 14:53:59 +0200 Subject: [PATCH 1/4] fix: fixes the span name by accepting the route on response time. --- composer.json | 4 ++-- src/ZipkinBundle/KernelListener.php | 2 +- src/ZipkinBundle/Response.php | 14 +++++++++++++- tests/E2E/Makefile | 1 + tests/Unit/ResponseTest.php | 5 +++-- 5 files changed, 20 insertions(+), 6 deletions(-) diff --git a/composer.json b/composer.json index be6979d..898ff3a 100644 --- a/composer.json +++ b/composer.json @@ -7,7 +7,7 @@ "symfony/http-kernel": "^4.4|^5.0", "symfony/routing": "^4.4|^5.0", "symfony/dependency-injection": "^4.4|^5.0", - "openzipkin/zipkin": "2.0.0" + "openzipkin/zipkin": "dev-master" }, "require-dev": { "jcchavezs/httptest": "~0.2", @@ -39,4 +39,4 @@ "lint": "./vendor/bin/phpcs --standard=ZEND --standard=PSR2 --ignore=*/vendor/* --ignore=*/tests/E2E/test-app/* ./", "fix-lint": "./vendor/bin/phpcbf --standard=ZEND --standard=PSR2 --ignore=*/vendor/* --ignore=*/tests/Integration/test-app/* ./" } -} \ No newline at end of file +} diff --git a/src/ZipkinBundle/KernelListener.php b/src/ZipkinBundle/KernelListener.php index 7c9e2eb..1d01294 100644 --- a/src/ZipkinBundle/KernelListener.php +++ b/src/ZipkinBundle/KernelListener.php @@ -223,7 +223,7 @@ private function finishSpan(HttpFoundationRequest $request, ?HttpFoundationRespo $routePath = $this->routeMapper->mapToPath($request); if ($response != null) { $this->parser->response( - new Response($response, new Request($request, $routePath)), + new Response($response, new Request($request), $routePath), $span->getContext(), $request->attributes->get(self::SPAN_CUSTOMIZER_KEY) ); diff --git a/src/ZipkinBundle/Response.php b/src/ZipkinBundle/Response.php index f903d4d..3d9c620 100644 --- a/src/ZipkinBundle/Response.php +++ b/src/ZipkinBundle/Response.php @@ -18,12 +18,19 @@ final class Response extends ServerResponse */ private $request; + /** + * @var string|null + */ + private $route; + public function __construct( HttpFoundationResponse $delegate, - ?Request $request + ?Request $request, + string $route = null ) { $this->delegate = $delegate; $this->request = $request; + $this->route = $route; } public function getRequest(): ?ServerRequest @@ -36,6 +43,11 @@ public function getStatusCode(): int return $this->delegate->getStatusCode(); } + public function getRoute(): ?string + { + return $this->route; + } + public function unwrap(): HttpFoundationResponse { return $this->delegate; diff --git a/tests/E2E/Makefile b/tests/E2E/Makefile index 1061310..ff800a5 100644 --- a/tests/E2E/Makefile +++ b/tests/E2E/Makefile @@ -13,6 +13,7 @@ run-app: ## Runs symfony app run-zipkin: ## Runs zipkin server docker run -d -p 9411:9411 openzipkin/zipkin + @echo "\nCheck your traces at http://localhost:9411/zipkin/\n" clean: ## Cleans build rm -rf test-app diff --git a/tests/Unit/ResponseTest.php b/tests/Unit/ResponseTest.php index 1a881b7..39bc250 100644 --- a/tests/Unit/ResponseTest.php +++ b/tests/Unit/ResponseTest.php @@ -18,10 +18,11 @@ public static function createResponse( int $statusCode, $headers = [], $body = null, - ServerRequest $request = null + ServerRequest $request = null, + string $route = null ): array { $delegateResponse = new HttpFoundationResponse($body, $statusCode, $headers); - $response = new Response($delegateResponse, $request); + $response = new Response($delegateResponse, $request, $route); return [$response, $delegateResponse, $request]; } From 6fe3d864dbfbe7acb797bdff7fd3dd469a8816f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Carlos=20Ch=C3=A1vez?= Date: Wed, 21 Oct 2020 15:21:26 +0200 Subject: [PATCH 2/4] tests: makes sure E2E checks also span name to verify the route. --- tests/E2E/test.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/E2E/test.sh b/tests/E2E/test.sh index fe2d9d8..fa9f13f 100755 --- a/tests/E2E/test.sh +++ b/tests/E2E/test.sh @@ -46,6 +46,9 @@ TRACES=$(curl -s $ZIPKIN_SERVER_HOSTPORT/api/v2/traces) test $(echo "$TRACES" | jq '.[0] | length') -eq 1 # makes sure the trace does not contain errors -test $(echo "$TRACES" | jq -c '.[0][0].tags.error') = "null" +test "$(echo "$TRACES" | jq -c '.[0][0].tags.error')" = "null" + +# makes sure the span has the right name +test "$(echo "$TRACES" | jq -cr ".[0][0].name")" = "get /_health" exit $? From 30f2cdd67243b5257e4cda9845d879b73b91154c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Carlos=20Ch=C3=A1vez?= Date: Wed, 21 Oct 2020 15:48:41 +0200 Subject: [PATCH 3/4] tests: improves wording. --- tests/E2E/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/E2E/test.sh b/tests/E2E/test.sh index fa9f13f..dc5a4f6 100755 --- a/tests/E2E/test.sh +++ b/tests/E2E/test.sh @@ -48,7 +48,7 @@ test $(echo "$TRACES" | jq '.[0] | length') -eq 1 # makes sure the trace does not contain errors test "$(echo "$TRACES" | jq -c '.[0][0].tags.error')" = "null" -# makes sure the span has the right name +# makes sure the server span has the right name test "$(echo "$TRACES" | jq -cr ".[0][0].name")" = "get /_health" exit $? From d051fa56a701f5eb038687c4552bb92eb9f9fd5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Carlos=20Ch=C3=A1vez?= Date: Wed, 21 Oct 2020 19:08:55 +0200 Subject: [PATCH 4/4] chore(deps): uses a release instead --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 898ff3a..8ef9000 100644 --- a/composer.json +++ b/composer.json @@ -7,7 +7,7 @@ "symfony/http-kernel": "^4.4|^5.0", "symfony/routing": "^4.4|^5.0", "symfony/dependency-injection": "^4.4|^5.0", - "openzipkin/zipkin": "dev-master" + "openzipkin/zipkin": "^2.0.1" }, "require-dev": { "jcchavezs/httptest": "~0.2",