-
Notifications
You must be signed in to change notification settings - Fork 828
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.
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 vert.x route containing dupicate segments when RoutingContext.next is used #12260
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.javaagent.instrumentation.vertx; | ||
|
||
import static io.opentelemetry.context.ContextKey.named; | ||
|
||
import io.opentelemetry.context.Context; | ||
import io.opentelemetry.context.ContextKey; | ||
import io.opentelemetry.context.ImplicitContextKeyed; | ||
|
||
public final class RouteHolder implements ImplicitContextKeyed { | ||
private static final ContextKey<RouteHolder> KEY = named("opentelemetry-vertx-route"); | ||
|
||
private String route; | ||
|
||
private RouteHolder(String route) { | ||
this.route = route; | ||
} | ||
|
||
public static Context with(Context context, String route) { | ||
if (context.get(KEY) != null) { | ||
return context; | ||
} | ||
return context.with(new RouteHolder(route)); | ||
} | ||
|
||
public static String get(Context context) { | ||
RouteHolder holder = context.get(KEY); | ||
return holder != null ? holder.route : null; | ||
} | ||
|
||
public static void set(Context context, String route) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe rename to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are similar |
||
RouteHolder holder = context.get(KEY); | ||
if (holder != null) { | ||
holder.route = route; | ||
} | ||
} | ||
|
||
@Override | ||
public Context storeInContext(Context context) { | ||
return context.with(KEY, this); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.javaagent.instrumentation.vertx; | ||
|
||
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; | ||
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; | ||
import static net.bytebuddy.matcher.ElementMatchers.isPublic; | ||
import static net.bytebuddy.matcher.ElementMatchers.named; | ||
import static net.bytebuddy.matcher.ElementMatchers.takesNoArguments; | ||
|
||
import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge; | ||
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; | ||
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; | ||
import io.vertx.ext.web.RoutingContext; | ||
import net.bytebuddy.asm.Advice; | ||
import net.bytebuddy.description.type.TypeDescription; | ||
import net.bytebuddy.matcher.ElementMatcher; | ||
|
||
public class RoutingContextInstrumentation implements TypeInstrumentation { | ||
@Override | ||
public ElementMatcher<ClassLoader> classLoaderOptimization() { | ||
return hasClassesNamed("io.vertx.ext.web.RoutingContext"); | ||
} | ||
|
||
@Override | ||
public ElementMatcher<TypeDescription> typeMatcher() { | ||
return implementsInterface(named("io.vertx.ext.web.RoutingContext")); | ||
} | ||
|
||
@Override | ||
public void transform(TypeTransformer transformer) { | ||
transformer.applyAdviceToMethod( | ||
isPublic().and(named("next")).and(takesNoArguments()), | ||
this.getClass().getName() + "$NextAdvice"); | ||
} | ||
|
||
@SuppressWarnings("unused") | ||
public static class NextAdvice { | ||
|
||
@Advice.OnMethodEnter(suppress = Throwable.class) | ||
public static void next(@Advice.This RoutingContext routingContext) { | ||
// calling next tells router to move to the next matching route | ||
// restore remembered route to remove currently matched route from it | ||
String previousRoute = RoutingContextUtil.getRoute(routingContext); | ||
if (previousRoute != null) { | ||
RouteHolder.set(Java8BytecodeBridge.currentContext(), previousRoute); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.javaagent.instrumentation.vertx; | ||
|
||
import io.opentelemetry.instrumentation.api.util.VirtualField; | ||
import io.vertx.ext.web.RoutingContext; | ||
|
||
public final class RoutingContextUtil { | ||
private static final VirtualField<RoutingContext, String> routeField = | ||
VirtualField.find(RoutingContext.class, String.class); | ||
|
||
public static void setRoute(RoutingContext routingContext, String route) { | ||
routeField.set(routingContext, route); | ||
} | ||
|
||
public static String getRoute(RoutingContext routingContext) { | ||
return routeField.get(routingContext); | ||
} | ||
|
||
private RoutingContextUtil() {} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe rename to
withIfNotExists
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or simply
init
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed to
init
becauseinit
is in a couple of otherImplicitContextKeyed
implementations