PR feedback

signalfx · Nov 21, 2024 · 1c3db17 · 1c3db17
1 parent d572adc
commit 1c3db17
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 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
@@ -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
@@ -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);
     }
 }