Review changes

Signed-off-by: David Schwilk <david.schwilk@bosch.io>
eclipse-ditto · Oct 26, 2022 · 541517c · 541517c
1 parent 3d752da
commit 541517c
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 28 deletions.
diff --git a/.../java/org/eclipse/ditto/gateway/service/endpoints/directives/RequestTracingDirective.java b/.../java/org/eclipse/ditto/gateway/service/endpoints/directives/RequestTracingDirective.java
@@ -56,18 +56,7 @@ public final class RequestTracingDirective {
     private static final DittoLogger LOGGER = DittoLoggerFactory.getLogger(RequestTracingDirective.class);
 
     private RequestTracingDirective() {
-        super();
-    }
-
-    /**
-     * Return new instance of {@code RequestTracingDirective} which disables tracing for operations with the specified
-     * set of operation names.
-     *
-     * @return the new instance.
-     * @throws NullPointerException if {@code disabledSpanOperationNames} is {@code null}.
-     */
-    public static RequestTracingDirective newInstanceWithDisabled() {
-        return new RequestTracingDirective();
+        throw new AssertionError();
     }
 
     /**
@@ -81,12 +70,14 @@ public static RequestTracingDirective newInstanceWithDisabled() {
      * @return the new Route wrapping {@code inner} with tracing.
      * @throws NullPointerException if {@code inner} is {@code null}.
      */
-    public Route traceRequest(final Supplier<Route> innerRouteSupplier, @Nullable final CharSequence correlationId) {
+    public static Route traceRequest(final Supplier<Route> innerRouteSupplier,
+            @Nullable final CharSequence correlationId) {
+
         checkNotNull(innerRouteSupplier, "innerRouteSupplier");
         return extractRequest(request -> {
             final Route result;
             @Nullable final var operationName = tryToResolveSpanOperationName(request, correlationId);
-            if (isTracingDisabledForOperationName(operationName)) {
+            if (null == operationName) {
                 result = innerRouteSupplier.get();
             } else {
                 result = getRouteWithEnabledTracing(
@@ -115,7 +106,7 @@ private static SpanOperationName tryToResolveSpanOperationName(
 
     private static SpanOperationName resolveSpanOperationName(final HttpRequest httpRequest) {
         return SpanOperationName.of(
-                MessageFormat.format("{0} {1}", getTraceUri(httpRequest), getRequestMethodName(httpRequest))
+                MessageFormat.format("{0} {1}", getRequestMethodName(httpRequest), getTraceUri(httpRequest))
         );
     }
 
@@ -135,10 +126,6 @@ private static String getRequestMethodName(final HttpRequest httpRequest) {
         return httpMethod.name();
     }
 
-    private static boolean isTracingDisabledForOperationName(@Nullable final SpanOperationName traceOperationName) {
-        return null == traceOperationName;
-    }
-
     private static Map<String, String> getHttpHeadersAsMap(final Iterable<HttpHeader> httpHeaders) {
         return StreamSupport.stream(httpHeaders.spliterator(), false)
                 .collect(Collectors.toMap(HttpHeader::name, HttpHeader::value, (oldValue, value) -> value));

diff --git a/...y/service/src/main/java/org/eclipse/ditto/gateway/service/endpoints/routes/RootRoute.java b/...y/service/src/main/java/org/eclipse/ditto/gateway/service/endpoints/routes/RootRoute.java
@@ -179,7 +179,6 @@ private Route buildRoute() {
     }
 
     private Route wrapWithRootDirectives(final Function<String, Route> rootRoute) {
-        final var requestTracingDirective = RequestTracingDirective.newInstanceWithDisabled();
 
         final Function<Function<String, Route>, Route> outerRouteProvider = innerRouteProvider ->
                 /* the outer handleExceptions is for handling exceptions in the directives wrapping the rootRoute
@@ -188,7 +187,7 @@ private Route wrapWithRootDirectives(final Function<String, Route> rootRoute) {
                         CorrelationIdEnsuringDirective.ensureCorrelationId(
                                 correlationId -> requestTimeoutHandlingDirective
                                         .handleRequestTimeout(correlationId, () ->
-                                                requestTracingDirective.traceRequest(
+                                                RequestTracingDirective.traceRequest(
                                                         () -> RequestResultLoggingDirective.logRequestResult(
                                                                 correlationId,
                                                                 () -> innerRouteProvider.apply(correlationId)

diff --git a/gateway/service/src/main/resources/gateway.conf b/gateway/service/src/main/resources/gateway.conf
@@ -399,7 +399,7 @@ ditto {
   tracing {
     filter: {
       includes = ["*"],
-      excludes = ["/ws/2 GET"]
+      excludes = ["GET /ws/2"]
     }
   }
 }

diff --git a/...a/org/eclipse/ditto/gateway/service/endpoints/directives/RequestTracingDirectiveTest.java b/...a/org/eclipse/ditto/gateway/service/endpoints/directives/RequestTracingDirectiveTest.java
@@ -92,17 +92,15 @@ public void assertImmutability() {
 
     @Test
     public void traceRequestWithNullInnerRouteSupplierThrowsNullPointerException() {
-        final var underTest = RequestTracingDirective.newInstanceWithDisabled();
 
         Assertions.assertThatNullPointerException()
-                .isThrownBy(() -> underTest.traceRequest(null, null))
+                .isThrownBy(() -> RequestTracingDirective.traceRequest(null, null))
                 .withMessage("The innerRouteSupplier must not be null!")
                 .withNoCause();
     }
 
     @Test
     public void traceRequestCallsDittoTracingIfTracingIsEnabledForResolvedSpanOperationName() {
-        final var underTest = RequestTracingDirective.newInstanceWithDisabled();
 
         final var headersMap = Map.ofEntries(
                 Map.entry(
@@ -123,7 +121,7 @@ public void traceRequestCallsDittoTracingIfTracingIsEnabledForResolvedSpanOperat
                     .thenReturn(preparedSpan);
             final var testRoute = testRoute(
                     extractRequestContext(
-                            requestContext -> underTest.traceRequest(
+                            requestContext -> RequestTracingDirective.traceRequest(
                                     () -> Mockito.mock(Route.class),
                                     testNameCorrelationId.getCorrelationId()
                             )
@@ -146,7 +144,6 @@ public void traceRequestCallsDittoTracingIfTracingIsEnabledForResolvedSpanOperat
 
     @Test
     public void traceRequestWithExistingW3cTracingHeadersReplacesThoseHeadersWithCurrentSpanContextHeaders() {
-        final var underTest = RequestTracingDirective.newInstanceWithDisabled();
         final var expectedStatus = StatusCodes.NO_CONTENT;
         final var effectiveHttpRequestHeader = new CompletableFuture<Map<String, String>>();
         final var routeFactory = new AllDirectives() {
@@ -169,7 +166,7 @@ public Route createRoute() {
         final var traceparentHeaderValue = "00-00000000000000002d773e5f58ee5636-28cae4bd320cbc11-0";
         final var fooHeaderValue = "bar";
         final var testRoute = testRoute(
-                underTest.traceRequest(routeFactory::createRoute, testNameCorrelationId.getCorrelationId())
+                RequestTracingDirective.traceRequest(routeFactory::createRoute, testNameCorrelationId.getCorrelationId())
         );
 
         final var testRouteResult = testRoute.run(HttpRequest.create()