Skip to content

Commit

Permalink
Merge pull request #81 from jcchavezs/fixes_span_name
Browse files Browse the repository at this point in the history
fix: fixes the span name by accepting the route on response time.
  • Loading branch information
jcchavezs authored Oct 23, 2020
2 parents 71c072d + d051fa5 commit 7518deb
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 7 deletions.
4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": "^2.0.1"
},
"require-dev": {
"jcchavezs/httptest": "~0.2",
Expand Down Expand Up @@ -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/* ./"
}
}
}
2 changes: 1 addition & 1 deletion src/ZipkinBundle/KernelListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
Expand Down
14 changes: 13 additions & 1 deletion src/ZipkinBundle/Response.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down
1 change: 1 addition & 0 deletions tests/E2E/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 4 additions & 1 deletion tests/E2E/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 server span has the right name
test "$(echo "$TRACES" | jq -cr ".[0][0].name")" = "get /_health"

exit $?
5 changes: 3 additions & 2 deletions tests/Unit/ResponseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}

Expand Down

0 comments on commit 7518deb

Please sign in to comment.