Do not set the http.route attribute in JSF instrumentations by default (#5819)
* Do not set the http.route attribute in JSF instrumentations by default * code review comments
This commit is contained in:
parent
784f4b6704
commit
4d34d90437
|
@ -5,26 +5,34 @@
|
|||
|
||||
package io.opentelemetry.javaagent.instrumentation.jsf;
|
||||
|
||||
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteGetter;
|
||||
import io.opentelemetry.api.trace.Span;
|
||||
import io.opentelemetry.context.Context;
|
||||
import io.opentelemetry.instrumentation.api.server.ServerSpan;
|
||||
import io.opentelemetry.javaagent.bootstrap.servlet.ServletContextPath;
|
||||
import javax.faces.component.UIViewRoot;
|
||||
import javax.faces.context.FacesContext;
|
||||
|
||||
public class JsfServerSpanNaming {
|
||||
public final class JsfServerSpanNaming {
|
||||
|
||||
public static final HttpRouteGetter<FacesContext> SERVER_SPAN_NAME =
|
||||
(context, facesContext) -> {
|
||||
UIViewRoot uiViewRoot = facesContext.getViewRoot();
|
||||
if (uiViewRoot == null) {
|
||||
return null;
|
||||
}
|
||||
public static void updateViewName(Context context, FacesContext facesContext) {
|
||||
// just update the server span name, without touching the http.route
|
||||
Span serverSpan = ServerSpan.fromContextOrNull(context);
|
||||
if (serverSpan == null) {
|
||||
return;
|
||||
}
|
||||
|
||||
// JSF spec 7.6.2
|
||||
// view id is a context relative path to the web application resource that produces the
|
||||
// view, such as a JSP page or a Facelets page.
|
||||
String viewId = uiViewRoot.getViewId();
|
||||
return ServletContextPath.prepend(context, viewId);
|
||||
};
|
||||
UIViewRoot uiViewRoot = facesContext.getViewRoot();
|
||||
if (uiViewRoot == null) {
|
||||
return;
|
||||
}
|
||||
|
||||
// JSF spec 7.6.2
|
||||
// view id is a context relative path to the web application resource that produces the
|
||||
// view, such as a JSP page or a Facelets page.
|
||||
String viewId = uiViewRoot.getViewId();
|
||||
String name = ServletContextPath.prepend(context, viewId);
|
||||
serverSpan.updateName(name);
|
||||
}
|
||||
|
||||
private JsfServerSpanNaming() {}
|
||||
}
|
||||
|
|
|
@ -8,6 +8,7 @@ import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
|
|||
import io.opentelemetry.instrumentation.test.asserts.TraceAssert
|
||||
import io.opentelemetry.instrumentation.test.base.HttpServerTestTrait
|
||||
import io.opentelemetry.sdk.trace.data.SpanData
|
||||
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
|
||||
import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpRequest
|
||||
import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpResponse
|
||||
import io.opentelemetry.testing.internal.armeria.common.HttpData
|
||||
|
@ -82,7 +83,7 @@ abstract class BaseJsfTest extends AgentInstrumentationSpecification implements
|
|||
@Unroll
|
||||
def "test #path"() {
|
||||
setup:
|
||||
AggregatedHttpResponse response = client.get(address.resolve("hello.jsf").toString()).aggregate().join()
|
||||
AggregatedHttpResponse response = client.get(address.resolve(path).toString()).aggregate().join()
|
||||
|
||||
expect:
|
||||
response.status().code() == 200
|
||||
|
@ -95,12 +96,28 @@ abstract class BaseJsfTest extends AgentInstrumentationSpecification implements
|
|||
name getContextPath() + "/hello.xhtml"
|
||||
kind SpanKind.SERVER
|
||||
hasNoParent()
|
||||
attributes {
|
||||
"$SemanticAttributes.NET_TRANSPORT" SemanticAttributes.NetTransportValues.IP_TCP
|
||||
"$SemanticAttributes.NET_PEER_PORT" Long
|
||||
"$SemanticAttributes.NET_PEER_IP" "127.0.0.1"
|
||||
"$SemanticAttributes.HTTP_METHOD" "GET"
|
||||
"$SemanticAttributes.HTTP_SCHEME" "http"
|
||||
"$SemanticAttributes.HTTP_HOST" { it == "localhost" || it == "localhost:$port" }
|
||||
"$SemanticAttributes.HTTP_TARGET" "/jetty-context/" + path
|
||||
"$SemanticAttributes.HTTP_USER_AGENT" TEST_USER_AGENT
|
||||
"$SemanticAttributes.HTTP_FLAVOR" SemanticAttributes.HttpFlavorValues.HTTP_1_1
|
||||
"$SemanticAttributes.HTTP_STATUS_CODE" 200
|
||||
"$SemanticAttributes.HTTP_ROUTE" "/jetty-context/" + route
|
||||
"$SemanticAttributes.HTTP_CLIENT_IP" { it == null || it == TEST_CLIENT_IP }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
where:
|
||||
path << ['hello.jsf', 'faces/hello.xhtml']
|
||||
path | route
|
||||
"hello.jsf" | "*.jsf"
|
||||
"faces/hello.xhtml" | "faces/*"
|
||||
}
|
||||
|
||||
def "test greeting"() {
|
||||
|
|
|
@ -5,12 +5,10 @@
|
|||
|
||||
package io.opentelemetry.javaagent.instrumentation.mojarra;
|
||||
|
||||
import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteSource.CONTROLLER;
|
||||
import static io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge.currentContext;
|
||||
import static net.bytebuddy.matcher.ElementMatchers.named;
|
||||
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
|
||||
|
||||
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder;
|
||||
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
|
||||
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
|
||||
import io.opentelemetry.javaagent.instrumentation.jsf.JsfServerSpanNaming;
|
||||
|
@ -38,8 +36,7 @@ public class RestoreViewPhaseInstrumentation implements TypeInstrumentation {
|
|||
|
||||
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
|
||||
public static void onExit(@Advice.Argument(0) FacesContext facesContext) {
|
||||
HttpRouteHolder.updateHttpRoute(
|
||||
currentContext(), CONTROLLER, JsfServerSpanNaming.SERVER_SPAN_NAME, facesContext);
|
||||
JsfServerSpanNaming.updateViewName(currentContext(), facesContext);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -5,12 +5,10 @@
|
|||
|
||||
package io.opentelemetry.javaagent.instrumentation.myfaces;
|
||||
|
||||
import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteSource.CONTROLLER;
|
||||
import static io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge.currentContext;
|
||||
import static net.bytebuddy.matcher.ElementMatchers.named;
|
||||
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
|
||||
|
||||
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder;
|
||||
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
|
||||
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
|
||||
import io.opentelemetry.javaagent.instrumentation.jsf.JsfServerSpanNaming;
|
||||
|
@ -38,8 +36,7 @@ public class RestoreViewExecutorInstrumentation implements TypeInstrumentation {
|
|||
|
||||
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
|
||||
public static void onExit(@Advice.Argument(0) FacesContext facesContext) {
|
||||
HttpRouteHolder.updateHttpRoute(
|
||||
currentContext(), CONTROLLER, JsfServerSpanNaming.SERVER_SPAN_NAME, facesContext);
|
||||
JsfServerSpanNaming.updateViewName(currentContext(), facesContext);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -48,7 +48,7 @@ class AttributesAssert {
|
|||
|
||||
def methodMissing(String name, args) {
|
||||
if (args.length == 0) {
|
||||
throw new IllegalArgumentException(args.toString())
|
||||
throw new IllegalArgumentException("Attribute $name needs to be provided expected value; zero args were passed: $args")
|
||||
}
|
||||
attribute(name, args[0])
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue