From d572adc4c24839b4200dc39161e0712490f97749 Mon Sep 17 00:00:00 2001 From: Jakub Malinowski Date: Wed, 20 Nov 2024 23:19:38 +0100 Subject: [PATCH 1/2] fix: multiple server-tracing headers aren't handled --- .../rum/RumResponseAttributesExtractor.java | 18 ++++++++++++------ .../RumResponseAttributesExtractorTest.java | 10 ++++------ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/splunk-otel-android/src/main/java/com/splunk/rum/RumResponseAttributesExtractor.java b/splunk-otel-android/src/main/java/com/splunk/rum/RumResponseAttributesExtractor.java index 7f8cdef4..423b7a1c 100644 --- a/splunk-otel-android/src/main/java/com/splunk/rum/RumResponseAttributesExtractor.java +++ b/splunk-otel-android/src/main/java/com/splunk/rum/RumResponseAttributesExtractor.java @@ -28,6 +28,7 @@ class RumResponseAttributesExtractor implements AttributesExtractor { + public static final String SERVER_TIMING_HEADER = "server-timing"; private final ServerTimingHeaderParser serverTimingHeaderParser; public RumResponseAttributesExtractor(ServerTimingHeaderParser serverTimingHeaderParser) { @@ -52,11 +53,16 @@ public void onEnd( } private void onResponse(AttributesBuilder attributes, Response response) { - String serverTimingHeader = response.header("Server-Timing"); - String[] ids = serverTimingHeaderParser.parse(serverTimingHeader); - if (ids.length == 2) { - attributes.put(LINK_TRACE_ID_KEY, ids[0]); - attributes.put(LINK_SPAN_ID_KEY, ids[1]); - } + response.headers().forEach(header -> { + if (!header.getFirst().equalsIgnoreCase(SERVER_TIMING_HEADER)) { + return; + } + + String[] ids = serverTimingHeaderParser.parse(header.getSecond()); + if (ids.length == 2) { + attributes.put(LINK_TRACE_ID_KEY, ids[0]); + attributes.put(LINK_SPAN_ID_KEY, ids[1]); + } + }); } } diff --git a/splunk-otel-android/src/test/java/com/splunk/rum/RumResponseAttributesExtractorTest.java b/splunk-otel-android/src/test/java/com/splunk/rum/RumResponseAttributesExtractorTest.java index 1ca693b2..73372aa2 100644 --- a/splunk-otel-android/src/test/java/com/splunk/rum/RumResponseAttributesExtractorTest.java +++ b/splunk-otel-android/src/test/java/com/splunk/rum/RumResponseAttributesExtractorTest.java @@ -36,10 +36,6 @@ class RumResponseAttributesExtractorTest { @Test void spanDecoration() { - ServerTimingHeaderParser headerParser = mock(ServerTimingHeaderParser.class); - when(headerParser.parse("headerValue")) - .thenReturn(new String[] {"9499195c502eb217c448a68bfe0f967c", "fe16eca542cd5d86"}); - Request fakeRequest = mock(Request.class); Response response = new Response.Builder() @@ -47,11 +43,13 @@ void spanDecoration() { .protocol(Protocol.HTTP_1_1) .message("hello") .code(200) - .addHeader("Server-Timing", "headerValue") + .addHeader("Server-Timing", "othervalue 1") + .addHeader("Server-Timing", "traceparent;desc=\"00-9499195c502eb217c448a68bfe0f967c-fe16eca542cd5d86-01\"") + .addHeader("Server-Timing", "othervalue 2") .build(); RumResponseAttributesExtractor attributesExtractor = - new RumResponseAttributesExtractor(headerParser); + new RumResponseAttributesExtractor(new ServerTimingHeaderParser()); AttributesBuilder attributesBuilder = Attributes.builder(); attributesExtractor.onStart(attributesBuilder, Context.root(), fakeRequest); attributesExtractor.onEnd(attributesBuilder, Context.root(), fakeRequest, response, null); From 1c3db17479cc3e6cc862d7264235c1543d74e79d Mon Sep 17 00:00:00 2001 From: Jakub Malinowski Date: Thu, 21 Nov 2024 14:46:04 +0100 Subject: [PATCH 2/2] PR feedback --- .../rum/RumResponseAttributesExtractor.java | 22 ++--- .../RumResponseAttributesExtractorTest.java | 88 ++++++++++++++----- 2 files changed, 76 insertions(+), 34 deletions(-) diff --git a/splunk-otel-android/src/main/java/com/splunk/rum/RumResponseAttributesExtractor.java b/splunk-otel-android/src/main/java/com/splunk/rum/RumResponseAttributesExtractor.java index 423b7a1c..3b116c72 100644 --- a/splunk-otel-android/src/main/java/com/splunk/rum/RumResponseAttributesExtractor.java +++ b/splunk-otel-android/src/main/java/com/splunk/rum/RumResponseAttributesExtractor.java @@ -53,16 +53,18 @@ public void onEnd( } private void onResponse(AttributesBuilder attributes, Response response) { - response.headers().forEach(header -> { - if (!header.getFirst().equalsIgnoreCase(SERVER_TIMING_HEADER)) { - return; - } + response.headers() + .forEach( + header -> { + if (!header.getFirst().equalsIgnoreCase(SERVER_TIMING_HEADER)) { + return; + } - String[] ids = serverTimingHeaderParser.parse(header.getSecond()); - if (ids.length == 2) { - attributes.put(LINK_TRACE_ID_KEY, ids[0]); - attributes.put(LINK_SPAN_ID_KEY, ids[1]); - } - }); + String[] ids = serverTimingHeaderParser.parse(header.getSecond()); + if (ids.length == 2) { + attributes.put(LINK_TRACE_ID_KEY, ids[0]); + attributes.put(LINK_SPAN_ID_KEY, ids[1]); + } + }); } } diff --git a/splunk-otel-android/src/test/java/com/splunk/rum/RumResponseAttributesExtractorTest.java b/splunk-otel-android/src/test/java/com/splunk/rum/RumResponseAttributesExtractorTest.java index 73372aa2..7aae9061 100644 --- a/splunk-otel-android/src/test/java/com/splunk/rum/RumResponseAttributesExtractorTest.java +++ b/splunk-otel-android/src/test/java/com/splunk/rum/RumResponseAttributesExtractorTest.java @@ -38,28 +38,62 @@ class RumResponseAttributesExtractorTest { void spanDecoration() { Request fakeRequest = mock(Request.class); Response response = - new Response.Builder() - .request(fakeRequest) - .protocol(Protocol.HTTP_1_1) - .message("hello") - .code(200) + getBaseRuestBuilder(fakeRequest) + .addHeader( + "Server-Timing", + "traceparent;desc=\"00-00000000000000000000000000000001-0000000000000001-01\"") + .build(); + Attributes attributes = performAttributesExtraction(fakeRequest, response); + + assertThat(attributes) + .containsOnly( + entry(COMPONENT_KEY, "http"), + entry(LINK_TRACE_ID_KEY, "00000000000000000000000000000001"), + entry(LINK_SPAN_ID_KEY, "0000000000000001")); + } + + @Test + void ignoresMalformed() { + Request fakeRequest = mock(Request.class); + Response response = + getBaseRuestBuilder(fakeRequest) .addHeader("Server-Timing", "othervalue 1") - .addHeader("Server-Timing", "traceparent;desc=\"00-9499195c502eb217c448a68bfe0f967c-fe16eca542cd5d86-01\"") + .addHeader( + "Server-Timing", + "traceparent;desc=\"00-00000000000000000000000000000001-0000000000000001-01\"") .addHeader("Server-Timing", "othervalue 2") .build(); + Attributes attributes = performAttributesExtraction(fakeRequest, response); - RumResponseAttributesExtractor attributesExtractor = - new RumResponseAttributesExtractor(new ServerTimingHeaderParser()); - AttributesBuilder attributesBuilder = Attributes.builder(); - attributesExtractor.onStart(attributesBuilder, Context.root(), fakeRequest); - attributesExtractor.onEnd(attributesBuilder, Context.root(), fakeRequest, response, null); - Attributes attributes = attributesBuilder.build(); + assertThat(attributes) + .containsOnly( + entry(COMPONENT_KEY, "http"), + entry(LINK_TRACE_ID_KEY, "00000000000000000000000000000001"), + entry(LINK_SPAN_ID_KEY, "0000000000000001")); + } + + @Test + void lastMatchingWins() { + Request fakeRequest = mock(Request.class); + Response response = + getBaseRuestBuilder(fakeRequest) + .addHeader( + "Server-Timing", + "traceparent;desc=\"00-00000000000000000000000000000001-0000000000000001-01\"") + .addHeader( + "Server-Timing", + "traceparent;desc=\"00-00000000000000000000000000000002-0000000000000002-01\"") + .addHeader( + "Server-Timing", + "traceparent;desc=\"00-00000000000000000000000000000003-0000000000000003-01\"") + .build(); + Attributes attributes = performAttributesExtraction(fakeRequest, response); assertThat(attributes) .containsOnly( entry(COMPONENT_KEY, "http"), - entry(LINK_TRACE_ID_KEY, "9499195c502eb217c448a68bfe0f967c"), - entry(LINK_SPAN_ID_KEY, "fe16eca542cd5d86")); + entry(LINK_TRACE_ID_KEY, "00000000000000000000000000000003"), + entry(LINK_SPAN_ID_KEY, "0000000000000003")); } @Test @@ -68,21 +102,27 @@ void spanDecoration_noLinkingHeader() { when(headerParser.parse(null)).thenReturn(new String[0]); Request fakeRequest = mock(Request.class); - Response response = - new Response.Builder() - .request(fakeRequest) - .protocol(Protocol.HTTP_1_1) - .message("hello") - .code(200) - .build(); + Response response = getBaseRuestBuilder(fakeRequest).build(); + Attributes attributes = performAttributesExtraction(fakeRequest, response); + assertThat(attributes).containsOnly(entry(COMPONENT_KEY, "http")); + } + + private static Attributes performAttributesExtraction(Request fakeRequest, Response response) { RumResponseAttributesExtractor attributesExtractor = - new RumResponseAttributesExtractor(headerParser); + new RumResponseAttributesExtractor(new ServerTimingHeaderParser()); AttributesBuilder attributesBuilder = Attributes.builder(); - attributesExtractor.onEnd(attributesBuilder, Context.root(), fakeRequest, response, null); attributesExtractor.onStart(attributesBuilder, Context.root(), fakeRequest); + attributesExtractor.onEnd(attributesBuilder, Context.root(), fakeRequest, response, null); Attributes attributes = attributesBuilder.build(); + return attributes; + } - assertThat(attributes).containsOnly(entry(COMPONENT_KEY, "http")); + private Response.Builder getBaseRuestBuilder(Request fakeRequest) { + return new Response.Builder() + .request(fakeRequest) + .protocol(Protocol.HTTP_1_1) + .message("hello") + .code(200); } }