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/SelfObservabilityEnabled-8      236.9n ± 13%   177.1n ± 8%  -25.28% (p=0.000 n=10)
TraceStart/SelfObservabilityEnabled-8   856.2n ±  2%   739.8n ± 5%  -13.60% (p=0.000 n=10)
geomean                                 450.4n         361.9n       -19.65%

                                      │  main.out  │    enc-trace-sdk-tracer-obs.out    │
                                      │    B/op    │    B/op     vs base                │
SpanEnd/SelfObservabilityEnabled-8      64.00 ± 0%   48.00 ± 0%  -25.00% (p=0.000 n=10)
TraceStart/SelfObservabilityEnabled-8   608.0 ± 0%   576.0 ± 0%   -5.26% (p=0.000 n=10)
geomean                                 197.3        166.3       -15.71%

                                      │  main.out  │    enc-trace-sdk-tracer-obs.out    │
                                      │ allocs/op  │ allocs/op   vs base                │
SpanEnd/SelfObservabilityEnabled-8      2.000 ± 0%   1.000 ± 0%  -50.00% (p=0.000 n=10)
TraceStart/SelfObservabilityEnabled-8   5.000 ± 0%   3.000 ± 0%  -40.00% (p=0.000 n=10)
geomean                                 3.162        1.732       -45.23%

@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
Copy link

codecov bot commented Sep 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.9%. Comparing base (285cbe9) to head (0adfbf4).

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #7331   +/-   ##
=====================================
  Coverage   82.9%   82.9%           
=====================================
  Files        267     268    +1     
  Lines      24982   24988    +6     
=====================================
+ Hits       20728   20736    +8     
+ Misses      3878    3876    -2     
  Partials     376     376           
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 85.4% <100.0%> (+<0.1%) ⬆️
sdk/trace/tracer.go 100.0% <100.0%> (ø)

... and 3 files 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

@Copilot 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.

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.

1 participant