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

Use HttpRouteHolder in vertx-web instrumentation #5240

Merged
merged 2 commits into from
Jan 27, 2022
Merged

Use HttpRouteHolder in vertx-web instrumentation #5240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad731eb
Use HttpRouteHolder in vertx-web instrumentation
Jan 26, 2022
2fe5c25
Fix vertx-reactive tests
Jan 26, 2022
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
2 changes: 2 additions & 0 deletions ...vertx-reactive-3.5/javaagent/src/latestDepTest/groovy/VertxReactivePropagationTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class VertxReactivePropagationTest extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_SCHEME" "http"
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_ROUTE" "/listProducts"
}
}
span(1) {
Expand Down Expand Up @@ -164,6 +165,7 @@ class VertxReactivePropagationTest extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_SCHEME" "http"
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_ROUTE" "/listProducts"
"${TEST_REQUEST_ID_ATTRIBUTE}" requestId
}
}
Expand Down
23 changes: 1 addition & 22 deletions ...vertx-reactive-3.5/javaagent/src/latestDepTest/groovy/server/VertxRxHttpServerTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@

package server

import io.opentelemetry.api.common.AttributeKey

import io.opentelemetry.instrumentation.test.AgentTestTrait
import io.opentelemetry.instrumentation.test.base.HttpServerTest
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
import io.vertx.core.DeploymentOptions
import io.vertx.core.Promise
import io.vertx.core.Vertx
Expand All @@ -24,7 +23,6 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.INDEXED_CHILD
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT
Expand Down Expand Up @@ -64,25 +62,6 @@ class VertxRxHttpServerTest extends HttpServerTest<Vertx> implements AgentTestTr
return true
}

@Override
Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
def attributes = super.httpAttributes(endpoint)
attributes.remove(SemanticAttributes.HTTP_ROUTE)
attributes
}

@Override
String expectedServerSpanName(ServerEndpoint endpoint, String method) {
switch (endpoint) {
case PATH_PARAM:
return "/path/:id/param"
case NOT_FOUND:
return "HTTP GET"
default:
return endpoint.getPath()
}
}

@Override
boolean verifyServerSpanEndTime() {
// server spans are ended inside of the controller spans
Expand Down
2 changes: 2 additions & 0 deletions ...vertx-reactive-3.5/javaagent/src/version35Test/groovy/VertxReactivePropagationTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class VertxReactivePropagationTest extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_SCHEME" "http"
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_ROUTE" "/listProducts"
}
}
span(1) {
Expand Down Expand Up @@ -164,6 +165,7 @@ class VertxReactivePropagationTest extends AgentInstrumentationSpecification {
"$SemanticAttributes.HTTP_SCHEME" "http"
"$SemanticAttributes.HTTP_FLAVOR" "1.1"
"$SemanticAttributes.HTTP_USER_AGENT" String
"$SemanticAttributes.HTTP_ROUTE" "/listProducts"
"${TEST_REQUEST_ID_ATTRIBUTE}" requestId
}
}
Expand Down
23 changes: 1 addition & 22 deletions ...vertx-reactive-3.5/javaagent/src/version35Test/groovy/server/VertxRxHttpServerTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@

package server

import io.opentelemetry.api.common.AttributeKey

import io.opentelemetry.instrumentation.test.AgentTestTrait
import io.opentelemetry.instrumentation.test.base.HttpServerTest
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
import io.vertx.core.DeploymentOptions
import io.vertx.core.Future
import io.vertx.core.Vertx
Expand All @@ -24,7 +23,6 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.INDEXED_CHILD
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT
Expand Down Expand Up @@ -64,25 +62,6 @@ class VertxRxHttpServerTest extends HttpServerTest<Vertx> implements AgentTestTr
return true
}

@Override
Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
def attributes = super.httpAttributes(endpoint)
attributes.remove(SemanticAttributes.HTTP_ROUTE)
attributes
}

