Skip to content

Commit

Permalink
spanprofiler: do less work on unsampled traces (#528)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
bboreham authored May 31, 2024
1 parent 27d7d41 commit 5141842
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spanprofiler/spanprofiler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions spanprofiler/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 20 additions & 3 deletions spanprofiler/tracer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -78,15 +79,15 @@ 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.
c := &jaegercfg.Configuration{
ServiceName: "test",
Sampler: &jaegercfg.SamplerConfig{
Type: jaeger.SamplerTypeConst,
Param: 1,
Param: sampleRate,
},
Reporter: &jaegercfg.ReporterConfig{
LocalAgentHostPort: "127.0.0.100:16686",
Expand Down Expand Up @@ -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))
}

0 comments on commit 5141842

Please sign in to comment.