From d148b1fb3e7e0be4e387df46a4fee9c3715d625d Mon Sep 17 00:00:00 2001 From: Kate Anderson Date: Wed, 28 Feb 2024 08:16:18 -0800 Subject: [PATCH 1/7] add check for duplicates in 1.4 and 1.40 grpc modules --- .../nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java | 4 +++- .../nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/instrumentation/grpc-1.4.0/src/main/java/com/nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java b/instrumentation/grpc-1.4.0/src/main/java/com/nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java index 3359805d0a..7a9dd5e92b 100644 --- a/instrumentation/grpc-1.4.0/src/main/java/com/nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java +++ b/instrumentation/grpc-1.4.0/src/main/java/com/nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java @@ -27,7 +27,9 @@ public HeaderType getHeaderType() { @Override public void setHeader(String key, String value) { if (GrpcConfig.disributedTracingEnabled) { - metadata.put(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER), value); + if (!metadata.containsKey(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER))) { + metadata.put(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER), value); + } } } } diff --git a/instrumentation/grpc-1.40.0/src/main/java/com/nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java b/instrumentation/grpc-1.40.0/src/main/java/com/nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java index ac2ad7e61b..c408c87fa6 100644 --- a/instrumentation/grpc-1.40.0/src/main/java/com/nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java +++ b/instrumentation/grpc-1.40.0/src/main/java/com/nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java @@ -27,7 +27,9 @@ public HeaderType getHeaderType() { @Override public void setHeader(String key, String value) { if (GrpcConfig.disributedTracingEnabled) { - metadata.put(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER), value); + if (!metadata.containsKey(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER))) { + metadata.put(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER), value); + } } } } From 68b02e26eb5f7a2d7d2a0b54923906779f6f4455 Mon Sep 17 00:00:00 2001 From: Kate Anderson Date: Wed, 28 Feb 2024 10:27:50 -0800 Subject: [PATCH 2/7] check for duplicate headers in 1.22 and 1.30 grpc modules --- .../nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java | 4 +++- .../nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/instrumentation/grpc-1.22.0/src/main/java/com/nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java b/instrumentation/grpc-1.22.0/src/main/java/com/nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java index 3359805d0a..7a9dd5e92b 100644 --- a/instrumentation/grpc-1.22.0/src/main/java/com/nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java +++ b/instrumentation/grpc-1.22.0/src/main/java/com/nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java @@ -27,7 +27,9 @@ public HeaderType getHeaderType() { @Override public void setHeader(String key, String value) { if (GrpcConfig.disributedTracingEnabled) { - metadata.put(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER), value); + if (!metadata.containsKey(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER))) { + metadata.put(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER), value); + } } } } diff --git a/instrumentation/grpc-1.30.0/src/main/java/com/nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java b/instrumentation/grpc-1.30.0/src/main/java/com/nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java index 3359805d0a..7a9dd5e92b 100644 --- a/instrumentation/grpc-1.30.0/src/main/java/com/nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java +++ b/instrumentation/grpc-1.30.0/src/main/java/com/nr/agent/instrumentation/grpc/OutboundHeadersWrapper.java @@ -27,7 +27,9 @@ public HeaderType getHeaderType() { @Override public void setHeader(String key, String value) { if (GrpcConfig.disributedTracingEnabled) { - metadata.put(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER), value); + if (!metadata.containsKey(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER))) { + metadata.put(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER), value); + } } } } From 5fe663eb5d1de2a1878f27fc8c1761e5f8d32b82 Mon Sep 17 00:00:00 2001 From: Kate Anderson Date: Wed, 28 Feb 2024 11:01:29 -0800 Subject: [PATCH 3/7] Allow multiple equal headers in parseHeaders and add log message --- .../newrelic/agent/tracing/W3CTraceParentParser.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/tracing/W3CTraceParentParser.java b/newrelic-agent/src/main/java/com/newrelic/agent/tracing/W3CTraceParentParser.java index 5c493be5cf..69995ddf94 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/tracing/W3CTraceParentParser.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/tracing/W3CTraceParentParser.java @@ -7,19 +7,25 @@ package com.newrelic.agent.tracing; +import com.newrelic.agent.Agent; import com.newrelic.agent.MetricNames; import com.newrelic.agent.service.ServiceFactory; import java.util.List; +import java.util.logging.Level; public class W3CTraceParentParser { static W3CTraceParent parseHeaders(List traceParentHeaders) { if (traceParentHeaders.size() != 1) { ServiceFactory.getStatsService().getMetricAggregator().incrementCounter(MetricNames.SUPPORTABILITY_TRACE_CONTEXT_INVALID_PARENT_HEADER_COUNT); - return null; + Agent.LOG.log(Level.WARNING, "Multiple traceparent headers found on inbound request."); + // Multiple values ok if all are equal + boolean allHeadersEqual = traceParentHeaders.stream().allMatch(h -> h.equals(traceParentHeaders.get(0))); + if (traceParentHeaders.isEmpty() || !allHeadersEqual) { + return null; + } } - String traceParentHeader = traceParentHeaders.get(0); return parseHeader(traceParentHeader); } From 04d71f85e3d1cb47efdc6d54e852e508307ccb16 Mon Sep 17 00:00:00 2001 From: Kate Anderson Date: Mon, 4 Mar 2024 12:39:38 -0800 Subject: [PATCH 4/7] Update W3CTraceParentHeaderTest to validate duplicates --- .../agent/tracing/W3CTraceParentHeaderTest.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/tracing/W3CTraceParentHeaderTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/tracing/W3CTraceParentHeaderTest.java index 7e8fd8d7bc..b89bdd3dfc 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/tracing/W3CTraceParentHeaderTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/tracing/W3CTraceParentHeaderTest.java @@ -69,12 +69,20 @@ public void testCreateTraceParentPayloadPadsAndLowerCasesTraceId() { } @Test - public void testParseMultiple_oopsTooMany() { + public void testParseMultiple_differentValues_shouldFail() { W3CTraceParent result = W3CTraceParentParser.parseHeaders( - Arrays.asList("00-12345678123456781234567812345678-1234123412341234-01", "00-12345678123456781234567812345678-1234123412341234-01")); + Arrays.asList("00-12345678123456781234567812345678-1234123412341234-01", "00-87654321876543218765432187654321-1234123412341234-01")); assertNull(result); } + @Test + public void testParseMultiple_sameValues_shouldPass() { + W3CTraceParent result = W3CTraceParentParser.parseHeaders( + Arrays.asList("00-12345678123456781234567812345678-1234123412341234-01", "00-12345678123456781234567812345678-1234123412341234-01")); + W3CTraceParent expected = new W3CTraceParent("00", "12345678123456781234567812345678", "1234123412341234", 1); + assertEquals(expected, result); + } + @Test public void testParseMultiple_butReallyJustOne() { W3CTraceParent result = W3CTraceParentParser.parseHeaders( From a7516243e78b80d1ec27e465dab89932d2f0f6f3 Mon Sep 17 00:00:00 2001 From: Kate Anderson Date: Mon, 4 Mar 2024 16:53:06 -0800 Subject: [PATCH 5/7] Add test for grpc and DT --- .../test/java/GrpcDistributedTracingTest.java | 68 +++++++++++++++++++ .../test/resources/enable_dist_tracing.yml | 3 + .../test/java/GrpcDistributedTracingTest.java | 67 ++++++++++++++++++ .../test/resources/enable_dist_tracing.yml | 3 + .../test/java/GrpcDistributedTracingTest.java | 68 +++++++++++++++++++ .../test/resources/enable_dist_tracing.yml | 3 + .../test/java/GrpcDistributedTracingTest.java | 67 ++++++++++++++++++ .../test/resources/enable_dist_tracing.yml | 3 + 8 files changed, 282 insertions(+) create mode 100644 instrumentation/grpc-1.22.0/src/test/java/GrpcDistributedTracingTest.java create mode 100644 instrumentation/grpc-1.22.0/src/test/resources/enable_dist_tracing.yml create mode 100644 instrumentation/grpc-1.30.0/src/test/java/GrpcDistributedTracingTest.java create mode 100644 instrumentation/grpc-1.30.0/src/test/resources/enable_dist_tracing.yml create mode 100644 instrumentation/grpc-1.4.0/src/test/java/GrpcDistributedTracingTest.java create mode 100644 instrumentation/grpc-1.4.0/src/test/resources/enable_dist_tracing.yml create mode 100644 instrumentation/grpc-1.40.0/src/test/java/GrpcDistributedTracingTest.java create mode 100644 instrumentation/grpc-1.40.0/src/test/resources/enable_dist_tracing.yml diff --git a/instrumentation/grpc-1.22.0/src/test/java/GrpcDistributedTracingTest.java b/instrumentation/grpc-1.22.0/src/test/java/GrpcDistributedTracingTest.java new file mode 100644 index 0000000000..7273a94805 --- /dev/null +++ b/instrumentation/grpc-1.22.0/src/test/java/GrpcDistributedTracingTest.java @@ -0,0 +1,68 @@ +import app.TestClient; +import app.TestServer; +import com.newrelic.agent.introspec.InstrumentationTestConfig; +import com.newrelic.agent.introspec.InstrumentationTestRunner; +import com.newrelic.agent.introspec.Introspector; +import com.newrelic.agent.introspec.TransactionEvent; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.util.Collection; + +import static org.junit.Assert.assertEquals; + +@RunWith(InstrumentationTestRunner.class) +@InstrumentationTestConfig(includePrefixes = { "io.grpc", "com.nr.agent.instrumentation.grpc" }, configName="enable_dist_tracing.yml") +public class GrpcDistributedTracingTest { + + private static TestServer server; + private static TestClient client; + + @BeforeClass + public static void before() throws Exception { + server = new TestServer(); + server.start(); + client = new TestClient("localhost", server.getPort()); + } + + @AfterClass + public static void after() throws InterruptedException { + if (client != null) { + client.shutdown(); + } + if (server != null) { + server.stop(); + } + } + + @Test + public void testDTAsyncRequest() { + client.helloAsync("Async"); + + String clientTxName = "OtherTransaction/Custom/app.TestClient/helloAsync"; + String serverTxName = "WebTransaction/gRPC/helloworld.Greeter/SayHello"; + + Introspector introspector = InstrumentationTestRunner.getIntrospector(); + + assertEquals(2, introspector.getFinishedTransactionCount(30000)); + + Collection clientTxns = introspector.getTransactionEvents(clientTxName); + assertEquals(1, clientTxns.size()); + String clientGuid = ""; + String clientTripId = ""; + for (TransactionEvent txn: clientTxns) { + //make other assertions? + clientGuid = txn.getMyGuid(); + clientTripId = txn.getTripId(); + } + + Collection serverTxns = introspector.getTransactionEvents(serverTxName); + assertEquals(1, serverTxns.size()); + for (TransactionEvent txn: serverTxns) { + assertEquals(clientGuid, txn.getParentId()); + assertEquals(clientTripId, txn.getTripId()); + } + } +} diff --git a/instrumentation/grpc-1.22.0/src/test/resources/enable_dist_tracing.yml b/instrumentation/grpc-1.22.0/src/test/resources/enable_dist_tracing.yml new file mode 100644 index 0000000000..f2f195c174 --- /dev/null +++ b/instrumentation/grpc-1.22.0/src/test/resources/enable_dist_tracing.yml @@ -0,0 +1,3 @@ +common: &default_settings + distributed_tracing: + enabled: true diff --git a/instrumentation/grpc-1.30.0/src/test/java/GrpcDistributedTracingTest.java b/instrumentation/grpc-1.30.0/src/test/java/GrpcDistributedTracingTest.java new file mode 100644 index 0000000000..a70902d56e --- /dev/null +++ b/instrumentation/grpc-1.30.0/src/test/java/GrpcDistributedTracingTest.java @@ -0,0 +1,67 @@ +import app.TestClient; +import app.TestServer; +import com.newrelic.agent.introspec.InstrumentationTestConfig; +import com.newrelic.agent.introspec.InstrumentationTestRunner; +import com.newrelic.agent.introspec.Introspector; +import com.newrelic.agent.introspec.TransactionEvent; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.util.Collection; + +import static org.junit.Assert.assertEquals; + +@RunWith(InstrumentationTestRunner.class) +@InstrumentationTestConfig(includePrefixes = { "io.grpc", "com.nr.agent.instrumentation.grpc" }, configName="enable_dist_tracing.yml") +public class GrpcDistributedTracingTest { + + private static TestServer server; + private static TestClient client; + + @BeforeClass + public static void before() throws Exception { + server = new TestServer(); + server.start(); + client = new TestClient("localhost", server.getPort()); + } + + @AfterClass + public static void after() throws InterruptedException { + if (client != null) { + client.shutdown(); + } + if (server != null) { + server.stop(); + } + } + + @Test + public void testDTAsyncRequest() { + client.helloAsync("Async"); + + String clientTxName = "OtherTransaction/Custom/app.TestClient/helloAsync"; + String serverTxName = "WebTransaction/gRPC/helloworld.Greeter/SayHello"; + + Introspector introspector = InstrumentationTestRunner.getIntrospector(); + + assertEquals(2, introspector.getFinishedTransactionCount(30000)); + + Collection clientTxns = introspector.getTransactionEvents(clientTxName); + assertEquals(1, clientTxns.size()); + String clientGuid = ""; + String clientTripId = ""; + for (TransactionEvent txn: clientTxns) { + clientGuid = txn.getMyGuid(); + clientTripId = txn.getTripId(); + } + + Collection serverTxns = introspector.getTransactionEvents(serverTxName); + assertEquals(1, serverTxns.size()); + for (TransactionEvent txn: serverTxns) { + assertEquals(clientGuid, txn.getParentId()); + assertEquals(clientTripId, txn.getTripId()); + } + } +} diff --git a/instrumentation/grpc-1.30.0/src/test/resources/enable_dist_tracing.yml b/instrumentation/grpc-1.30.0/src/test/resources/enable_dist_tracing.yml new file mode 100644 index 0000000000..f2f195c174 --- /dev/null +++ b/instrumentation/grpc-1.30.0/src/test/resources/enable_dist_tracing.yml @@ -0,0 +1,3 @@ +common: &default_settings + distributed_tracing: + enabled: true diff --git a/instrumentation/grpc-1.4.0/src/test/java/GrpcDistributedTracingTest.java b/instrumentation/grpc-1.4.0/src/test/java/GrpcDistributedTracingTest.java new file mode 100644 index 0000000000..7273a94805 --- /dev/null +++ b/instrumentation/grpc-1.4.0/src/test/java/GrpcDistributedTracingTest.java @@ -0,0 +1,68 @@ +import app.TestClient; +import app.TestServer; +import com.newrelic.agent.introspec.InstrumentationTestConfig; +import com.newrelic.agent.introspec.InstrumentationTestRunner; +import com.newrelic.agent.introspec.Introspector; +import com.newrelic.agent.introspec.TransactionEvent; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.util.Collection; + +import static org.junit.Assert.assertEquals; + +@RunWith(InstrumentationTestRunner.class) +@InstrumentationTestConfig(includePrefixes = { "io.grpc", "com.nr.agent.instrumentation.grpc" }, configName="enable_dist_tracing.yml") +public class GrpcDistributedTracingTest { + + private static TestServer server; + private static TestClient client; + + @BeforeClass + public static void before() throws Exception { + server = new TestServer(); + server.start(); + client = new TestClient("localhost", server.getPort()); + } + + @AfterClass + public static void after() throws InterruptedException { + if (client != null) { + client.shutdown(); + } + if (server != null) { + server.stop(); + } + } + + @Test + public void testDTAsyncRequest() { + client.helloAsync("Async"); + + String clientTxName = "OtherTransaction/Custom/app.TestClient/helloAsync"; + String serverTxName = "WebTransaction/gRPC/helloworld.Greeter/SayHello"; + + Introspector introspector = InstrumentationTestRunner.getIntrospector(); + + assertEquals(2, introspector.getFinishedTransactionCount(30000)); + + Collection clientTxns = introspector.getTransactionEvents(clientTxName); + assertEquals(1, clientTxns.size()); + String clientGuid = ""; + String clientTripId = ""; + for (TransactionEvent txn: clientTxns) { + //make other assertions? + clientGuid = txn.getMyGuid(); + clientTripId = txn.getTripId(); + } + + Collection serverTxns = introspector.getTransactionEvents(serverTxName); + assertEquals(1, serverTxns.size()); + for (TransactionEvent txn: serverTxns) { + assertEquals(clientGuid, txn.getParentId()); + assertEquals(clientTripId, txn.getTripId()); + } + } +} diff --git a/instrumentation/grpc-1.4.0/src/test/resources/enable_dist_tracing.yml b/instrumentation/grpc-1.4.0/src/test/resources/enable_dist_tracing.yml new file mode 100644 index 0000000000..f2f195c174 --- /dev/null +++ b/instrumentation/grpc-1.4.0/src/test/resources/enable_dist_tracing.yml @@ -0,0 +1,3 @@ +common: &default_settings + distributed_tracing: + enabled: true diff --git a/instrumentation/grpc-1.40.0/src/test/java/GrpcDistributedTracingTest.java b/instrumentation/grpc-1.40.0/src/test/java/GrpcDistributedTracingTest.java new file mode 100644 index 0000000000..f14b4edc2d --- /dev/null +++ b/instrumentation/grpc-1.40.0/src/test/java/GrpcDistributedTracingTest.java @@ -0,0 +1,67 @@ + +import app.TestClient; +import app.TestServer; +import com.newrelic.agent.introspec.*; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Ignore; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.util.Collection; + +import static org.junit.Assert.assertEquals; + +@RunWith(InstrumentationTestRunner.class) +@InstrumentationTestConfig(includePrefixes = { "io.grpc", "com.nr.agent.instrumentation.grpc" }, configName="enable_dist_tracing.yml") +public class GrpcDistributedTracingTest { + + private static TestServer server; + private static TestClient client; + + @BeforeClass + public static void before() throws Exception { + server = new TestServer(); + server.start(); + client = new TestClient("localhost", server.getPort()); + } + + @AfterClass + public static void after() throws InterruptedException { + if (client != null) { + client.shutdown(); + } + if (server != null) { + server.stop(); + } + } + + @Test + public void testDTAsyncRequest() { + client.helloAsync("Async"); + + String clientTxName = "OtherTransaction/Custom/app.TestClient/helloAsync"; + String serverTxName = "WebTransaction/gRPC/helloworld.Greeter/SayHello"; + + Introspector introspector = InstrumentationTestRunner.getIntrospector(); + + assertEquals(2, introspector.getFinishedTransactionCount(30000)); + + Collection clientTxns = introspector.getTransactionEvents(clientTxName); + assertEquals(1, clientTxns.size()); + String clientGuid = ""; + String clientTripId = ""; + for (TransactionEvent txn: clientTxns) { + //make other assertions? + clientGuid = txn.getMyGuid(); + clientTripId = txn.getTripId(); + } + + Collection serverTxns = introspector.getTransactionEvents(serverTxName); + assertEquals(1, serverTxns.size()); + for (TransactionEvent txn: serverTxns) { + assertEquals(clientGuid, txn.getParentId()); + assertEquals(clientTripId, txn.getTripId()); + } + } +} diff --git a/instrumentation/grpc-1.40.0/src/test/resources/enable_dist_tracing.yml b/instrumentation/grpc-1.40.0/src/test/resources/enable_dist_tracing.yml new file mode 100644 index 0000000000..f2f195c174 --- /dev/null +++ b/instrumentation/grpc-1.40.0/src/test/resources/enable_dist_tracing.yml @@ -0,0 +1,3 @@ +common: &default_settings + distributed_tracing: + enabled: true From 53f5c9c0ee83bcd336bd4baa25ab8b90f45f5a24 Mon Sep 17 00:00:00 2001 From: Kate Anderson Date: Tue, 5 Mar 2024 13:55:38 -0800 Subject: [PATCH 6/7] Use for-loop instead of streams in parseHeaders --- .../agent/tracing/W3CTraceParentParser.java | 16 +++++++++++++--- .../agent/tracing/W3CTraceParentHeaderTest.java | 6 ++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/tracing/W3CTraceParentParser.java b/newrelic-agent/src/main/java/com/newrelic/agent/tracing/W3CTraceParentParser.java index 69995ddf94..9c7c4e0e08 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/tracing/W3CTraceParentParser.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/tracing/W3CTraceParentParser.java @@ -17,12 +17,22 @@ public class W3CTraceParentParser { static W3CTraceParent parseHeaders(List traceParentHeaders) { - if (traceParentHeaders.size() != 1) { + if (traceParentHeaders.isEmpty()) { + return null; + } + if (traceParentHeaders.size() > 1) { ServiceFactory.getStatsService().getMetricAggregator().incrementCounter(MetricNames.SUPPORTABILITY_TRACE_CONTEXT_INVALID_PARENT_HEADER_COUNT); Agent.LOG.log(Level.WARNING, "Multiple traceparent headers found on inbound request."); // Multiple values ok if all are equal - boolean allHeadersEqual = traceParentHeaders.stream().allMatch(h -> h.equals(traceParentHeaders.get(0))); - if (traceParentHeaders.isEmpty() || !allHeadersEqual) { + boolean allHeadersEqual = true; + String first = traceParentHeaders.get(0); + for (String header : traceParentHeaders) { + if (!header.equals(first)) { + allHeadersEqual = false; + break; + } + } + if (!allHeadersEqual) { return null; } } diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/tracing/W3CTraceParentHeaderTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/tracing/W3CTraceParentHeaderTest.java index b89bdd3dfc..f32dab7034 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/tracing/W3CTraceParentHeaderTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/tracing/W3CTraceParentHeaderTest.java @@ -83,6 +83,12 @@ public void testParseMultiple_sameValues_shouldPass() { assertEquals(expected, result); } + @Test + public void testParseMultiple_empty_shouldFail() { + W3CTraceParent result = W3CTraceParentParser.parseHeaders(Arrays.asList()); + assertNull(result); + } + @Test public void testParseMultiple_butReallyJustOne() { W3CTraceParent result = W3CTraceParentParser.parseHeaders( From c5ff8506238ac8bbbdedeeaaf9688fa69db9ccb2 Mon Sep 17 00:00:00 2001 From: Kate Anderson Date: Tue, 5 Mar 2024 15:08:55 -0800 Subject: [PATCH 7/7] Remove redundant loop flag --- .../com/newrelic/agent/tracing/W3CTraceParentParser.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/tracing/W3CTraceParentParser.java b/newrelic-agent/src/main/java/com/newrelic/agent/tracing/W3CTraceParentParser.java index 9c7c4e0e08..9d47cd0da4 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/tracing/W3CTraceParentParser.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/tracing/W3CTraceParentParser.java @@ -24,17 +24,12 @@ static W3CTraceParent parseHeaders(List traceParentHeaders) { ServiceFactory.getStatsService().getMetricAggregator().incrementCounter(MetricNames.SUPPORTABILITY_TRACE_CONTEXT_INVALID_PARENT_HEADER_COUNT); Agent.LOG.log(Level.WARNING, "Multiple traceparent headers found on inbound request."); // Multiple values ok if all are equal - boolean allHeadersEqual = true; String first = traceParentHeaders.get(0); for (String header : traceParentHeaders) { if (!header.equals(first)) { - allHeadersEqual = false; - break; + return null; } } - if (!allHeadersEqual) { - return null; - } } String traceParentHeader = traceParentHeaders.get(0); return parseHeader(traceParentHeader);