Skip to content

Commit

Permalink
Merge pull request #24017 from radcortez/opentelemetry
Browse files Browse the repository at this point in the history 
Consistent Span name and http.route.name
  • Loading branch information
@ebullient
ebullient authored Mar 1, 2022
2 parents 731b9a4 + 21bd03f commit 86a9462
Show file tree
Hide file tree
Showing 14 changed files with 94 additions and 106 deletions.
10 changes: 2 additions & 8 deletions ...deployment/src/main/java/io/quarkus/opentelemetry/deployment/tracing/TracerProcessor.java
View file
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);
}
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
2 changes: 1 addition & 1 deletion ...eployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetryDevModeTest.java
View file
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
2 changes: 1 addition & 1 deletion ...eployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetryHttpCDITest.java
View file
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
2 changes: 1 addition & 1 deletion ...ployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetryResourceTest.java
View file
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
12 changes: 6 additions & 6 deletions ...oyment/src/test/java/io/quarkus/opentelemetry/deployment/RestClientOpenTelemetryTest.java
View file
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
4 changes: 2 additions & 2 deletions ...yment/src/test/java/io/quarkus/opentelemetry/deployment/VertxClientOpenTelemetryTest.java
View file
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
2 changes: 1 addition & 1 deletion .../deployment/src/test/java/io/quarkus/opentelemetry/deployment/VertxOpenTelemetryTest.java
View file
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
6 changes: 3 additions & 3 deletions ...n/java/io/quarkus/opentelemetry/runtime/tracing/restclient/OpenTelemetryClientFilter.java
View file
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
94 changes: 44 additions & 50 deletions ...main/java/io/quarkus/opentelemetry/runtime/tracing/vertx/HttpInstrumenterVertxTracer.java
View file
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
4 changes: 2 additions & 2 deletions ...e/src/test/java/io/quarkus/it/opentelemetry/reactive/OpenTelemetryReactiveClientTest.java
View file
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
8 changes: 4 additions & 4 deletions .../opentelemetry-vertx/src/test/java/io/quarkus/it/opentelemetry/vertx/HelloRouterTest.java
View file
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
2 changes: 1 addition & 1 deletion ...ts/opentelemetry-vertx/src/test/java/io/quarkus/it/opentelemetry/vertx/SqlClientTest.java
View file
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

0 comments on commit 86a9462

Please sign in to comment.