Skip to content

Conversation

@MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Sep 8, 2025

Split from #7316

Follow guidelines and move instrumentation into its own type.

Benchmarks

Added sdk/trace/internal/observ benchmarks

goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/trace/internal/observ
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
                     │ enc-trace-sdk-tracer-obs.out │
                     │            sec/op            │
Tracer/SpanStarted-8                    7.436n ± 6%
Tracer/SpanLive-8                       9.987n ± 8%
Tracer/SpanEnded-8                      11.32n ± 7%
NewTracer-8                             87.64n ± 6%
geomean                                 16.48n

                     │ enc-trace-sdk-tracer-obs.out │
                     │             B/op             │
Tracer/SpanStarted-8                   0.000 ± 0%
Tracer/SpanLive-8                      0.000 ± 0%
Tracer/SpanEnded-8                     0.000 ± 0%
NewTracer-8                            0.000 ± 0%
geomean                                           ¹
¹ summaries must be >0 to compute geomean

                     │ enc-trace-sdk-tracer-obs.out │
                     │          allocs/op           │
Tracer/SpanStarted-8                   0.000 ± 0%
Tracer/SpanLive-8                      0.000 ± 0%
Tracer/SpanEnded-8                     0.000 ± 0%
NewTracer-8                            0.000 ± 0%
geomean                                           ¹
¹ summaries must be >0 to compute geomean

Existing sdk/trace benchmarks

> benchstat main.out enc-trace-sdk-tracer-obs.out
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/trace
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
                                  │  main.out   │     enc-trace-sdk-tracer-obs.out     │
                                  │   sec/op    │    sec/op     vs base                │
SpanEnd/ObservabilityEnabled-8      188.5n ± 4%   131.5n ± 32%  -30.24% (p=0.000 n=10)
TraceStart/ObservabilityEnabled-8   886.9n ± 8%   663.9n ±  2%  -25.14% (p=0.000 n=10)
geomean                             408.9n        295.5n        -27.73%

                                  │  main.out  │      enc-trace-sdk-tracer-obs.out       │
                                  │    B/op    │    B/op     vs base                     │
SpanEnd/ObservabilityEnabled-8      16.00 ± 0%    0.00 ± 0%  -100.00% (p=0.000 n=10)
TraceStart/ObservabilityEnabled-8   608.0 ± 0%   576.0 ± 0%    -5.26% (p=0.000 n=10)
geomean                             98.63                    ?                       ¹ ²
¹ summaries must be >0 to compute geomean
² ratios must be >0 to compute geomean

                                  │  main.out  │      enc-trace-sdk-tracer-obs.out       │
                                  │ allocs/op  │ allocs/op   vs base                     │
SpanEnd/ObservabilityEnabled-8      1.000 ± 0%   0.000 ± 0%  -100.00% (p=0.000 n=10)
TraceStart/ObservabilityEnabled-8   5.000 ± 0%   3.000 ± 0%   -40.00% (p=0.000 n=10)
geomean                             2.236                    ?                       ¹ ²
¹ summaries must be >0 to compute geomean
² ratios must be >0 to compute geomean

@MrAlias MrAlias added this to the v1.39.0 milestone Sep 8, 2025
@MrAlias MrAlias added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Sep 8, 2025
@MrAlias MrAlias force-pushed the enc-trace-sdk-tracer-obs branch from 57eb2d4 to 6d02ae4 Compare September 8, 2025 19:29
@codecov
Copy link

codecov bot commented Sep 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.4%. Comparing base (e4ab314) to head (2373f5d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #7331     +/-   ##
=======================================
- Coverage   85.4%   85.4%   -0.1%     
=======================================
  Files        267     268      +1     
  Lines      24295   24297      +2     
=======================================
+ Hits       20758   20759      +1     
- Misses      3160    3161      +1     
  Partials     377     377             
Files with missing lines Coverage Δ
sdk/trace/internal/observ/tracer.go 100.0% <100.0%> (ø)
sdk/trace/provider.go 86.6% <100.0%> (-0.8%) ⬇️
sdk/trace/span.go 84.2% <100.0%> (-0.1%) ⬇️
sdk/trace/tracer.go 100.0% <100.0%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@MrAlias MrAlias force-pushed the enc-trace-sdk-tracer-obs branch 2 times, most recently from 58591c0 to cccf667 Compare September 8, 2025 20:04
@MrAlias MrAlias force-pushed the enc-trace-sdk-tracer-obs branch from cccf667 to 10b4c11 Compare September 8, 2025 20:36
@MrAlias MrAlias marked this pull request as ready for review September 8, 2025 20:43
@pellared pellared requested a review from Copilot September 9, 2025 18:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR encapsulates SDK Tracer observability functionality by moving instrumentation logic from the main tracer type into a dedicated internal observability package. The refactoring improves maintainability by separating concerns and provides better performance through optimized attribute caching.

  • Extracts observability instrumentation into a new sdk/trace/internal/observ package
  • Replaces direct metric handling in tracer with a dedicated observ.Tracer type
  • Updates all references to use the new observability constants and interfaces

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
sdk/trace/tracer.go Replaces inline observability fields and logic with observ.Tracer instance
sdk/trace/provider.go Updates tracer creation to use new observ.NewTracer() constructor
sdk/trace/span.go Simplifies span end observability calls using new interface
sdk/trace/internal/observ/tracer.go New observability tracer implementation with optimized attribute caching
sdk/trace/internal/observ/tracer_test.go Comprehensive tests for the new observability functionality
sdk/trace/trace_test.go Updates test expectations to use new observability scope constants
sdk/trace/provider_test.go Adjusts test assertions for new observability interface
.codespellignore Adds "observ" to ignored words list

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@MrAlias MrAlias merged commit 5b808c6 into open-telemetry:main Sep 15, 2025
30 of 32 checks passed
@MrAlias MrAlias deleted the enc-trace-sdk-tracer-obs branch September 15, 2025 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skip Changelog PRs that do not require a CHANGELOG.md entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants