From c3c050aa792ebefb1a4e67930cfa19fd7e445333 Mon Sep 17 00:00:00 2001
From: cthuang <chungting@cloudflare.com>
Date: Tue, 18 Oct 2022 12:15:51 +0100
Subject: [PATCH] TUN-6867: Clear spans right after they are serialized to
 avoid returning duplicate spans

---
 ingress/packet_router.go |  2 --
 tracing/client.go        | 25 +++++++++++--------------
 tracing/tracing.go       |  6 +-----
 3 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/ingress/packet_router.go b/ingress/packet_router.go
index 1b311aa5..9d08b4f9 100644
--- a/ingress/packet_router.go
+++ b/ingress/packet_router.go
@@ -176,8 +176,6 @@ func (pr *packetResponder) exportSpan() {
 	}
 	spans := pr.tracedCtx.GetProtoSpans()
 	if len(spans) > 0 {
-		// Make sure spans are cleared after they are sent
-		defer pr.tracedCtx.ClearSpans()
 		pr.datagramMuxer.SendPacket(&quicpogs.TracingSpanPacket{
 			Spans:           spans,
 			TracingIdentity: pr.serializedIdentity,
diff --git a/tracing/client.go b/tracing/client.go
index 81d31f05..28a61826 100644
--- a/tracing/client.go
+++ b/tracing/client.go
@@ -24,11 +24,9 @@ type InMemoryClient interface {
 	// Spans returns a copy of the list of in-memory stored spans as a base64
 	// encoded otlp protobuf string.
 	Spans() (string, error)
-	// ProtoSpans returns a copy of the list of in-memory stored spans as otlp
-	// protobuf byte array.
-	ProtoSpans() ([]byte, error)
-	// Clear spans removes all in-memory spans
-	ClearSpans()
+	// ExportProtoSpans returns a copy of the list of in-memory stored spans as otlp
+	// protobuf byte array and clears the in-memory spans.
+	ExportProtoSpans() ([]byte, error)
 }
 
 // InMemoryOtlpClient is a client implementation for otlptrace.Client
@@ -60,7 +58,7 @@ func (mc *InMemoryOtlpClient) UploadTraces(_ context.Context, protoSpans []*trac
 
 // Spans returns the list of in-memory stored spans as a base64 encoded otlp protobuf string.
 func (mc *InMemoryOtlpClient) Spans() (string, error) {
-	data, err := mc.ProtoSpans()
+	data, err := mc.ExportProtoSpans()
 	if err != nil {
 		return "", err
 	}
@@ -68,7 +66,7 @@ func (mc *InMemoryOtlpClient) Spans() (string, error) {
 }
 
 // ProtoSpans returns the list of in-memory stored spans as the protobuf byte array.
-func (mc *InMemoryOtlpClient) ProtoSpans() ([]byte, error) {
+func (mc *InMemoryOtlpClient) ExportProtoSpans() ([]byte, error) {
 	mc.mu.Lock()
 	defer mc.mu.Unlock()
 	if len(mc.spans) <= 0 {
@@ -77,13 +75,12 @@ func (mc *InMemoryOtlpClient) ProtoSpans() ([]byte, error) {
 	pbRequest := &coltracepb.ExportTraceServiceRequest{
 		ResourceSpans: mc.spans,
 	}
-	return proto.Marshal(pbRequest)
-}
-
-func (mc *InMemoryOtlpClient) ClearSpans() {
-	mc.mu.Lock()
-	defer mc.mu.Unlock()
+	serializedSpans, err := proto.Marshal(pbRequest)
+	if err != nil {
+		return nil, err
+	}
 	mc.spans = make([]*tracepb.ResourceSpans, 0)
+	return serializedSpans, nil
 }
 
 // NoopOtlpClient is a client implementation for otlptrace.Client that does nothing
@@ -107,7 +104,7 @@ func (mc *NoopOtlpClient) Spans() (string, error) {
 }
 
 // Spans always returns no traces error
-func (mc *NoopOtlpClient) ProtoSpans() ([]byte, error) {
+func (mc *NoopOtlpClient) ExportProtoSpans() ([]byte, error) {
 	return nil, errNoopTracer
 }
 
diff --git a/tracing/tracing.go b/tracing/tracing.go
index 6cd59134..57744120 100644
--- a/tracing/tracing.go
+++ b/tracing/tracing.go
@@ -157,7 +157,7 @@ func (cft *cfdTracer) GetSpans() (enc string) {
 
 // GetProtoSpans returns the spans as the otlp traces in protobuf byte array.
 func (cft *cfdTracer) GetProtoSpans() (proto []byte) {
-	proto, err := cft.exporter.ProtoSpans()
+	proto, err := cft.exporter.ExportProtoSpans()
 	switch err {
 	case nil:
 		break
@@ -189,10 +189,6 @@ func (cft *cfdTracer) AddSpans(headers http.Header) {
 	headers[CanonicalCloudflaredTracingHeader] = []string{enc}
 }
 
-func (cft *cfdTracer) ClearSpans() {
-	cft.exporter.ClearSpans()
-}
-
 // End will set the OK status for the span and then end it.
 func End(span trace.Span) {
 	endSpan(span, -1, codes.Ok, nil)