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

Jump to bottom

Update webflux to Instrumenter API (and improvements/simplifications) #3798

Merged
merged 17 commits into from
Aug 13, 2021
Merged

Update webflux to Instrumenter API (and improvements/simplifications) #3798

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
c5ebca5
Update Webflux to Instrumenter API
trask Aug 8, 2021
6695abc
Change webflux handler to match other handlers
trask Aug 8, 2021
bc9f077
Further simplification
trask Aug 8, 2021
e56b099
Fix Mono failure tests
trask Aug 8, 2021
d3c4a05
Comment
trask Aug 8, 2021
9df4a14
FLAKY
trask Aug 8, 2021
e95e964
Comment
trask Aug 8, 2021
c152c6d
Revert "FLAKY"
trask Aug 8, 2021
26f36fb
Merge remote-tracking branch 'upstream/main' into update-webflux-to-i…
trask Aug 9, 2021
d5a9bd8
Use extractors!
trask Aug 9, 2021
72a27e3
Renames
trask Aug 9, 2021
df2cfa8
Fix comment
trask Aug 10, 2021
8f6cd24
Update instrumentation/spring/spring-webflux-5.0/javaagent/src/main/j…
trask Aug 10, 2021
111a885
Merge remote-tracking branch 'upstream/main' into update-webflux-to-i…
trask Aug 11, 2021
04e4bf3
Keep extra cancellation behavior for now
trask Aug 11, 2021
821b1b5
Revert "Keep extra cancellation behavior for now"
trask Aug 11, 2021
20db675
Merge remote-tracking branch 'upstream/main' into update-webflux-to-i…
trask Aug 12, 2021
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
Prev Previous commit
Next Next commit
Further simplification
  • Loading branch information
@trask
trask committed Aug 8, 2021
commit bc9f0771a5d0ff63bbf8bfb9f57454d180588b0b
2 changes: 0 additions & 2 deletions ...in/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/server/AdviceUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@

public class AdviceUtils {

public static final String CONTEXT_ATTRIBUTE = AdviceUtils.class.getName() + ".Context";

public static String spanNameForHandler(Object handler) {
String className = ClassNames.simpleName(handler.getClass());
Copy link
Contributor

Choose a reason for hiding this comment

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

Although this part wasn't changed in this pr getting a span name from an object is also used in other instrumentations so it would be nice if someone figured out how to do this properly. The problem here is that handler could be anything. For example SpringWebFluxTestApplication when you add

  @Component("greetingHandlerFunction")
  @Scope(value = "prototype", proxyMode = ScopedProxyMode.TARGET_CLASS)
  static class GreetingHandlerFunction implements HandlerFunction<ServerResponse> {
    private final GreetingHandler greetingHandler

    GreetingHandlerFunction(GreetingHandler greetingHandler) {
      this.greetingHandler = greetingHandler
    }

    @Override
    Mono<ServerResponse> handle(ServerRequest request) {
      return greetingHandler.defaultGreet()
    }
  }

and change the start of greetRouterFunction to

  @Bean
  RouterFunction<ServerResponse> greetRouterFunction(GreetingHandler greetingHandler, HandlerFunction<ServerResponse> greetingHandlerFunction) {
    return route(GET("/greet"), greetingHandlerFunction

Then span name is
SpringWebFluxTestApplication$GreetingHandlerFunction$$EnhancerBySpringCGLIB$$d662c0c3.handle
When you change scope annotation to
@Scope(value = "prototype", proxyMode = ScopedProxyMode.INTERFACES)
then span name is
$Proxy119.handle
If we'd assume that handler is a spring bean we could try to unwrap it with something like spring-projects/spring-framework@efe3a35

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 I created issue for this #3813

int lambdaIdx = className.indexOf("$$Lambda$");
Expand Down
51 changes: 0 additions & 51 deletions ...try/javaagent/instrumentation/spring/webflux/server/DispatcherHandlerInstrumentation.java
View file
Open in desktop

This file was deleted.

6 changes: 1 addition & 5 deletions ...emetry/javaagent/instrumentation/spring/webflux/server/HandlerAdapterInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,7 @@ public static void methodEnter(
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {

Context parentContext = exchange.getAttribute(AdviceUtils.CONTEXT_ATTRIBUTE);

if (parentContext == null) {
return;
}
Context parentContext = Context.current();

Span serverSpan = ServerSpan.fromContextOrNull(parentContext);

Expand Down
9 changes: 3 additions & 6 deletions .../opentelemetry/javaagent/instrumentation/spring/webflux/server/RouteOnSuccessOrError.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import java.util.regex.Pattern;
import org.springframework.web.reactive.function.server.HandlerFunction;
import org.springframework.web.reactive.function.server.RouterFunction;
import org.springframework.web.reactive.function.server.ServerRequest;

public class RouteOnSuccessOrError implements BiConsumer<HandlerFunction<?>, Throwable> {

Expand All @@ -22,20 +21,18 @@ public class RouteOnSuccessOrError implements BiConsumer<HandlerFunction<?>, Thr
private static final Pattern METHOD_REGEX =
Pattern.compile("^(GET|HEAD|POST|PUT|DELETE|CONNECT|OPTIONS|TRACE|PATCH) ");

private final RouterFunction routerFunction;
private final ServerRequest serverRequest;
private final RouterFunction<?> routerFunction;

public RouteOnSuccessOrError(RouterFunction routerFunction, ServerRequest serverRequest) {
public RouteOnSuccessOrError(RouterFunction<?> routerFunction) {
this.routerFunction = routerFunction;
this.serverRequest = serverRequest;
}

@Override
public void accept(HandlerFunction<?> handler, Throwable throwable) {
if (handler != null) {
String predicateString = parsePredicateString();
if (predicateString != null) {
Context context = (Context) serverRequest.attributes().get(AdviceUtils.CONTEXT_ATTRIBUTE);
Context context = Context.current();
if (context != null) {
Span serverSpan = ServerSpan.fromContextOrNull(context);
if (serverSpan != null) {
Expand Down
6 changes: 2 additions & 4 deletions ...emetry/javaagent/instrumentation/spring/webflux/server/RouterFunctionInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import net.bytebuddy.matcher.ElementMatcher;
import org.springframework.web.reactive.function.server.HandlerFunction;
import org.springframework.web.reactive.function.server.RouterFunction;
import org.springframework.web.reactive.function.server.ServerRequest;
import reactor.core.publisher.Mono;

public class RouterFunctionInstrumentation implements TypeInstrumentation {
Expand Down Expand Up @@ -64,12 +63,11 @@ public static class RouteAdvice {

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void methodExit(
@Advice.This RouterFunction thiz,
@Advice.Argument(0) ServerRequest serverRequest,
@Advice.This RouterFunction<?> thiz,
@Advice.Return(readOnly = false) Mono<HandlerFunction<?>> result,
@Advice.Thrown Throwable throwable) {
if (throwable == null) {
result = result.doOnSuccessOrError(new RouteOnSuccessOrError(thiz, serverRequest));
result = result.doOnSuccessOrError(new RouteOnSuccessOrError(thiz));
}
}
}
Expand Down
5 changes: 1 addition & 4 deletions ...y/javaagent/instrumentation/spring/webflux/server/WebfluxServerInstrumentationModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ public WebfluxServerInstrumentationModule() {

@Override
public List<TypeInstrumentation> typeInstrumentations() {
return asList(
new DispatcherHandlerInstrumentation(),
new HandlerAdapterInstrumentation(),
new RouterFunctionInstrumentation());
return asList(new HandlerAdapterInstrumentation(), new RouterFunctionInstrumentation());
}
}