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.

Sign up for GitHub

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

Fix OpenTelemetry HTTP span naming #18366

Merged
merged 1 commit into from
Jul 2, 2021
Merged

Fix OpenTelemetry HTTP span naming #18366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 7 additions & 2 deletions ...ime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/vertx/VertxTracingAdapter.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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