From 514184267d5cdd82edbababb51462d5049507552 Mon Sep 17 00:00:00 2001
From: Bryan Boreham <bjboreham@gmail.com>
Date: Fri, 31 May 2024 12:58:05 +0300
Subject: [PATCH] spanprofiler: do less work on unsampled traces (#528)

Skip the work to examine the span and decorate it, if the whole trace
is not sampled hence the data is going nowhere.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
---
 CHANGELOG.md                      |  1 +
 spanprofiler/spanprofiler_test.go |  2 +-
 spanprofiler/tracer.go            |  3 +++
 spanprofiler/tracer_test.go       | 23 ++++++++++++++++++++---
 4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a3fa0e03f..d65882366 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -210,6 +210,7 @@
 * [ENHANCEMENT] Add `outcome` label to `gate_duration_seconds` metric. Possible values are `rejected_canceled`, `rejected_deadline_exceeded`, `rejected_other`, and `permitted`. #512
 * [ENHANCEMENT] Expose `InstancesWithTokensCount` and `InstancesWithTokensInZoneCount` in `ring.ReadRing` interface. #516
 * [ENHANCEMENT] Middleware: determine route name in a single place, and add `middleware.ExtractRouteName()` method to allow consuming applications to retrieve the route name. #527
+* [ENHANCEMENT] SpanProfiler: do less work on unsampled traces. #528
 * [BUGFIX] spanlogger: Support multiple tenant IDs. #59
 * [BUGFIX] Memberlist: fixed corrupted packets when sending compound messages with more than 255 messages or messages bigger than 64KB. #85
 * [BUGFIX] Ring: `ring_member_ownership_percent` and `ring_tokens_owned` metrics are not updated on scale down. #109
diff --git a/spanprofiler/spanprofiler_test.go b/spanprofiler/spanprofiler_test.go
index 6c6974330..65d70a77f 100644
--- a/spanprofiler/spanprofiler_test.go
+++ b/spanprofiler/spanprofiler_test.go
@@ -9,7 +9,7 @@ import (
 )
 
 func TestSpanProfiler_pprof_labels_propagation(t *testing.T) {
-	tt := initTestTracer(t)
+	tt := initTestTracer(t, 1)
 	defer func() { require.NoError(t, tt.Close()) }()
 
 	t.Run("pprof labels are not propagated to child spans", func(t *testing.T) {
diff --git a/spanprofiler/tracer.go b/spanprofiler/tracer.go
index c28b52b11..e4ed2974a 100644
--- a/spanprofiler/tracer.go
+++ b/spanprofiler/tracer.go
@@ -41,6 +41,9 @@ func (t *tracer) StartSpan(operationName string, opts ...opentracing.StartSpanOp
 	if !ok {
 		return span
 	}
+	if !spanCtx.IsSampled() {
+		return span
+	}
 	// pprof labels are attached only once, at the span root level.
 	if !isRootSpan(opts...) {
 		return span
diff --git a/spanprofiler/tracer_test.go b/spanprofiler/tracer_test.go
index 71edc5c2c..c27eb16e7 100644
--- a/spanprofiler/tracer_test.go
+++ b/spanprofiler/tracer_test.go
@@ -7,13 +7,14 @@ import (
 	"testing"
 
 	"github.com/opentracing/opentracing-go"
+	"github.com/opentracing/opentracing-go/ext"
 	"github.com/stretchr/testify/require"
 	"github.com/uber/jaeger-client-go"
 	jaegercfg "github.com/uber/jaeger-client-go/config"
 )
 
 func TestTracer_pprof_labels_propagation(t *testing.T) {
-	tt := initTestTracer(t)
+	tt := initTestTracer(t, 1)
 	defer func() { require.NoError(t, tt.Close()) }()
 
 	t.Run("root span name and ID are propagated as pprof labels", func(t *testing.T) {
@@ -78,7 +79,7 @@ func TestTracer_pprof_labels_propagation(t *testing.T) {
 	})
 }
 
-func initTestTracer(t *testing.T) io.Closer {
+func initTestTracer(t testing.TB, sampleRate float64) io.Closer {
 	t.Helper()
 	// We can't use mock tracer as we actually rely on the
 	// Jaeger tracer implementation details.
@@ -86,7 +87,7 @@ func initTestTracer(t *testing.T) io.Closer {
 		ServiceName: "test",
 		Sampler: &jaegercfg.SamplerConfig{
 			Type:  jaeger.SamplerTypeConst,
-			Param: 1,
+			Param: sampleRate,
 		},
 		Reporter: &jaegercfg.ReporterConfig{
 			LocalAgentHostPort: "127.0.0.100:16686",
@@ -126,3 +127,19 @@ func spanTags(span opentracing.Span) opentracing.Tags {
 	}
 	return s.Tags()
 }
+
+func BenchmarkTracer(b *testing.B) {
+	testTag := opentracing.Tag{Key: string(ext.Component), Value: "test"}
+	run := func(samplingRate float64) func(b *testing.B) {
+		return func(b *testing.B) {
+			tt := initTestTracer(b, samplingRate)
+			defer func() { require.NoError(b, tt.Close()) }()
+
+			for i := 0; i < b.N; i++ {
+				opentracing.StartSpan("foo", ext.RPCServerOption(nil), testTag)
+			}
+		}
+	}
+	b.Run("unsampled", run(0))
+	b.Run("sampled", run(1))
+}