@Override
String expectedServerSpanName(ServerEndpoint endpoint, String method) {
switch (endpoint) {
case PATH_PARAM:
return "/path/:id/param"
case NOT_FOUND:
return "HTTP GET"
default:
return endpoint.getPath()
}
}

@Override
boolean verifyServerSpanEndTime() {
// server spans are ended inside of the controller spans
Expand Down
24 changes: 11 additions & 13 deletions ...n/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,19 @@

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteSource;
import io.opentelemetry.instrumentation.api.server.ServerSpan;
import io.vertx.core.Handler;
import io.vertx.ext.web.RoutingContext;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.UndeclaredThrowableException;
import java.util.concurrent.CompletionException;
import java.util.concurrent.ExecutionException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/** This is used to wrap Vert.x Handlers to provide nice user-friendly SERVER span names */
public final class RoutingContextHandlerWrapper implements Handler<RoutingContext> {

private static final Logger logger = LoggerFactory.getLogger(RoutingContextHandlerWrapper.class);

private final Handler<RoutingContext> handler;

public RoutingContextHandlerWrapper(Handler<RoutingContext> handler) {
Expand All @@ -30,25 +28,25 @@ public RoutingContextHandlerWrapper(Handler<RoutingContext> handler) {

@Override
public void handle(RoutingContext context) {
Span serverSpan = ServerSpan.fromContextOrNull(Context.current());
try {
if (serverSpan != null) {
// TODO should update SERVER span name/route using ServerSpanNaming
serverSpan.updateName(context.currentRoute().getPath());
}
} catch (RuntimeException ex) {
logger.error("Failed to update server span name with vert.x route", ex);
}
Context otelContext = Context.current();
HttpRouteHolder.updateHttpRoute(
otelContext, HttpRouteSource.CONTROLLER, RoutingContextHandlerWrapper::getRoute, context);

try {
handler.handle(context);
} catch (Throwable throwable) {
Span serverSpan = ServerSpan.fromContextOrNull(otelContext);
if (serverSpan != null) {
serverSpan.recordException(unwrapThrowable(throwable));
}
throw throwable;
}
}

private static String getRoute(Context otelContext, RoutingContext routingContext) {
return routingContext.currentRoute().getPath();
}

private static Throwable unwrapThrowable(Throwable throwable) {
if (throwable.getCause() != null
&& (throwable instanceof ExecutionException
Expand Down
26 changes: 1 addition & 25 deletions ...ion/vertx/vertx-web-3.0/testing/src/main/groovy/server/AbstractVertxHttpServerTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@

package server

import io.opentelemetry.api.common.AttributeKey

import io.opentelemetry.instrumentation.test.AgentTestTrait
import io.opentelemetry.instrumentation.test.base.HttpServerTest
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
import io.vertx.core.AbstractVerticle
import io.vertx.core.DeploymentOptions
import io.vertx.core.Vertx
Expand All @@ -18,9 +17,6 @@ import io.vertx.core.json.JsonObject
import java.util.concurrent.CompletableFuture
import java.util.concurrent.TimeUnit

import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM

abstract class AbstractVertxHttpServerTest extends HttpServerTest<Vertx> implements AgentTestTrait {
@Override
Vertx startServer(int port) {
Expand Down Expand Up @@ -60,24 +56,4 @@ abstract class AbstractVertxHttpServerTest extends HttpServerTest<Vertx> impleme
// server spans are ended inside of the controller spans
return false
}

@Override
Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
def attributes = super.httpAttributes(endpoint)
attributes.remove(SemanticAttributes.HTTP_ROUTE)
attributes
}

@Override
String expectedServerSpanName(ServerEndpoint endpoint, String method) {
switch (endpoint) {
case PATH_PARAM:
return "/path/:id/param"
case NOT_FOUND:
return "HTTP GET"
default:
return endpoint.getPath()
}
}

Comment on lines -63 to -82
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

}