Merge pull request #2088 from beyonnex-io/bugfix/trace-parent-relatio…

…nships fix Gateway trace not keeping correct parent "hierarchy" when receiving existing "traceparent" header
eclipse-ditto · Jan 9, 2025 · ce48b7c · ce48b7c
2 parents 571fb59 + b45f230
commit ce48b7c
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 35 deletions.
diff --git a/...ce/src/main/java/org/eclipse/ditto/connectivity/service/messaging/BasePublisherActor.java b/...ce/src/main/java/org/eclipse/ditto/connectivity/service/messaging/BasePublisherActor.java
@@ -482,7 +482,11 @@ private SendingOrDropped publishToGenericTarget(final ExpressionResolver resolve
                     .tag(SpanTagKey.CONNECTION_TARGET.getTagForValue(publishTarget.toString()))
                     .start();
             final var mappedMessageWithTraceContext =
-                    mappedMessage.withHeaders(startedSpan.propagateContext(mappedMessage.getHeaders()));
+                    mappedMessage.withHeaders(startedSpan.propagateContext(
+                            DittoHeaders.newBuilder(mappedMessage.getHeaders())
+                                    .removeHeader(DittoHeaderDefinition.W3C_TRACEPARENT.getKey())
+                                    .build()
+                    ));
 
             final CompletionStage<SendResult> responsesFuture = publishMessage(outboundSource,
                     autoAckTarget,

diff --git a/.../java/org/eclipse/ditto/gateway/service/endpoints/directives/RequestTracingDirective.java b/.../java/org/eclipse/ditto/gateway/service/endpoints/directives/RequestTracingDirective.java
@@ -157,29 +157,7 @@ private static Route getRouteWithEnabledTracing(
             @Nullable final CharSequence correlationId
     ) {
         return mapRequest(
-                req -> {
-                    final Set<String> headerNames = new HashSet<>();
-                    return adjustSpanContextHeadersOfRequest(req, startedSpan.propagateContext(
-                            StreamSupport.stream(httpRequest.getHeaders().spliterator(), false)
-                                    .map(httpHeader -> {
-                                        if (!headerNames.add(httpHeader.name())) {
-                                            throw GatewayDuplicateHeaderException.newBuilder(httpHeader.name())
-                                                    .dittoHeaders(DittoHeaders.newBuilder()
-                                                            .correlationId(correlationId)
-                                                            .build()
-                                                    ).build();
-                                        }
-                                        return httpHeader;
-                                    })
-                                    .collect(Collectors.toMap(HttpHeader::name, HttpHeader::value, (dv1, dv2) -> {
-                                        throw GatewayDuplicateHeaderException.newBuilder()
-                                                .dittoHeaders(DittoHeaders.newBuilder()
-                                                        .correlationId(correlationId)
-                                                        .build()
-                                                ).build();
-                                    }))
-                    ));
-                },
+                req -> adjustSpanContextHeadersOfRequest(req, correlationId, startedSpan),
                 () -> mapRouteResult(
                         routeResult -> tryToHandleRouteResult(routeResult, httpRequest, startedSpan, correlationId),
                         innerRouteSupplier
@@ -189,13 +167,37 @@ private static Route getRouteWithEnabledTracing(
 
     private static HttpRequest adjustSpanContextHeadersOfRequest(
             final HttpRequest originalRequest,
-            final Map<String, String> spanContextHeaders
+            @Nullable final CharSequence correlationId,
+            final StartedSpan startedSpan
     ) {
+        final Set<String> headerNames = new HashSet<>();
+        final Map<String, String> httpHeaders = StreamSupport.stream(originalRequest.getHeaders().spliterator(), false)
+                .map(httpHeader -> {
+                    if (!headerNames.add(httpHeader.name())) {
+                        throw GatewayDuplicateHeaderException.newBuilder(httpHeader.name())
+                                .dittoHeaders(DittoHeaders.newBuilder()
+                                        .correlationId(correlationId)
+                                        .build()
+                                ).build();
+                    }
+                    return httpHeader;
+                })
+                .filter(httpHeader ->
+                        !DittoHeaderDefinition.W3C_TRACEPARENT.getKey().equals(httpHeader.name())
+                )
+                .collect(Collectors.toMap(HttpHeader::name, HttpHeader::value, (dv1, dv2) -> {
+                    throw GatewayDuplicateHeaderException.newBuilder()
+                            .dittoHeaders(DittoHeaders.newBuilder()
+                                    .correlationId(correlationId)
+                                    .build()
+                            ).build();
+                }));
 
+        final Map<String, String> propagatedHeaders = startedSpan.propagateContext(httpHeaders);
         // Replace W3C tracing headers of original request because from now
         // on the newly started span is the parent of all subsequent spans.
         var result = originalRequest;
-        for (final var w3cTracingHeader : getW3cTracingHeaders(spanContextHeaders)) {
+        for (final var w3cTracingHeader : getW3cTracingHeaders(propagatedHeaders)) {
             result = result.removeHeader(w3cTracingHeader.name()).addHeader(w3cTracingHeader);
         }
         return result;

diff --git a/...a/org/eclipse/ditto/gateway/service/endpoints/directives/RequestTracingDirectiveTest.java b/...a/org/eclipse/ditto/gateway/service/endpoints/directives/RequestTracingDirectiveTest.java
@@ -166,7 +166,8 @@ public Route createRoute() {
                         .containsEntry("foo", fooHeaderValue)
                         .containsKeys(W3C_TRACEPARENT.getKey(), W3C_TRACESTATE.getKey())
                         .containsValue(tracestateHeaderValue)
-                        .containsValue(traceparentHeaderValue));
+                        .doesNotContainValue(traceparentHeaderValue) // it must explicitly contain another trace parent instead
+                );
     }
 
 }
diff --git a/...g/eclipse/ditto/internal/utils/cluster/AbstractJsonifiableWithDittoHeadersSerializer.java b/...g/eclipse/ditto/internal/utils/cluster/AbstractJsonifiableWithDittoHeadersSerializer.java
@@ -167,7 +167,7 @@ public void toBinary(final Object object, final ByteBuffer buf) {
                 throw new IllegalArgumentException(errorMessage, e);
             } catch (final IOException e) {
                 final var errorMessage = MessageFormat.format(
-                        "Serialization failed with {} on Jsonifiable with string representation <{}>",
+                        "Serialization failed with {0} on Jsonifiable with string representation <{1}>",
                         e.getClass().getName(),
                         jsonObject
                 );
@@ -191,12 +191,10 @@ private static StartedSpan startTracingSpanForSerialization(
             final Object objectToSerialize
     ) {
         final String spanSerializeName;
-        if (objectToSerialize instanceof WithType withType) {
-            spanSerializeName = withType.getType();
-        } else if (objectToSerialize instanceof DittoRuntimeException dre) {
-            spanSerializeName = dre.getErrorCode();
-        } else {
-            spanSerializeName = objectToSerialize.getClass().getSimpleName();
+        switch (objectToSerialize) {
+            case WithType withType -> spanSerializeName = withType.getType();
+            case DittoRuntimeException dre -> spanSerializeName = dre.getErrorCode();
+            default -> spanSerializeName = objectToSerialize.getClass().getSimpleName();
         }
         final var startInstant = StartInstant.now();
         return DittoTracing.newPreparedSpan(
@@ -363,7 +361,7 @@ private JsonObject deserializeByteBufferAsJsonObjectOrThrow(final ByteBuffer byt
                     manifest,
                     serializerName);
             throw JsonParseException.newBuilder()
-                    .message(MessageFormat.format("<{}> is not a valid {} object! (It''s a value.)",
+                    .message(MessageFormat.format("<{0}> is not a valid {1} object! (It''s a value.)",
                             BinaryToHexConverter.createDebugMessageByTryingToConvertToHexString(byteBuffer),
                             serializerName))
                     .build();