From 88d3fedf1a323c928c5ffcd7b9d8ab908b0121b3 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 25 Sep 2025 09:07:15 -0700 Subject: [PATCH] Optimize the return type of ExportSpans (#7405) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Do not allocate a return function from `ExportSpans` to the heap. Use the added `ExportOp` type instead. ### Benchmarks #### `stdouttrace` ```terminal goos: linux goarch: amd64 pkg: go.opentelemetry.io/otel/exporters/stdout/stdouttrace cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz │ main.bmark.results │ stdouttrace-optimize-end.bmark.results │ │ sec/op │ sec/op vs base │ ExporterExportSpans/Observability-8 23.37µ ± 2% 22.79µ ± 3% -2.50% (p=0.025 n=10) ExporterExportSpans/NoObservability-8 23.07µ ± 7% 22.29µ ± 1% -3.38% (p=0.000 n=10) geomean 23.22µ 22.54µ -2.94% │ main.bmark.results │ stdouttrace-optimize-end.bmark.results │ │ B/op │ B/op vs base │ ExporterExportSpans/Observability-8 4.253Ki ± 0% 4.190Ki ± 0% -1.47% (p=0.000 n=10) ExporterExportSpans/NoObservability-8 3.975Ki ± 0% 3.975Ki ± 0% ~ (p=0.474 n=10) geomean 4.111Ki 4.081Ki -0.74% │ main.bmark.results │ stdouttrace-optimize-end.bmark.results │ │ allocs/op │ allocs/op vs base │ ExporterExportSpans/Observability-8 67.00 ± 0% 66.00 ± 0% -1.49% (p=0.000 n=10) ExporterExportSpans/NoObservability-8 64.00 ± 0% 64.00 ± 0% ~ (p=1.000 n=10) ¹ geomean 65.48 64.99 -0.75% ¹ all samples are equal ``` #### `stdouttrace/internal/observ` ```terminal goos: linux goarch: amd64 pkg: go.opentelemetry.io/otel/exporters/stdout/stdouttrace/internal/observ cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz │ main.bmark.results │ stdouttrace-optimize-end.bmark.results │ │ sec/op │ sec/op vs base │ InstrumentationExportSpans/NoError-8 197.9n ± 7% 153.3n ± 5% -22.51% (p=0.000 n=10) InstrumentationExportSpans/PartialError-8 754.4n ± 6% 663.2n ± 6% -12.08% (p=0.001 n=10) InstrumentationExportSpans/FullError-8 772.8n ± 4% 669.2n ± 4% -13.39% (p=0.000 n=10) geomean 486.8n 408.3n -16.13% │ main.bmark.results │ stdouttrace-optimize-end.bmark.results │ │ B/op │ B/op vs base │ InstrumentationExportSpans/NoError-8 64.00 ± 0% 0.00 ± 0% -100.00% (p=0.000 n=10) InstrumentationExportSpans/PartialError-8 280.0 ± 0% 216.0 ± 0% -22.86% (p=0.000 n=10) InstrumentationExportSpans/FullError-8 280.0 ± 0% 216.0 ± 0% -22.86% (p=0.000 n=10) geomean 171.2 ? ¹ ² ¹ summaries must be >0 to compute geomean ² ratios must be >0 to compute geomean │ main.bmark.results │ stdouttrace-optimize-end.bmark.results │ │ allocs/op │ allocs/op vs base │ InstrumentationExportSpans/NoError-8 1.000 ± 0% 0.000 ± 0% -100.00% (p=0.000 n=10) InstrumentationExportSpans/PartialError-8 3.000 ± 0% 2.000 ± 0% -33.33% (p=0.000 n=10) InstrumentationExportSpans/FullError-8 3.000 ± 0% 2.000 ± 0% -33.33% (p=0.000 n=10) geomean 2.080 ? ¹ ² ¹ summaries must be >0 to compute geomean ² ratios must be >0 to compute geomean ``` --- .../internal/observ/instrumentation.go | 103 ++++++++++-------- .../internal/observ/instrumentation_test.go | 45 ++++---- exporters/stdout/stdouttrace/trace.go | 4 +- 3 files changed, 84 insertions(+), 68 deletions(-) diff --git a/exporters/stdout/stdouttrace/internal/observ/instrumentation.go b/exporters/stdout/stdouttrace/internal/observ/instrumentation.go index abcb86e86..051fe6ed6 100644 --- a/exporters/stdout/stdouttrace/internal/observ/instrumentation.go +++ b/exporters/stdout/stdouttrace/internal/observ/instrumentation.go @@ -145,16 +145,9 @@ func NewInstrumentation(id int64) (*Instrumentation, error) { return i, err } -// ExportSpansDone is a function that is called when a call to an Exporter's -// ExportSpans method completes. -// -// The number of successful exports is provided as success. Any error that is -// encountered is provided as err. -type ExportSpansDone func(success int64, err error) - // ExportSpans instruments the ExportSpans method of the exporter. It returns a // function that needs to be deferred so it is called when the method returns. -func (i *Instrumentation) ExportSpans(ctx context.Context, nSpans int) ExportSpansDone { +func (i *Instrumentation) ExportSpans(ctx context.Context, nSpans int) ExportOp { start := time.Now() addOpt := get[metric.AddOption](addOptPool) @@ -162,44 +155,60 @@ func (i *Instrumentation) ExportSpans(ctx context.Context, nSpans int) ExportSpa *addOpt = append(*addOpt, i.setOpt) i.inflightSpans.Add(ctx, int64(nSpans), *addOpt...) - return i.end(ctx, start, int64(nSpans)) -} - -func (i *Instrumentation) end(ctx context.Context, start time.Time, n int64) ExportSpansDone { - return func(success int64, err error) { - addOpt := get[metric.AddOption](addOptPool) - defer put(addOptPool, addOpt) - *addOpt = append(*addOpt, i.setOpt) - - i.inflightSpans.Add(ctx, -n, *addOpt...) - - // Record the success and duration of the operation. - // - // Do not exclude 0 values, as they are valid and indicate no spans - // were exported which is meaningful for certain aggregations. - i.exportedSpans.Add(ctx, success, *addOpt...) - - mOpt := i.setOpt - if err != nil { - attrs := get[attribute.KeyValue](measureAttrsPool) - defer put(measureAttrsPool, attrs) - *attrs = append(*attrs, i.attrs...) - *attrs = append(*attrs, semconv.ErrorType(err)) - - // Do not inefficiently make a copy of attrs by using - // WithAttributes instead of WithAttributeSet. - set := attribute.NewSet(*attrs...) - mOpt = metric.WithAttributeSet(set) - - // Reset addOpt with new attribute set. - *addOpt = append((*addOpt)[:0], mOpt) - - i.exportedSpans.Add(ctx, n-success, *addOpt...) - } - - recordOpt := get[metric.RecordOption](recordOptPool) - defer put(recordOptPool, recordOpt) - *recordOpt = append(*recordOpt, mOpt) - i.opDuration.Record(ctx, time.Since(start).Seconds(), *recordOpt...) + return ExportOp{ + ctx: ctx, + start: start, + nSpans: int64(nSpans), + inst: i, } } + +// ExportOp is an in-progress ExportSpans operation. +type ExportOp struct { + ctx context.Context + start time.Time + nSpans int64 + inst *Instrumentation +} + +// End ends the ExportSpans operation, recording its success and duration. +// +// The success parameter indicates how many spans were successfully exported. +// The err parameter indicates whether the operation failed. If err is not nil, +// the number of failed spans (nSpans - success) is also recorded. +func (e ExportOp) End(success int64, err error) { + addOpt := get[metric.AddOption](addOptPool) + defer put(addOptPool, addOpt) + *addOpt = append(*addOpt, e.inst.setOpt) + + e.inst.inflightSpans.Add(e.ctx, -e.nSpans, *addOpt...) + + // Record the success and duration of the operation. + // + // Do not exclude 0 values, as they are valid and indicate no spans + // were exported which is meaningful for certain aggregations. + e.inst.exportedSpans.Add(e.ctx, success, *addOpt...) + + mOpt := e.inst.setOpt + if err != nil { + attrs := get[attribute.KeyValue](measureAttrsPool) + defer put(measureAttrsPool, attrs) + *attrs = append(*attrs, e.inst.attrs...) + *attrs = append(*attrs, semconv.ErrorType(err)) + + // Do not inefficiently make a copy of attrs by using + // WithAttributes instead of WithAttributeSet. + set := attribute.NewSet(*attrs...) + mOpt = metric.WithAttributeSet(set) + + // Reset addOpt with new attribute set. + *addOpt = append((*addOpt)[:0], mOpt) + + e.inst.exportedSpans.Add(e.ctx, e.nSpans-success, *addOpt...) + } + + recordOpt := get[metric.RecordOption](recordOptPool) + defer put(recordOptPool, recordOpt) + *recordOpt = append(*recordOpt, mOpt) + e.inst.opDuration.Record(e.ctx, time.Since(e.start).Seconds(), *recordOpt...) +} diff --git a/exporters/stdout/stdouttrace/internal/observ/instrumentation_test.go b/exporters/stdout/stdouttrace/internal/observ/instrumentation_test.go index db1499c6e..1235d9f17 100644 --- a/exporters/stdout/stdouttrace/internal/observ/instrumentation_test.go +++ b/exporters/stdout/stdouttrace/internal/observ/instrumentation_test.go @@ -190,8 +190,7 @@ func TestInstrumentationExportSpans(t *testing.T) { inst, collect := setup(t) const n = 10 - end := inst.ExportSpans(t.Context(), n) - end(n, nil) + inst.ExportSpans(t.Context(), n).End(n, nil) assertMetrics(t, collect(), n, n, nil) } @@ -200,9 +199,8 @@ func TestInstrumentationExportSpansAllErrored(t *testing.T) { inst, collect := setup(t) const n = 10 - end := inst.ExportSpans(t.Context(), n) const success = 0 - end(success, assert.AnError) + inst.ExportSpans(t.Context(), n).End(success, assert.AnError) assertMetrics(t, collect(), n, success, assert.AnError) } @@ -211,28 +209,37 @@ func TestInstrumentationExportSpansPartialErrored(t *testing.T) { inst, collect := setup(t) const n = 10 - end := inst.ExportSpans(t.Context(), n) const success = 5 - end(success, assert.AnError) + inst.ExportSpans(t.Context(), n).End(success, assert.AnError) assertMetrics(t, collect(), n, success, assert.AnError) } func BenchmarkInstrumentationExportSpans(b *testing.B) { - b.Setenv("OTEL_GO_X_OBSERVABILITY", "true") - inst, err := observ.NewInstrumentation(ID) - if err != nil { - b.Fatalf("failed to create instrumentation: %v", err) + setup := func(b *testing.B) *observ.Instrumentation { + b.Helper() + b.Setenv("OTEL_GO_X_OBSERVABILITY", "true") + inst, err := observ.NewInstrumentation(ID) + if err != nil { + b.Fatalf("failed to create instrumentation: %v", err) + } + return inst } - var end observ.ExportSpansDone - err = errors.New("benchmark error") - - b.ReportAllocs() - b.ResetTimer() - for b.Loop() { - end = inst.ExportSpans(b.Context(), 10) - end(4, err) + const nSpans = 10 + err := errors.New("benchmark error") + run := func(n int64, err error) func(*testing.B) { + return func(b *testing.B) { + inst := setup(b) + b.ReportAllocs() + b.ResetTimer() + for b.Loop() { + inst.ExportSpans(b.Context(), nSpans).End(n, err) + } + } } - _ = end + + b.Run("NoError", run(nSpans, nil)) + b.Run("PartialError", run(4, err)) + b.Run("FullError", run(0, err)) } diff --git a/exporters/stdout/stdouttrace/trace.go b/exporters/stdout/stdouttrace/trace.go index b1dd6cb65..822ed5d7e 100644 --- a/exporters/stdout/stdouttrace/trace.go +++ b/exporters/stdout/stdouttrace/trace.go @@ -56,8 +56,8 @@ type Exporter struct { func (e *Exporter) ExportSpans(ctx context.Context, spans []trace.ReadOnlySpan) (err error) { var success int64 if e.inst != nil { - end := e.inst.ExportSpans(ctx, len(spans)) - defer func() { end(success, err) }() + op := e.inst.ExportSpans(ctx, len(spans)) + defer func() { op.End(success, err) }() } if err := ctx.Err(); err != nil {