Skip to content

Commit

Permalink
Merge pull request #18366 from kenfinnigan/fix-otel-span-name
Browse files Browse the repository at this point in the history 
Fix OpenTelemetry HTTP span naming
  • Loading branch information
@kenfinnigan
kenfinnigan authored Jul 2, 2021
2 parents 81f758e + f0ce563 commit f1d848f
Show file tree
Hide file tree
Showing 5 changed files with 286 additions and 5 deletions.
9 changes: 7 additions & 2 deletions ...ime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/vertx/VertxTracingAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public <R> Span receiveRequest(

// Add attributes
currentSpan.setAttribute(HTTP_FLAVOR, convertHttpVersion(httpServerRequest.version()));
currentSpan.setAttribute(HTTP_METHOD, httpServerRequest.method().name());
currentSpan.setAttribute(HTTP_METHOD, operation);
currentSpan.setAttribute(HTTP_TARGET, httpServerRequest.path());
currentSpan.setAttribute(HTTP_SCHEME, httpServerRequest.scheme());
currentSpan.setAttribute(HTTP_HOST, httpServerRequest.host());
Expand All @@ -122,7 +122,12 @@ public <R> Span receiveRequest(

private <R> String operationName(R request, String operationName) {
if (request instanceof HttpServerRequest) {
return ((HttpServerRequest) request).uri().substring(1);
final String uri = ((HttpServerRequest) request).uri();
if (uri.length() > 1) {
return uri.substring(1);
} else {
return "HTTP " + operationName;
}
}
return operationName;
}
Expand Down
15 changes: 13 additions & 2 deletions ...client/runtime/src/main/java/io/quarkus/opentelemetry/restclient/ClientTracingFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ public ClientTracingFilter(Tracer tracer) {
@Override
public void filter(ClientRequestContext requestContext) throws IOException {
// Create new span
SpanBuilder builder = tracer.spanBuilder(requestContext.getUri().getPath().substring(1))
String spanName = getSpanName(requestContext);
SpanBuilder builder = tracer.spanBuilder(getSpanName(requestContext))
.setSpanKind(SpanKind.CLIENT);

// Add attributes
Expand All @@ -50,11 +51,21 @@ public void filter(ClientRequestContext requestContext) throws IOException {
TEXT_MAP_PROPAGATOR.inject(Context.current().with(clientSpan), requestContext.getHeaders(), SETTER);
}

private String getSpanName(ClientRequestContext requestContext) {
final String uriPath = requestContext.getUri().getPath();
if (uriPath.length() > 1) {
return uriPath.substring(1);
} else {
// Generate span name as we have empty or "/" on @Path
return "HTTP " + ((ClientRequestContextImpl) requestContext).getInvocation().getMethod();
}
}

@Override
public void filter(ClientRequestContext requestContext, ClientResponseContext responseContext) throws IOException {
if (clientSpan != null) {
String pathTemplate = (String) requestContext.getProperty("UrlPathTemplate");
if (pathTemplate != null && !pathTemplate.isEmpty()) {
if (pathTemplate != null && pathTemplate.length() > 1) {
clientSpan.updateName(pathTemplate.substring(1));
}

Expand Down
38 changes: 37 additions & 1 deletion ...gration-tests/opentelemetry/src/main/java/io/quarkus/it/opentelemetry/SimpleResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,49 @@
import javax.ws.rs.Produces;
import javax.ws.rs.core.MediaType;

@Path("/")
import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;
import org.eclipse.microprofile.rest.client.inject.RestClient;

@Path("")
@Produces(MediaType.APPLICATION_JSON)
public class SimpleResource {
@RegisterRestClient(configKey = "simple")
public interface SimpleClient {
@Path("")
@GET
TraceData noPath();

@Path("/")
@GET
TraceData slashPath();
}

@Inject
TracedService tracedService;

@Inject
@RestClient
SimpleClient simpleClient;

@GET
public TraceData noPath() {
TraceData data = new TraceData();
data.message = "No path trace";
return data;
}

@GET
@Path("/nopath")
public TraceData noPathClient() {
return simpleClient.noPath();
}

@GET
@Path("/slashpath")
public TraceData slashPathClient() {
return simpleClient.slashPath();
}

@GET
@Path("/direct")
public TraceData directTrace() {
Expand Down
1 change: 1 addition & 0 deletions integration-tests/opentelemetry/src/main/resources/application.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ quarkus.application.name=opentelemetry-integration-test
quarkus.application.version=999-SNAPSHOT

pingpong/mp-rest/url=${test.url}
simple/mp-rest/url=${test.url}
228 changes: 228 additions & 0 deletions ...-tests/opentelemetry/src/test/java/io/quarkus/it/opentelemetry/OpenTelemetryTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,234 @@ void testResourceTracing() {
Assertions.assertNotNull(spanData.get("attr_http.user_agent"));
}

@Test
void testEmptyClientPath() {
resetExporter();

given()
.contentType("application/json")
.when().get("/nopath")
.then()
.statusCode(200)
.body("message", equalTo("No path trace"));

Awaitility.await().atMost(Duration.ofMinutes(2)).until(() -> getSpans().size() == 3);

boolean outsideServerFound = false;
boolean clientFound = false;
boolean clientServerFound = false;

String serverSpanId = null;
String serverTraceId = null;
String clientSpanId = null;

for (Map<String, Object> spanData : getSpans()) {
Assertions.assertNotNull(spanData);
Assertions.assertNotNull(spanData.get("spanId"));

if (spanData.get("kind").equals(SpanKind.SERVER.toString())
&& spanData.get("name").equals("nopath")) {
outsideServerFound = true;
// Server Span
serverSpanId = (String) spanData.get("spanId");
serverTraceId = (String) spanData.get("traceId");

verifyResource(spanData);

Assertions.assertEquals("nopath", spanData.get("name"));
Assertions.assertEquals(SpanKind.SERVER.toString(), spanData.get("kind"));
Assertions.assertTrue((Boolean) spanData.get("ended"));

Assertions.assertEquals(SpanId.getInvalid(), spanData.get("parent_spanId"));
Assertions.assertEquals(TraceId.getInvalid(), spanData.get("parent_traceId"));
Assertions.assertFalse((Boolean) spanData.get("parent_valid"));
Assertions.assertFalse((Boolean) spanData.get("parent_remote"));

Assertions.assertEquals("GET", spanData.get("attr_http.method"));
Assertions.assertEquals("1.1", spanData.get("attr_http.flavor"));
Assertions.assertEquals("/nopath", spanData.get("attr_http.target"));
Assertions.assertEquals(pathParamUrl.getAuthority(), spanData.get("attr_http.host"));
Assertions.assertEquals("http", spanData.get("attr_http.scheme"));
Assertions.assertEquals("/nopath", spanData.get("attr_http.route"));
Assertions.assertEquals("200", spanData.get("attr_http.status_code"));
Assertions.assertNotNull(spanData.get("attr_http.client_ip"));
Assertions.assertNotNull(spanData.get("attr_http.user_agent"));
} else if (spanData.get("kind").equals(SpanKind.CLIENT.toString())
&& spanData.get("name").equals("HTTP GET")) {
clientFound = true;
// Client span
verifyResource(spanData);

Assertions.assertEquals("HTTP GET", spanData.get("name"));
Assertions.assertEquals(SpanKind.CLIENT.toString(), spanData.get("kind"));
Assertions.assertTrue((Boolean) spanData.get("ended"));

if (serverSpanId != null) {
Assertions.assertEquals(serverSpanId, spanData.get("parent_spanId"));
}
if (serverTraceId != null) {
Assertions.assertEquals(serverTraceId, spanData.get("parent_traceId"));
}
Assertions.assertTrue((Boolean) spanData.get("parent_valid"));
Assertions.assertFalse((Boolean) spanData.get("parent_remote"));

Assertions.assertEquals("GET", spanData.get("attr_http.method"));
Assertions.assertEquals("http://localhost:8081", spanData.get("attr_http.url"));
Assertions.assertEquals("200", spanData.get("attr_http.status_code"));

clientSpanId = (String) spanData.get("spanId");
} else if (spanData.get("kind").equals(SpanKind.SERVER.toString())
&& spanData.get("name").equals("HTTP GET")) {
clientServerFound = true;
// Server span of client
verifyResource(spanData);

Assertions.assertEquals("HTTP GET", spanData.get("name"));
Assertions.assertEquals(SpanKind.SERVER.toString(), spanData.get("kind"));
Assertions.assertTrue((Boolean) spanData.get("ended"));

if (clientSpanId != null) {
Assertions.assertEquals(clientSpanId, spanData.get("parent_spanId"));
}
if (serverTraceId != null) {
Assertions.assertEquals(serverTraceId, spanData.get("parent_traceId"));
}
Assertions.assertTrue((Boolean) spanData.get("parent_valid"));
Assertions.assertTrue((Boolean) spanData.get("parent_remote"));

Assertions.assertEquals("GET", spanData.get("attr_http.method"));
Assertions.assertEquals("1.1", spanData.get("attr_http.flavor"));
Assertions.assertEquals("/", spanData.get("attr_http.target"));
Assertions.assertEquals(pathParamUrl.getAuthority(), spanData.get("attr_http.host"));
Assertions.assertEquals("http", spanData.get("attr_http.scheme"));
Assertions.assertNull(spanData.get("attr_http.route"));
Assertions.assertEquals("200", spanData.get("attr_http.status_code"));
Assertions.assertNotNull(spanData.get("attr_http.client_ip"));
Assertions.assertNotNull(spanData.get("attr_http.user_agent"));
} else {
Assertions.fail("Received an unknown Span - " + spanData.get("name"));
}
}

Assertions.assertTrue(outsideServerFound);
Assertions.assertTrue(clientFound);
Assertions.assertTrue(clientServerFound);
}

@Test
void testSlashClientPath() {
resetExporter();

given()
.contentType("application/json")
.when().get("/slashpath")
.then()
.statusCode(200)
.body("message", equalTo("No path trace"));

Awaitility.await().atMost(Duration.ofMinutes(2)).until(() -> getSpans().size() == 3);

boolean outsideServerFound = false;
boolean clientFound = false;
boolean clientServerFound = false;

String serverSpanId = null;
String serverTraceId = null;
String clientSpanId = null;

for (Map<String, Object> spanData : getSpans()) {
Assertions.assertNotNull(spanData);
Assertions.assertNotNull(spanData.get("spanId"));

if (spanData.get("kind").equals(SpanKind.SERVER.toString())
&& spanData.get("name").equals("slashpath")) {
outsideServerFound = true;
// Server Span
serverSpanId = (String) spanData.get("spanId");
serverTraceId = (String) spanData.get("traceId");

verifyResource(spanData);

Assertions.assertEquals("slashpath", spanData.get("name"));
Assertions.assertEquals(SpanKind.SERVER.toString(), spanData.get("kind"));
Assertions.assertTrue((Boolean) spanData.get("ended"));

Assertions.assertEquals(SpanId.getInvalid(), spanData.get("parent_spanId"));
Assertions.assertEquals(TraceId.getInvalid(), spanData.get("parent_traceId"));
Assertions.assertFalse((Boolean) spanData.get("parent_valid"));
Assertions.assertFalse((Boolean) spanData.get("parent_remote"));

Assertions.assertEquals("GET", spanData.get("attr_http.method"));
Assertions.assertEquals("1.1", spanData.get("attr_http.flavor"));
Assertions.assertEquals("/slashpath", spanData.get("attr_http.target"));
Assertions.assertEquals(pathParamUrl.getAuthority(), spanData.get("attr_http.host"));
Assertions.assertEquals("http", spanData.get("attr_http.scheme"));
Assertions.assertEquals("/slashpath", spanData.get("attr_http.route"));
Assertions.assertEquals("200", spanData.get("attr_http.status_code"));
Assertions.assertNotNull(spanData.get("attr_http.client_ip"));
Assertions.assertNotNull(spanData.get("attr_http.user_agent"));
} else if (spanData.get("kind").equals(SpanKind.CLIENT.toString())
&& spanData.get("name").equals("HTTP GET")) {
clientFound = true;
// Client span
verifyResource(spanData);

Assertions.assertEquals("HTTP GET", spanData.get("name"));
Assertions.assertEquals(SpanKind.CLIENT.toString(), spanData.get("kind"));
Assertions.assertTrue((Boolean) spanData.get("ended"));

if (serverSpanId != null) {
Assertions.assertEquals(serverSpanId, spanData.get("parent_spanId"));
}
if (serverTraceId != null) {
Assertions.assertEquals(serverTraceId, spanData.get("parent_traceId"));
}
Assertions.assertTrue((Boolean) spanData.get("parent_valid"));
Assertions.assertFalse((Boolean) spanData.get("parent_remote"));

Assertions.assertEquals("GET", spanData.get("attr_http.method"));
Assertions.assertEquals("http://localhost:8081/", spanData.get("attr_http.url"));
Assertions.assertEquals("200", spanData.get("attr_http.status_code"));

clientSpanId = (String) spanData.get("spanId");
} else if (spanData.get("kind").equals(SpanKind.SERVER.toString())
&& spanData.get("name").equals("HTTP GET")) {
clientServerFound = true;
// Server span of client
verifyResource(spanData);

Assertions.assertEquals("HTTP GET", spanData.get("name"));
Assertions.assertEquals(SpanKind.SERVER.toString(), spanData.get("kind"));
Assertions.assertTrue((Boolean) spanData.get("ended"));

if (clientSpanId != null) {
Assertions.assertEquals(clientSpanId, spanData.get("parent_spanId"));
}
if (serverTraceId != null) {
Assertions.assertEquals(serverTraceId, spanData.get("parent_traceId"));
}
Assertions.assertTrue((Boolean) spanData.get("parent_valid"));
Assertions.assertTrue((Boolean) spanData.get("parent_remote"));

Assertions.assertEquals("GET", spanData.get("attr_http.method"));
Assertions.assertEquals("1.1", spanData.get("attr_http.flavor"));
Assertions.assertEquals("/", spanData.get("attr_http.target"));
Assertions.assertEquals(pathParamUrl.getAuthority(), spanData.get("attr_http.host"));
Assertions.assertEquals("http", spanData.get("attr_http.scheme"));
Assertions.assertNull(spanData.get("attr_http.route"));
Assertions.assertEquals("200", spanData.get("attr_http.status_code"));
Assertions.assertNotNull(spanData.get("attr_http.client_ip"));
Assertions.assertNotNull(spanData.get("attr_http.user_agent"));
} else {
Assertions.fail("Received an unknown Span - " + spanData.get("name"));
}
}

Assertions.assertTrue(outsideServerFound);
Assertions.assertTrue(clientFound);
Assertions.assertTrue(clientServerFound);
}

@Test
void testChainedResourceTracing() {
resetExporter();
Expand Down

0 comments on commit f1d848f

Please sign in to comment.