diff --git a/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy b/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy index 5b9caa2657..513699959d 100644 --- a/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy +++ b/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy @@ -90,7 +90,7 @@ class DropwizardTest extends HttpServerTest { @Override void serverSpan(TraceAssert trace, int index, String traceID = null, String parentID = null, String method = "GET", Long responseContentLength = null, ServerEndpoint endpoint = SUCCESS) { trace.span(index) { - name "$method ${endpoint == PATH_PARAM ? "/path/{id}/param" : endpoint.resolvePath(address).path}" + name "${endpoint == PATH_PARAM ? "/path/{id}/param" : endpoint.resolvePath(address).path}" kind SERVER errored endpoint.errored if (parentID != null) { diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java index 73b2931e43..8593c50da7 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java @@ -64,7 +64,7 @@ public class JaxRsAnnotationsTracer extends BaseTracer { Map classMap = spanNames.get(target); if (classMap == null) { - spanNames.putIfAbsent(target, new ConcurrentHashMap()); + spanNames.putIfAbsent(target, new ConcurrentHashMap<>()); classMap = spanNames.get(target); // classMap should not be null at this point because we have a // strong reference to target and don't manually clear the map. @@ -96,7 +96,7 @@ public class JaxRsAnnotationsTracer extends BaseTracer { } } } - spanName = buildSpanName(httpMethod, classPath, methodPath); + spanName = buildSpanName(classPath, methodPath); classMap.put(method, spanName); } @@ -155,20 +155,17 @@ public class JaxRsAnnotationsTracer extends BaseTracer { return null; } - private String buildSpanName(String httpMethod, Path classPath, Path methodPath) { + private String buildSpanName(Path classPath, Path methodPath) { String spanName; StringBuilder spanNameBuilder = new StringBuilder(); - if (httpMethod != null) { - spanNameBuilder.append(httpMethod); - spanNameBuilder.append(" "); - } boolean skipSlash = false; if (classPath != null) { - if (!classPath.value().startsWith("/")) { + String classPathValue = classPath.value(); + if (!classPathValue.startsWith("/")) { spanNameBuilder.append("/"); } - spanNameBuilder.append(classPath.value()); - skipSlash = classPath.value().endsWith("/"); + spanNameBuilder.append(classPathValue); + skipSlash = classPathValue.endsWith("/") || classPathValue.isEmpty(); } if (methodPath != null) { @@ -184,6 +181,7 @@ public class JaxRsAnnotationsTracer extends BaseTracer { } spanName = spanNameBuilder.toString().trim(); + return spanName; } diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsAnnotationsInstrumentationTest.groovy b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsAnnotationsInstrumentationTest.groovy index 1b3b2db484..e5d7530f72 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsAnnotationsInstrumentationTest.groovy +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsAnnotationsInstrumentationTest.groovy @@ -16,6 +16,7 @@ import javax.ws.rs.OPTIONS import javax.ws.rs.POST import javax.ws.rs.PUT import javax.ws.rs.Path +import spock.lang.Unroll abstract class JaxRsAnnotationsInstrumentationTest extends AgentTestRunner { @@ -32,7 +33,7 @@ abstract class JaxRsAnnotationsInstrumentationTest extends AgentTestRunner { assertTraces(1) { trace(0, 1) { span(0) { - name "POST /a" + name "/a" attributes { } } @@ -40,7 +41,8 @@ abstract class JaxRsAnnotationsInstrumentationTest extends AgentTestRunner { } } - def "span named '#name' from annotations on class when is not root span"() { + @Unroll + def "span named '#paramName' from annotations on class when is not root span"() { setup: def startingCacheSize = spanNames.size() runUnderServerTrace("test") { @@ -78,53 +80,53 @@ abstract class JaxRsAnnotationsInstrumentationTest extends AgentTestRunner { spanNames.get(obj.class).size() == 1 where: - paramName | obj - "/a" | new Jax() { + paramName | obj + "/a" | new Jax() { @Path("/a") void call() { } } - "GET /b" | new Jax() { + "/b" | new Jax() { @GET @Path("/b") void call() { } } - "POST /interface/c" | new InterfaceWithPath() { + "/interface/c" | new InterfaceWithPath() { @POST @Path("/c") void call() { } } - "HEAD /interface" | new InterfaceWithPath() { + "/interface" | new InterfaceWithPath() { @HEAD void call() { } } - "POST /abstract/d" | new AbstractClassWithPath() { + "/abstract/d" | new AbstractClassWithPath() { @POST @Path("/d") void call() { } } - "PUT /abstract" | new AbstractClassWithPath() { + "/abstract" | new AbstractClassWithPath() { @PUT void call() { } } - "OPTIONS /child/e" | new ChildClassWithPath() { + "/child/e" | new ChildClassWithPath() { @OPTIONS @Path("/e") void call() { } } - "DELETE /child/call" | new ChildClassWithPath() { + "/child/call" | new ChildClassWithPath() { @DELETE void call() { } } - "POST /child/call" | new ChildClassWithPath() - "GET /child/call" | new JavaInterfaces.ChildClassOnInterface() + "/child/call" | new ChildClassWithPath() + "/child/call" | new JavaInterfaces.ChildClassOnInterface() // TODO: uncomment when we drop support for Java 7 // "GET /child/invoke" | new JavaInterfaces.DefaultChildClassOnInterface() diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsFilterTest.groovy b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsFilterTest.groovy index b6a75579d4..2bcda03196 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsFilterTest.groovy +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsFilterTest.groovy @@ -65,21 +65,21 @@ abstract class JaxRsFilterTest extends AgentTestRunner { } where: - resource | abortNormal | abortPrematch | parentSpanName | controllerName | expectedResponse - "/test/hello/bob" | false | false | "POST /test/hello/{name}" | "Test1.hello" | "Test1 bob!" - "/test2/hello/bob" | false | false | "POST /test2/hello/{name}" | "Test2.hello" | "Test2 bob!" - "/test3/hi/bob" | false | false | "POST /test3/hi/{name}" | "Test3.hello" | "Test3 bob!" + resource | abortNormal | abortPrematch | parentSpanName | controllerName | expectedResponse + "/test/hello/bob" | false | false | "/test/hello/{name}" | "Test1.hello" | "Test1 bob!" + "/test2/hello/bob" | false | false | "/test2/hello/{name}" | "Test2.hello" | "Test2 bob!" + "/test3/hi/bob" | false | false | "/test3/hi/{name}" | "Test3.hello" | "Test3 bob!" // Resteasy and Jersey give different resource class names for just the below case // Resteasy returns "SubResource.class" // Jersey returns "Test1.class - // "/test/hello/bob" | true | false | "POST /test/hello/{name}" | "Test1.hello" | "Aborted" + // "/test/hello/bob" | true | false | "/test/hello/{name}" | "Test1.hello" | "Aborted" - "/test2/hello/bob" | true | false | "POST /test2/hello/{name}" | "Test2.hello" | "Aborted" - "/test3/hi/bob" | true | false | "POST /test3/hi/{name}" | "Test3.hello" | "Aborted" - "/test/hello/bob" | false | true | null | "PrematchRequestFilter.filter" | "Aborted Prematch" - "/test2/hello/bob" | false | true | null | "PrematchRequestFilter.filter" | "Aborted Prematch" - "/test3/hi/bob" | false | true | null | "PrematchRequestFilter.filter" | "Aborted Prematch" + "/test2/hello/bob" | true | false | "/test2/hello/{name}" | "Test2.hello" | "Aborted" + "/test3/hi/bob" | true | false | "/test3/hi/{name}" | "Test3.hello" | "Aborted" + "/test/hello/bob" | false | true | null | "PrematchRequestFilter.filter" | "Aborted Prematch" + "/test2/hello/bob" | false | true | null | "PrematchRequestFilter.filter" | "Aborted Prematch" + "/test3/hi/bob" | false | true | null | "PrematchRequestFilter.filter" | "Aborted Prematch" } def "test nested call"() { @@ -115,8 +115,8 @@ abstract class JaxRsFilterTest extends AgentTestRunner { } where: - resource | parentResourceName | controller1Name | expectedResponse - "/test3/nested" | "POST /test3/nested" | "Test3.nested" | "Test3 nested!" + resource | parentResourceName | controller1Name | expectedResponse + "/test3/nested" | "/test3/nested" | "Test3.nested" | "Test3 nested!" } @Provider diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy index de3a048f7b..0767632030 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy @@ -161,7 +161,7 @@ abstract class JaxRsHttpServerTest extends HttpServerTest { int statusCode, String query) { trace.span(index) { - name method + " /" + path + name path kind SERVER errored isError if (parentID != null) {