From 98a4ef8069de996fb33fc112b479b694fb491659 Mon Sep 17 00:00:00 2001 From: NHebrard Date: Mon, 4 Jan 2021 11:42:21 +0100 Subject: [PATCH 1/2] Add onCreate callback to WebFluxSpanDecorator --- .../WebFluxTracingAutoConfigurationTest.java | 6 ++-- .../spring/web/webfilter/TracingOperator.java | 18 ++++++++-- .../web/webfilter/WebFluxSpanDecorator.java | 26 +++++++++++++-- .../web/webfilter/TracingSubscriberTest.java | 33 ++++++++++++------- 4 files changed, 63 insertions(+), 20 deletions(-) diff --git a/opentracing-spring-web-starter/src/test/java/io/opentracing/contrib/spring/web/starter/WebFluxTracingAutoConfigurationTest.java b/opentracing-spring-web-starter/src/test/java/io/opentracing/contrib/spring/web/starter/WebFluxTracingAutoConfigurationTest.java index 901d6828..d52e001b 100644 --- a/opentracing-spring-web-starter/src/test/java/io/opentracing/contrib/spring/web/starter/WebFluxTracingAutoConfigurationTest.java +++ b/opentracing-spring-web-starter/src/test/java/io/opentracing/contrib/spring/web/starter/WebFluxTracingAutoConfigurationTest.java @@ -1,5 +1,5 @@ /** - * Copyright 2016-2019 The OpenTracing Authors + * Copyright 2016-2021 The OpenTracing Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -126,8 +126,8 @@ public void testDecoratedProviderIsUsed() { Awaitility.await().until(reportedSpansSize(), IsEqual.equalTo(1)); assertThat(mockTracer.finishedSpans()).hasSize(1); - assertThat(Mockito.mockingDetails(mockDecorator1).getInvocations()).hasSize(2); - assertThat(Mockito.mockingDetails(mockDecorator2).getInvocations()).hasSize(2); + assertThat(Mockito.mockingDetails(mockDecorator1).getInvocations()).hasSize(3); + assertThat(Mockito.mockingDetails(mockDecorator2).getInvocations()).hasSize(3); } @Test diff --git a/opentracing-spring-web/src/main/java/io/opentracing/contrib/spring/web/webfilter/TracingOperator.java b/opentracing-spring-web/src/main/java/io/opentracing/contrib/spring/web/webfilter/TracingOperator.java index 68bf55b7..c30fa180 100644 --- a/opentracing-spring-web/src/main/java/io/opentracing/contrib/spring/web/webfilter/TracingOperator.java +++ b/opentracing-spring-web/src/main/java/io/opentracing/contrib/spring/web/webfilter/TracingOperator.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. Copyright 2019 The OpenTracing Authors. + * Copyright 2013-2021 the original author or authors. Copyright 2019 The OpenTracing Authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,8 @@ import io.opentracing.Tracer; import io.opentracing.propagation.Format; import io.opentracing.tag.Tags; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.web.server.ServerWebExchange; import reactor.core.CoreSubscriber; @@ -36,6 +38,9 @@ * @author Csaba Kos */ class TracingOperator extends MonoOperator { + + private static final Log LOG = LogFactory.getLog(TracingOperator.class); + private final Tracer tracer; private final ServerWebExchange exchange; private final List spanDecorators; @@ -70,8 +75,17 @@ public void subscribe(final CoreSubscriber subscriber) { .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER) .start(); + exchange.getAttributes().put(TracingWebFilter.SERVER_SPAN_CONTEXT, span.context()); + + for (WebFluxSpanDecorator spanDecorator : spanDecorators) { + try { + spanDecorator.onCreate(exchange, span); + } catch (RuntimeException exDecorator) { + LOG.error("Exception during decorating span", exDecorator); + } + } + try (final Scope scope = tracer.scopeManager().activate(span)) { - exchange.getAttributes().put(TracingWebFilter.SERVER_SPAN_CONTEXT, span.context()); source.subscribe(new TracingSubscriber(subscriber, exchange, context, span, spanDecorators)); } } diff --git a/opentracing-spring-web/src/main/java/io/opentracing/contrib/spring/web/webfilter/WebFluxSpanDecorator.java b/opentracing-spring-web/src/main/java/io/opentracing/contrib/spring/web/webfilter/WebFluxSpanDecorator.java index 9c237f9e..d46cb177 100644 --- a/opentracing-spring-web/src/main/java/io/opentracing/contrib/spring/web/webfilter/WebFluxSpanDecorator.java +++ b/opentracing-spring-web/src/main/java/io/opentracing/contrib/spring/web/webfilter/WebFluxSpanDecorator.java @@ -1,5 +1,5 @@ /** - * Copyright 2016-2019 The OpenTracing Authors + * Copyright 2016-2021 The OpenTracing Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -34,8 +34,17 @@ public interface WebFluxSpanDecorator { /** - * Decorate span before {@code .onSubscribe()} is called. This is called right after span in created. Span - * context is already present in request attributes with name {@link TracingWebFilter#SERVER_SPAN_CONTEXT}. + * Decorate span right after its creation. + * Span context is already present in request attributes with name {@link TracingWebFilter#SERVER_SPAN_CONTEXT}. + * + * @param exchange web exchange + * @param span span to decorate + */ + void onCreate(ServerWebExchange exchange, Span span); + + /** + * Decorate span when {@code .onSubscribe()} is called. + * Span context is already present in request attributes with name {@link TracingWebFilter#SERVER_SPAN_CONTEXT}. * * @param exchange web exchange * @param span span to decorate @@ -67,6 +76,11 @@ public interface WebFluxSpanDecorator { class StandardTags implements WebFluxSpanDecorator { static final String COMPONENT_NAME = "java-spring-webflux"; + @Override + public void onCreate(ServerWebExchange exchange, Span span) { + // No-op + } + @Override public void onRequest(final ServerWebExchange exchange, final Span span) { Tags.COMPONENT.set(span, COMPONENT_NAME); @@ -112,6 +126,12 @@ private Map logsForException(final Throwable throwable) { * Adds tags from WebFlux handler to span. */ class WebFluxTags implements WebFluxSpanDecorator { + + @Override + public void onCreate(ServerWebExchange exchange, Span span) { + // No-op + } + @Override public void onRequest(final ServerWebExchange exchange, final Span span) { // No-op diff --git a/opentracing-spring-web/src/test/java/io/opentracing/contrib/spring/web/webfilter/TracingSubscriberTest.java b/opentracing-spring-web/src/test/java/io/opentracing/contrib/spring/web/webfilter/TracingSubscriberTest.java index 40808e57..16ef6a2c 100644 --- a/opentracing-spring-web/src/test/java/io/opentracing/contrib/spring/web/webfilter/TracingSubscriberTest.java +++ b/opentracing-spring-web/src/test/java/io/opentracing/contrib/spring/web/webfilter/TracingSubscriberTest.java @@ -1,5 +1,5 @@ /** - * Copyright 2016-2020 The OpenTracing Authors + * Copyright 2016-2021 The OpenTracing Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -20,6 +20,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import org.springframework.http.HttpMethod; @@ -38,8 +39,8 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.verifyNoMoreInteractions; @RunWith(MockitoJUnitRunner.class) public class TracingSubscriberTest { @@ -91,9 +92,12 @@ public void testSpanIsFinishedWhenMonoHasCompleted() throws InterruptedException assertEquals(SignalType.ON_COMPLETE, finalSignalType.get()); Span finishedSpan = tracer.finishedSpans().get(0); - verify(spanDecorator).onRequest(exchange, finishedSpan); - verify(spanDecorator).onResponse(exchange, finishedSpan); - verify(spanDecorator, never()).onError(any(ServerWebExchange.class), any(Throwable.class), any(Span.class)); + + InOrder inOrder = inOrder(spanDecorator); + inOrder.verify(spanDecorator).onCreate(exchange, finishedSpan); + inOrder.verify(spanDecorator).onRequest(exchange, finishedSpan); + inOrder.verify(spanDecorator).onResponse(exchange, finishedSpan); + verifyNoMoreInteractions(spanDecorator); } @Test @@ -122,9 +126,12 @@ public void testSpanIsFinishedWhenMonoHasError() throws InterruptedException { assertEquals(SignalType.ON_ERROR, finalSignalType.get()); Span finishedSpan = tracer.finishedSpans().get(0); - verify(spanDecorator).onRequest(exchange, finishedSpan); - verify(spanDecorator).onError(eq(exchange), any(Throwable.class), eq(finishedSpan)); - verify(spanDecorator, never()).onResponse(any(ServerWebExchange.class), any(Span.class)); + + InOrder inOrder = inOrder(spanDecorator); + inOrder.verify(spanDecorator).onCreate(exchange, finishedSpan); + inOrder.verify(spanDecorator).onRequest(exchange, finishedSpan); + inOrder.verify(spanDecorator).onError(eq(exchange), any(Throwable.class), eq(finishedSpan)); + verifyNoMoreInteractions(spanDecorator); } @Test @@ -168,8 +175,10 @@ public void testSpanIsFinishedWhenMonoHasCanceled() throws InterruptedException assertEquals(SignalType.CANCEL, finalSignalType.get()); Span finishedSpan = tracer.finishedSpans().get(0); - verify(spanDecorator).onRequest(exchange, finishedSpan); - verify(spanDecorator, never()).onError(any(ServerWebExchange.class), any(Throwable.class), any(Span.class)); - verify(spanDecorator, never()).onResponse(any(ServerWebExchange.class), any(Span.class)); + + InOrder inOrder = inOrder(spanDecorator); + inOrder.verify(spanDecorator).onCreate(exchange, finishedSpan); + inOrder.verify(spanDecorator).onRequest(exchange, finishedSpan); + verifyNoMoreInteractions(spanDecorator); } } From 04f83ce0f10c533670d222053686e3a31cf6a28b Mon Sep 17 00:00:00 2001 From: NHebrard Date: Mon, 4 Jan 2021 12:08:03 +0100 Subject: [PATCH 2/2] Make onCreate default --- .../spring/web/webfilter/WebFluxSpanDecorator.java | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/opentracing-spring-web/src/main/java/io/opentracing/contrib/spring/web/webfilter/WebFluxSpanDecorator.java b/opentracing-spring-web/src/main/java/io/opentracing/contrib/spring/web/webfilter/WebFluxSpanDecorator.java index d46cb177..77201f3c 100644 --- a/opentracing-spring-web/src/main/java/io/opentracing/contrib/spring/web/webfilter/WebFluxSpanDecorator.java +++ b/opentracing-spring-web/src/main/java/io/opentracing/contrib/spring/web/webfilter/WebFluxSpanDecorator.java @@ -40,7 +40,7 @@ public interface WebFluxSpanDecorator { * @param exchange web exchange * @param span span to decorate */ - void onCreate(ServerWebExchange exchange, Span span); + default void onCreate(ServerWebExchange exchange, Span span) { } /** * Decorate span when {@code .onSubscribe()} is called. @@ -76,11 +76,6 @@ public interface WebFluxSpanDecorator { class StandardTags implements WebFluxSpanDecorator { static final String COMPONENT_NAME = "java-spring-webflux"; - @Override - public void onCreate(ServerWebExchange exchange, Span span) { - // No-op - } - @Override public void onRequest(final ServerWebExchange exchange, final Span span) { Tags.COMPONENT.set(span, COMPONENT_NAME); @@ -126,12 +121,6 @@ private Map logsForException(final Throwable throwable) { * Adds tags from WebFlux handler to span. */ class WebFluxTags implements WebFluxSpanDecorator { - - @Override - public void onCreate(ServerWebExchange exchange, Span span) { - // No-op - } - @Override public void onRequest(final ServerWebExchange exchange, final Span span) { // No-op