Fix Jaeger exporter data corruption (#1963)

* Fix Jaeger exporter data corruption when splitting spans to batches

* Update Jaeger exporter changelog

* Add unit test for verifying Batch span data in Jaeger exporter

Co-authored-by: Cijo Thomas <cithomas@microsoft.com>

src/OpenTelemetry.Exporter.Jaeger/CHANGELOG.md

@@ -7,6 +7,8 @@
   (ex:
   `services.Configure<JaegerExporterOptions>(this.Configuration.GetSection("Jaeger"));`).
   ([#1889](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1889))
+* Fixed data corruption when creating Jaeger Batch messages
+  ([#1372](https://github.com/open-telemetry/opentelemetry-dotnet/issues/1372))
 
 ## 1.1.0-beta1
 

src/OpenTelemetry.Exporter.Jaeger/JaegerExporter.cs

@@ -158,6 +158,13 @@ internal void AppendSpan(JaegerSpan jaegerSpan)
             if (this.batchByteSize + spanTotalBytesNeeded >= this.maxPayloadSizeInBytes)
             {
                 this.SendCurrentBatch();
+
+                // SendCurrentBatch clears/invalidates the BufferWriter in InMemoryTransport.
+                // The new spanMessage is still located in it, though. It might get overwritten later
+                // when spans are written in the buffer for the next batch.
+                // Move spanMessage to the beginning of the BufferWriter to avoid data corruption.
+                this.memoryTransport.Write(spanMessage.BufferWriter.Buffer, spanMessage.Offset, spanMessage.Count);
+                spanMessage = this.memoryTransport.ToBuffer();
             }
 
             this.Batch.Add(spanMessage);

test/OpenTelemetry.Exporter.Jaeger.Tests/JaegerExporterTests.cs

@@ -17,10 +17,12 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Linq;
 using OpenTelemetry.Exporter.Jaeger.Implementation;
 using OpenTelemetry.Exporter.Jaeger.Tests.Implementation;
 using OpenTelemetry.Resources;
 using OpenTelemetry.Trace;
+using Thrift.Protocol;
 using Xunit;
 
 namespace OpenTelemetry.Exporter.Jaeger.Tests
@@ -128,6 +130,77 @@ public void JaegerTraceExporter_BuildBatchesToTransmit_FlushedBatch()
             Assert.Equal(1, jaegerExporter.Batch.Count);
         }
 
+        [Fact]
+        public void JaegerTraceExporter_SpansSplitToBatches_SpansIncludedInBatches()
+        {
+            // Arrange
+            var memoryTransport = new InMemoryTransport();
+            using var jaegerExporter = new JaegerExporter(
+                new JaegerExporterOptions { MaxPayloadSizeInBytes = 1500 }, memoryTransport);
+            jaegerExporter.SetResourceAndInitializeBatch(Resource.Empty);
+
+            var tempTransport = new InMemoryTransport(initialCapacity: 3000);
+            var protocol = new TCompactProtocol(tempTransport);
+
+            // Create six spans, each taking more space than the previous one
+            var spans = new JaegerSpan[6];
+            for (int i = 0; i < 6; i++)
+            {
+                spans[i] = CreateTestJaegerSpan(
+                    additionalAttributes: new Dictionary<string, object>
+                    {
+                        ["foo"] = new string('_', 10 * i),
+                    });
+            }
+
+            var serializedSpans = spans.Select(s =>
+            {
+                s.Write(protocol);
+                return tempTransport.ToArray();
+            }).ToArray();
+
+            // Act
+            var sentBatches = new List<byte[]>();
+            foreach (var span in spans)
+            {
+                jaegerExporter.AppendSpan(span);
+                var sentBatch = memoryTransport.ToArray();
+                if (sentBatch.Length > 0)
+                {
+                    sentBatches.Add(sentBatch);
+                }
+            }
+
+            // Assert
+
+            // Appending the six spans will send two batches with the first four spans
+            Assert.Equal(2, sentBatches.Count);
+            Assert.True(
+                ContainsSequence(sentBatches[0], serializedSpans[0]),
+                "Expected span data not found in sent batch");
+            Assert.True(
+                ContainsSequence(sentBatches[0], serializedSpans[1]),
+                "Expected span data not found in sent batch");
+
+            Assert.True(
+                ContainsSequence(sentBatches[1], serializedSpans[2]),
+                "Expected span data not found in sent batch");
+            Assert.True(
+                ContainsSequence(sentBatches[1], serializedSpans[3]),
+                "Expected span data not found in sent batch");
+
+            // jaegerExporter.Batch should contain the two remaining spans
+            Assert.Equal(2, jaegerExporter.Batch.Count);
+            jaegerExporter.Batch.Write(protocol);
+            var serializedBatch = tempTransport.ToArray();
+            Assert.True(
+                ContainsSequence(serializedBatch, serializedSpans[4]),
+                "Expected span data not found in unsent batch");
+            Assert.True(
+                ContainsSequence(serializedBatch, serializedSpans[5]),
+                "Expected span data not found in unsent batch");
+        }
+
         internal static JaegerSpan CreateTestJaegerSpan(
             bool setAttributes = true,
             Dictionary<string, object> additionalAttributes = null,
@@ -141,5 +214,18 @@ internal static JaegerSpan CreateTestJaegerSpan(
                     setAttributes, additionalAttributes, addEvents, addLinks, resource, kind)
                 .ToJaegerSpan();
         }
+
+        private static bool ContainsSequence(byte[] source, byte[] pattern)
+        {
+            for (var start = 0; start < (source.Length - pattern.Length + 1); start++)
+            {
+                if (source.Skip(start).Take(pattern.Length).SequenceEqual(pattern))
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
     }
 }