Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consistent Span name and http.route.name #24017

Merged
merged 1 commit into from
Mar 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,7 @@ void dropNames(Optional<StaticResourcesBuildItem> staticResources,
if (config.isPropertyPresent("quarkus.http.root-path")) {
String rootPath = config.getValue("quarkus.http.root-path", new NormalizeRootHttpPathConverter());
String nonApplicationRootPath = config.getRawValue("quarkus.http.non-application-root-path");
// span names don't include the leading slash
nonApplicationUris.add(rootPath.substring(1) + nonApplicationRootPath);
nonApplicationUris.add(rootPath + nonApplicationRootPath);
ebullient marked this conversation as resolved.
Show resolved Hide resolved
}
dropNonApplicationUris.produce(new DropNonApplicationUrisBuildItem(nonApplicationUris));

Expand All @@ -161,12 +160,7 @@ void dropNames(Optional<StaticResourcesBuildItem> staticResources,
if (staticResources.isPresent()) {
for (StaticResourcesBuildItem.Entry entry : staticResources.get().getEntries()) {
if (!entry.isDirectory()) {
// span names don't include the leading slash
if (entry.getPath().startsWith("/") && entry.getPath().length() > 1) {
resources.add(entry.getPath().substring(1));
} else {
resources.add(entry.getPath());
}
resources.add(entry.getPath());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ void testDevMode() {
//and the hot replacement stuff is not messing things up
RestAssured.when().get("/hello").then()
.statusCode(200)
.body(is("hello"));
.body(is("/hello"));

RestAssured.when().get("/tracer").then()
.statusCode(200)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ void telemetry() {
List<SpanData> spans = spanExporter.getFinishedSpanItems(2);
assertEquals("HelloBean.hello", spans.get(0).getName());
assertEquals(INTERNAL, spans.get(0).getKind());
assertEquals("hello", spans.get(1).getName());
assertEquals("/hello", spans.get(1).getName());
assertEquals(SERVER, spans.get(1).getKind());
assertEquals(spans.get(0).getParentSpanId(), spans.get(1).getSpanId());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ void resource() {
.body(is("hello"));

List<SpanData> spans = spanExporter.getFinishedSpanItems(1);
assertEquals("hello", spans.get(0).getName());
assertEquals("/hello", spans.get(0).getName());
assertEquals(SERVER, spans.get(0).getKind());
assertEquals("authservice", spans.get(0).getResource().getAttribute(AttributeKey.stringKey("service.name")));
assertEquals(config.getRawValue("quarkus.uuid"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ void client() {

SpanData server = spans.get(0);
assertEquals(SERVER, server.getKind());
assertEquals("hello", server.getName());
assertEquals("/hello", server.getName());
assertEquals(HTTP_OK, server.getAttributes().get(HTTP_STATUS_CODE));
assertEquals(HttpMethod.GET, server.getAttributes().get(HTTP_METHOD));
assertEquals("/hello", server.getAttributes().get(HTTP_ROUTE));
Expand All @@ -71,7 +71,7 @@ void client() {

SpanData client = spans.get(1);
assertEquals(CLIENT, client.getKind());
assertEquals("hello", client.getName());
assertEquals("/hello", client.getName());
assertEquals(HTTP_OK, client.getAttributes().get(HTTP_STATUS_CODE));
assertEquals(HttpMethod.GET, client.getAttributes().get(HTTP_METHOD));
assertEquals(uri.toString() + "hello", client.getAttributes().get(HTTP_URL));
Expand All @@ -89,7 +89,7 @@ void spanNameWithoutQueryString() {

SpanData client = spans.get(1);
assertEquals(CLIENT, client.getKind());
assertEquals("hello", client.getName());
assertEquals("/hello", client.getName());
assertEquals(HTTP_OK, client.getAttributes().get(HTTP_STATUS_CODE));
assertEquals(HttpMethod.GET, client.getAttributes().get(HTTP_METHOD));
assertEquals(uri.toString() + "hello?query=1", client.getAttributes().get(HTTP_URL));
Expand All @@ -109,7 +109,7 @@ void urlWithoutAuthentication() {

SpanData client = spans.get(1);
assertEquals(CLIENT, client.getKind());
assertEquals("hello", client.getName());
assertEquals("/hello", client.getName());
assertEquals(HTTP_OK, client.getAttributes().get(HTTP_STATUS_CODE));
assertEquals(HttpMethod.GET, client.getAttributes().get(HTTP_METHOD));
assertEquals(uri.toString() + "hello?query=1", client.getAttributes().get(HTTP_URL));
Expand All @@ -127,7 +127,7 @@ void path() {

SpanData server = spans.get(0);
assertEquals(SERVER, server.getKind());
assertEquals("hello/{path}", server.getName());
assertEquals("/hello/{path}", server.getName());
assertEquals(HTTP_OK, server.getAttributes().get(HTTP_STATUS_CODE));
assertEquals(HttpMethod.GET, server.getAttributes().get(HTTP_METHOD));
assertEquals("/hello/{path}", server.getAttributes().get(HTTP_ROUTE));
Expand All @@ -136,7 +136,7 @@ void path() {

SpanData client = spans.get(1);
assertEquals(CLIENT, client.getKind());
assertEquals("hello/{path}", client.getName());
assertEquals("/hello/{path}", client.getName());
assertEquals(HTTP_OK, client.getAttributes().get(HTTP_STATUS_CODE));
assertEquals(HttpMethod.GET, client.getAttributes().get(HTTP_METHOD));
assertEquals(uri.toString() + "hello/another", client.getAttributes().get(HTTP_URL));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ void client() throws Exception {

SpanData server = spans.get(0);
assertEquals(SERVER, server.getKind());
assertEquals("hello", server.getName());
assertEquals("/hello", server.getName());
assertEquals(HTTP_OK, server.getAttributes().get(HTTP_STATUS_CODE));
assertEquals(HttpMethod.GET, server.getAttributes().get(HTTP_METHOD));
assertEquals("/hello", server.getAttributes().get(HTTP_ROUTE));
Expand Down Expand Up @@ -96,7 +96,7 @@ void path() throws Exception {

SpanData server = spans.get(0);
assertEquals(SERVER, server.getKind());
assertEquals("hello/:name", server.getName());
assertEquals("/hello/:name", server.getName());
assertEquals(HTTP_OK, server.getAttributes().get(HTTP_STATUS_CODE));
assertEquals(HttpMethod.GET, server.getAttributes().get(HTTP_METHOD));
assertEquals("/hello/:name", server.getAttributes().get(HTTP_ROUTE));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ void spanNameWithoutQueryString() {

assertEquals("io.quarkus.vertx.opentelemetry", spans.get(0).getName());
assertEquals("hello!", spans.get(0).getAttributes().get(stringKey("test.message")));
assertEquals("tracer", spans.get(1).getName());
assertEquals("/tracer", spans.get(1).getName());
assertEquals(200, spans.get(1).getAttributes().get(HTTP_STATUS_CODE));
assertEquals("1.1", spans.get(1).getAttributes().get(HTTP_FLAVOR));
assertEquals("/tracer?id=1", spans.get(1).getAttributes().get(HTTP_TARGET));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,12 @@ private static class ClientSpanNameExtractor implements SpanNameExtractor<Client
public String extract(final ClientRequestContext request) {
String pathTemplate = (String) request.getProperty("UrlPathTemplate");
if (pathTemplate != null && pathTemplate.length() > 1) {
return pathTemplate.substring(1);
return pathTemplate;
}

String uriPath = request.getUri().getPath();
if (uriPath.length() > 1) {
return uriPath.substring(1);
if (uriPath != null && uriPath.length() > 1) {
return uriPath;
}

return "HTTP " + request.getMethod();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,24 @@
package io.quarkus.opentelemetry.runtime.tracing.vertx;

import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteSource.FILTER;
import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.HTTP_CLIENT_IP;
import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.HTTP_ROUTE;
import static io.quarkus.opentelemetry.runtime.OpenTelemetryConfig.INSTRUMENTATION_NAME;

import java.net.URI;
import java.util.List;
import java.util.function.BiConsumer;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Scope;
import io.opentelemetry.context.propagation.TextMapGetter;
import io.opentelemetry.context.propagation.TextMapSetter;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder;
import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesGetter;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteGetter;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesGetter;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
Expand Down Expand Up @@ -81,6 +80,19 @@ public OpenTelemetryVertxTracer.SpanOperation spanOperation(
return OpenTelemetryVertxTracer.SpanOperation.span(context, requestSpan, headers, spanContext, scope);
}

@Override
public <R> void sendResponse(
final Context context,
final R response,
final OpenTelemetryVertxTracer.SpanOperation spanOperation,
final Throwable failure,
final TagExtractor<R> tagExtractor) {

HttpRouteHolder.updateHttpRoute(spanOperation.getSpanContext(), FILTER, RouteGetter.ROUTE_GETTER,
((HttpRequestSpan) spanOperation.getRequest()));
InstrumenterVertxTracer.super.sendResponse(context, response, spanOperation, failure, tagExtractor);
}

@Override
public HttpRequest writableHeaders(
final HttpRequest request, final BiConsumer<String, String> headers) {
Expand All @@ -93,12 +105,13 @@ private static Instrumenter<HttpRequest, HttpResponse> getServerInstrumenter(fin
InstrumenterBuilder<HttpRequest, HttpResponse> serverBuilder = Instrumenter.builder(
openTelemetry,
INSTRUMENTATION_NAME,
ServerSpanNameExtractor.create(serverAttributesExtractor));
HttpSpanNameExtractor.create(serverAttributesExtractor));

return serverBuilder
.setSpanStatusExtractor(HttpSpanStatusExtractor.create(serverAttributesExtractor))
.addAttributesExtractor(HttpServerAttributesExtractor.create(serverAttributesExtractor))
.addAttributesExtractor(new AdditionalServerAttributesExtractor())
.addContextCustomizer(HttpRouteHolder.get())
.newServerInstrumenter(new HttpRequestTextMapGetter());
}

Expand All @@ -117,6 +130,26 @@ private static Instrumenter<HttpRequest, HttpResponse> getClientInstrumenter(fin
.newClientInstrumenter(new HttpRequestTextMapSetter());
}

private static class RouteGetter implements HttpRouteGetter<HttpRequestSpan> {
static final RouteGetter ROUTE_GETTER = new RouteGetter();

@Override
public String get(final io.opentelemetry.context.Context context, final HttpRequestSpan requestSpan) {
// RESTEasy
String route = requestSpan.getContext().getLocal("UrlPathTemplate");
if (route == null) {
// Vert.x
route = requestSpan.getContext().getLocal("VertxRoute");
}

if (route != null && route.length() > 1) {
return route;
}

return null;
}
}

private static class ServerAttributesExtractor implements HttpServerAttributesGetter<HttpRequest, HttpResponse> {
@Override
public String flavor(final HttpRequest request) {
Expand Down Expand Up @@ -213,37 +246,13 @@ private static Long getContentLength(final MultiMap headers) {
}
}

// TODO - Ideally this should use HttpSpanNameExtractor, but to keep the name without the slash we use our own.
private static class ServerSpanNameExtractor implements SpanNameExtractor<HttpRequest> {
private final HttpServerAttributesGetter<HttpRequest, HttpResponse> serverAttributesExtractor;

ServerSpanNameExtractor(
final HttpServerAttributesGetter<HttpRequest, HttpResponse> serverAttributesExtractor) {
this.serverAttributesExtractor = serverAttributesExtractor;
}

@Override
public String extract(final HttpRequest httpRequest) {
if (httpRequest instanceof HttpServerRequest) {
String path = URI.create(httpRequest.uri()).getPath();
if (path != null && path.length() > 1) {
return path.substring(1);
} else {
return "HTTP " + httpRequest.method();
}
}
return null;
}

static SpanNameExtractor<HttpRequest> create(
HttpServerAttributesGetter<HttpRequest, HttpResponse> serverAttributesExtractor) {
return new ServerSpanNameExtractor(serverAttributesExtractor);
}
}

private static class AdditionalServerAttributesExtractor implements AttributesExtractor<HttpRequest, HttpResponse> {
@Override
public void onStart(final AttributesBuilder attributes, final HttpRequest httpRequest) {
public void onStart(
final AttributesBuilder attributes,
final io.opentelemetry.context.Context parentContext,
final HttpRequest httpRequest) {

if (httpRequest instanceof HttpServerRequest) {
set(attributes, HTTP_CLIENT_IP, VertxUtil.extractClientIP((HttpServerRequest) httpRequest));
}
Expand All @@ -252,26 +261,11 @@ public void onStart(final AttributesBuilder attributes, final HttpRequest httpRe
@Override
public void onEnd(
final AttributesBuilder attributes,
final io.opentelemetry.context.Context context,
final HttpRequest httpRequest,
final HttpResponse httpResponse,
final Throwable error) {

// The UrlPathTemplate is only added to the Vert.x context after the instrumenter start
if (httpRequest instanceof HttpRequestSpan) {
HttpRequestSpan httpRequestSpan = (HttpRequestSpan) httpRequest;
// RESTEasy
String route = httpRequestSpan.getContext().getLocal("UrlPathTemplate");
if (route == null) {
// Vert.x
route = httpRequestSpan.getContext().getLocal("VertxRoute");
}

if (route != null && route.length() > 1) {
set(attributes, HTTP_ROUTE, route);
Span span = Span.fromContext(httpRequestSpan.getSpanContext());
span.updateName(route.substring(1));
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ void get() {

assertEquals(SpanKind.SERVER.toString(), server.get("kind"));
assertEquals(server.get("parentSpanId"), client.get("spanId"));
assertEquals("reactive", server.get("name"));
assertEquals("/reactive", server.get("name"));
assertEquals("/reactive", ((Map<?, ?>) server.get("attributes")).get(HTTP_ROUTE.getKey()));
assertEquals("/reactive?name=Naruto", ((Map<?, ?>) server.get("attributes")).get(HTTP_TARGET.getKey()));
assertEquals(HTTP_OK, ((Map<?, ?>) server.get("attributes")).get(HTTP_STATUS_CODE.getKey()));
Expand Down Expand Up @@ -100,7 +100,7 @@ void post() {

assertEquals(SpanKind.SERVER.toString(), server.get("kind"));
assertEquals(server.get("parentSpanId"), client.get("spanId"));
assertEquals("reactive", server.get("name"));
assertEquals("/reactive", server.get("name"));
assertEquals("/reactive", ((Map<?, ?>) server.get("attributes")).get(HTTP_ROUTE.getKey()));
assertEquals("/reactive", ((Map<?, ?>) server.get("attributes")).get(HTTP_TARGET.getKey()));
assertEquals(HTTP_OK, ((Map<?, ?>) server.get("attributes")).get(HTTP_STATUS_CODE.getKey()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ void span() {
assertEquals(1, spans.size());

assertEquals(SERVER.toString(), spans.get(0).get("kind"));
assertEquals("hello", spans.get(0).get("name"));
assertEquals("/hello", spans.get(0).get("name"));
assertEquals(HTTP_OK, ((Map<?, ?>) spans.get(0).get("attributes")).get(HTTP_STATUS_CODE.toString()));
assertEquals(HttpMethod.GET.toString(), ((Map<?, ?>) spans.get(0).get("attributes")).get(HTTP_METHOD.toString()));
assertEquals("/hello", ((Map<?, ?>) spans.get(0).get("attributes")).get(HTTP_ROUTE.toString()));
Expand All @@ -74,7 +74,7 @@ void spanPath() {
assertEquals(1, spans.size());

assertEquals(SERVER.toString(), spans.get(0).get("kind"));
assertEquals("hello/:name", spans.get(0).get("name"));
assertEquals("/hello/:name", spans.get(0).get("name"));
assertEquals(HTTP_OK, ((Map<?, ?>) spans.get(0).get("attributes")).get(HTTP_STATUS_CODE.toString()));
assertEquals(HttpMethod.GET.toString(), ((Map<?, ?>) spans.get(0).get("attributes")).get(HTTP_METHOD.toString()));
assertEquals("/hello/:name", ((Map<?, ?>) spans.get(0).get("attributes")).get(HTTP_ROUTE.toString()));
Expand All @@ -95,7 +95,7 @@ void post() {
assertEquals(1, spans.size());

assertEquals(SERVER.toString(), spans.get(0).get("kind"));
assertEquals("hello", spans.get(0).get("name"));
assertEquals("/hello", spans.get(0).get("name"));
assertEquals(HTTP_OK, ((Map<?, ?>) spans.get(0).get("attributes")).get(HTTP_STATUS_CODE.toString()));
assertEquals(HttpMethod.POST.toString(), ((Map<?, ?>) spans.get(0).get("attributes")).get(HTTP_METHOD.toString()));
assertEquals("/hello", ((Map<?, ?>) spans.get(0).get("attributes")).get(HTTP_ROUTE.toString()));
Expand Down Expand Up @@ -137,7 +137,7 @@ void bus() {
assertEquals("bus", ((Map<?, ?>) spans.get(1).get("attributes")).get(MESSAGING_DESTINATION.toString()));

assertEquals(SERVER.toString(), spans.get(2).get("kind"));
assertEquals("bus", spans.get(2).get("name"));
assertEquals("/bus", spans.get(2).get("name"));
assertEquals(HTTP_OK, ((Map<?, ?>) spans.get(2).get("attributes")).get(HTTP_STATUS_CODE.toString()));
assertEquals(HttpMethod.GET.toString(), ((Map<?, ?>) spans.get(2).get("attributes")).get(HTTP_METHOD.toString()));
assertEquals("/bus", ((Map<?, ?>) spans.get(2).get("attributes")).get(HTTP_ROUTE.toString()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ void sqlClient() {
assertEquals(spans.get(0).getTraceId(), spans.get(1).getTraceId());
assertEquals(spans.get(0).getSpanId(), spans.get(1).getParentSpanId());

assertEquals("sqlClient", spans.get(0).getName());
assertEquals("/sqlClient", spans.get(0).getName());
assertEquals(HTTP_OK, spans.get(0).getAttributes().get(HTTP_STATUS_CODE));

assertEquals("SELECT USERS", spans.get(1).getName());
Expand Down
Loading