From 790a3b75dd7609d76638ea3625a9289f58b24378 Mon Sep 17 00:00:00 2001
From: Jacob Affinito <jaffinito@newrelic.com>
Date: Fri, 14 Jul 2023 15:32:26 -0700
Subject: [PATCH] fix: Remove invalid trailing comma added to W3C tracestate
 header. (#1779)

---
 .../DistributedTracePayloadHandler.cs         | 14 ++++--
 .../DistributedTracePayloadHandlerTests.cs    | 49 +++++++++++++++++++
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/src/Agent/NewRelic/Agent/Core/DistributedTracing/DistributedTracePayloadHandler.cs b/src/Agent/NewRelic/Agent/Core/DistributedTracing/DistributedTracePayloadHandler.cs
index d6e93a7fd..b9e44515d 100644
--- a/src/Agent/NewRelic/Agent/Core/DistributedTracing/DistributedTracePayloadHandler.cs
+++ b/src/Agent/NewRelic/Agent/Core/DistributedTracing/DistributedTracePayloadHandler.cs
@@ -12,6 +12,7 @@
 using NewRelic.Core.Logging;
 using System;
 using System.Collections.Generic;
+using System.Linq;
 
 namespace NewRelic.Agent.Core.DistributedTracing
 {
@@ -162,12 +163,15 @@ private string BuildTracestate(IInternalTransaction transaction, DateTime timest
             var newRelicTracestate = $"{accountKey}@nr={version}-{parentType}-{parentAccountId}-{appId}-{spanId}-{transactionId}-{sampled}-{priority}-{timestampInMillis}";
             var otherVendorTracestates = string.Empty;
 
-            if (transaction.TracingState != null)
+            if (transaction.TracingState?.VendorStateEntries != null && transaction.TracingState.VendorStateEntries.Any())
             {
-                if (transaction.TracingState.VendorStateEntries != null)
-                {
-                    otherVendorTracestates = string.Join(",", transaction.TracingState.VendorStateEntries);
-                }
+                otherVendorTracestates = string.Join(",", transaction.TracingState.VendorStateEntries);
+            }
+
+            // If otherVendorTracestates is null/empty we get a trailing comma.
+            if (string.IsNullOrWhiteSpace(otherVendorTracestates))
+            {
+                return newRelicTracestate;
             }
 
             return string.Join(",", newRelicTracestate, otherVendorTracestates);
diff --git a/tests/Agent/UnitTests/Core.UnitTest/Wrapper/AgentWrapperApi/DistributedTracing/DistributedTracePayloadHandlerTests.cs b/tests/Agent/UnitTests/Core.UnitTest/Wrapper/AgentWrapperApi/DistributedTracing/DistributedTracePayloadHandlerTests.cs
index 587069e36..7da66bb8a 100644
--- a/tests/Agent/UnitTests/Core.UnitTest/Wrapper/AgentWrapperApi/DistributedTracing/DistributedTracePayloadHandlerTests.cs
+++ b/tests/Agent/UnitTests/Core.UnitTest/Wrapper/AgentWrapperApi/DistributedTracing/DistributedTracePayloadHandlerTests.cs
@@ -1031,6 +1031,55 @@ public void TraceIdShouldBeSameForAllSpansWhenNoTraceIdReceived()
 
         #endregion TraceID Tests
 
+        [TestCase(true)]
+        [TestCase(true, "")]
+        [TestCase(true, "k1=v1", "k2=v2")]
+        [TestCase(false)]
+        [TestCase(false, "")]
+        [TestCase(false, "k1=v1", "k2=v2")]
+        public void W3C_BuildTracestate_EmptyVendors_NoCommas(bool hasIncomingPayload, params string[] vendorState)
+        {
+            // Arrange
+            Mock.Arrange(() => _configuration.SpanEventsEnabled).Returns(true);
+            Mock.Arrange(() => _configuration.PayloadSuccessMetricsEnabled).Returns(true);
+
+            var transaction = BuildMockTransaction(hasIncomingPayload: hasIncomingPayload, sampled: true);
+
+            var transactionGuid = GuidGenerator.GenerateNewRelicGuid();
+            Mock.Arrange(() => transaction.Guid).Returns(transactionGuid);
+
+            var expectedSpanGuid = GuidGenerator.GenerateNewRelicGuid();
+            var segment = Mock.Create<ISegment>();
+            Mock.Arrange(() => segment.SpanId).Returns(expectedSpanGuid);
+
+            Mock.Arrange(() => transaction.CurrentSegment).Returns(segment);
+
+            var headers = new List<KeyValuePair<string, string>>();
+            var setHeaders = new Action<List<KeyValuePair<string, string>>, string, string>((carrier, key, value) =>
+            {
+                carrier.Add(new KeyValuePair<string, string>(key, value));
+            });
+
+            var tracingState = Mock.Create<ITracingState>();
+
+            var vendorStateEntries = vendorState.ToList();
+
+            Mock.Arrange(() => tracingState.VendorStateEntries).Returns(vendorStateEntries);
+            Mock.Arrange(() => transaction.TracingState).Returns(tracingState);
+
+            Mock.Arrange(() => transaction.InsertDistributedTraceHeaders(
+                Arg.IsAny<List<KeyValuePair<string, string>>>(),
+                Arg.IsAny<Action<List<KeyValuePair<string, string>>, string, string>>()))
+                    .DoInstead(() => _distributedTracePayloadHandler.InsertDistributedTraceHeaders(transaction, headers, setHeaders));
+
+            // Act
+            transaction.InsertDistributedTraceHeaders(headers, setHeaders);
+
+            var tracestateHeaderValue = headers.Where(header => header.Key == TracestateHeaderName).Select(header => header.Value).FirstOrDefault();
+
+            Assert.That(!tracestateHeaderValue.EndsWith(","), "W3C Tracestate string has a trailing comma.");
+        }
+
         #endregion
 
         #region Supportability Metrics