Skip to content

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Aug 20, 2025

Alternative to #7215

Base on changes in #7222 and #7223.

This uses the github.com/MrAlias/bind package to bind reused attributes during instrument creation. This efficiently handles sending these values on the hot-path.

Additionally, the combination of #7223 and the bind package allow the error case to be as optimized as possible. The remaining allocations are from the new set allocations like in #7215. The difference here compared to #7215 is no additional pools are needed and the call-sites are not verbose.

Benchmarks

benchstat main.txt poc-bind-telemetry-stdouttrace.txt
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/exporters/stdout/stdouttrace
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
                                        │   main.txt   │ poc-bind-telemetry-stdouttrace.txt │
                                        │    sec/op    │   sec/op     vs base               │
ExporterExportSpans/SelfObservability-8   31.35µ ± 17%   30.24µ ± 7%       ~ (p=0.143 n=10)
ExporterExportSpans/NoObservability-8     30.10µ ±  2%   30.36µ ± 2%       ~ (p=0.436 n=10)

                                        │   main.txt   │  poc-bind-telemetry-stdouttrace.txt  │
                                        │     B/op     │     B/op      vs base                │
ExporterExportSpans/SelfObservability-8   6.353Ki ± 0%   4.867Ki ± 0%  -23.38% (p=0.000 n=10)
ExporterExportSpans/NoObservability-8     4.765Ki ± 0%   4.765Ki ± 0%        ~ (p=1.000 n=10)

                                        │  main.txt  │ poc-bind-telemetry-stdouttrace.txt  │
                                        │ allocs/op  │ allocs/op   vs base                 │
ExporterExportSpans/SelfObservability-8   134.0 ± 0%   122.0 ± 0%  -8.96% (p=0.000 n=10)
ExporterExportSpans/NoObservability-8     119.0 ± 0%   119.0 ± 0%       ~ (p=1.000 n=10) ¹
¹ all samples are equal

@MrAlias MrAlias added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Aug 20, 2025
Copy link

codecov bot commented Aug 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.9%. Comparing base (25d0274) to head (01eff24).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #7226     +/-   ##
=======================================
- Coverage   83.0%   82.9%   -0.1%     
=======================================
  Files        265     265             
  Lines      24850   24811     -39     
=======================================
- Hits       20631   20589     -42     
- Misses      3839    3842      +3     
  Partials     380     380             
Files with missing lines Coverage Δ
exporters/stdout/stdouttrace/trace.go 88.8% <100.0%> (-2.7%) ⬇️

... and 2 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 poc-bind-telemetry-stdouttrace branch from 64e4484 to 221ad42 Compare August 20, 2025 21:44
MrAlias added a commit that referenced this pull request Aug 26, 2025
This is not a performance critical exporter, but it is acting as the
foundation for all other self-observability work. We want to ensure
performance is considered for all this other work.

- Do not allocate the add and record measurement option slices.
- Do not allocate the `metric.attrOpt` to head when on default path
(i.e. `WithAttributeSet` or `WithAttributes`)

There are three additional allocations in the self-observability. It
appears these are added in the error scenario where we need to
dynamically build the attributes that are being recorded.

### Benchmarks

```terminal
$ benchstat main-49be00144.txt amortize-opts-stdouttrace.txt
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/exporters/stdout/stdouttrace
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
                                        │ main-49be00144.txt │   amortize-opts-stdouttrace.txt    │
                                        │       sec/op       │   sec/op     vs base               │
ExporterExportSpans/SelfObservability-8          26.89µ ± 3%   27.40µ ± 3%       ~ (p=0.143 n=10)
ExporterExportSpans/NoObservability-8            25.99µ ± 3%   27.22µ ± 2%  +4.76% (p=0.004 n=10)

                                        │ main-49be00144.txt │    amortize-opts-stdouttrace.txt     │
                                        │        B/op        │     B/op      vs base                │
ExporterExportSpans/SelfObservability-8         5.459Ki ± 0%   4.081Ki ± 0%  -25.24% (p=0.000 n=10)
ExporterExportSpans/NoObservability-8           3.873Ki ± 0%   3.873Ki ± 0%        ~ (p=1.000 n=10)

                                        │ main-49be00144.txt │    amortize-opts-stdouttrace.txt     │
                                        │     allocs/op      │ allocs/op   vs base                  │
ExporterExportSpans/SelfObservability-8           80.00 ± 0%   67.00 ± 0%  -16.25% (p=0.000 n=10)
ExporterExportSpans/NoObservability-8             65.00 ± 0%   65.00 ± 0%        ~ (p=1.000 n=10) ¹
¹ all samples are equal
```

## Follow-up

- Investigate if the `semconv` helper packages can be updated to accept
some of this complexity (e.g. add a `AddSet` method that accepts an
`attribute.Set` instead of just `...attribute.KeyValue`).

## Alternatives

A cleaner version is found in
#7226. That
relies on an external [`bind`](https://github.com/MrAlias/bind) package
and the clean up mentioned above.
@MrAlias MrAlias force-pushed the poc-bind-telemetry-stdouttrace branch from 221ad42 to e72bf43 Compare August 26, 2025 19:49
@flc1125
Copy link
Member

flc1125 commented Aug 27, 2025

If we refer to the tracer, we could pre-set some values at creation time, like this:

package main

import (
	"context"

	"go.opentelemetry.io/otel/attribute"
	"go.opentelemetry.io/otel/metric"
	"go.opentelemetry.io/otel/trace"
)

func main() {
	var (
		tracer trace.Tracer
		meter metric.Meter
	)

	tracer.Start(
		context.TODO(),
		"operationName",
		trace.WithAttributes(attribute.String("key", "value")), // <--- Refer to a design similar to this one
	)

	meter.Float64Counter(
		"example.com/metric",
		metric.WithDescription("An example metric"),
		metric.WithAttributeSet(attribute.NewSet(...)), //
		metric.WithAttributes(attribute.String()...)),
	)
}

@MrAlias
Copy link
Contributor Author

MrAlias commented Aug 27, 2025

If we refer to the tracer, we could pre-set some values at creation time, like this:

package main

import (
	"context"

	"go.opentelemetry.io/otel/attribute"
	"go.opentelemetry.io/otel/metric"
	"go.opentelemetry.io/otel/trace"
)

func main() {
	var (
		tracer trace.Tracer
		meter metric.Meter
	)

	tracer.Start(
		context.TODO(),
		"operationName",
		trace.WithAttributes(attribute.String("key", "value")), // <--- Refer to a design similar to this one
	)

	meter.Float64Counter(
		"example.com/metric",
		metric.WithDescription("An example metric"),
		metric.WithAttributeSet(attribute.NewSet(...)), //
		metric.WithAttributes(attribute.String()...)),
	)
}

Attributes are not a part of an instrument's definition: https://github.com/open-telemetry/opentelemetry-go/blob/metric/v1.37.0/metric/syncfloat64.go#L32

We would first need to resolution on open-telemetry/opentelemetry-specification#4126 before we could change the API.

@MrAlias MrAlias force-pushed the poc-bind-telemetry-stdouttrace branch 3 times, most recently from 1885081 to 01eff24 Compare August 27, 2025 19:07
@MrAlias MrAlias mentioned this pull request Aug 28, 2025
@MrAlias MrAlias changed the title [PoC] Bind stdouttrace self-observability instruments Bind stdouttrace self-observability instruments Aug 29, 2025
@MrAlias MrAlias force-pushed the poc-bind-telemetry-stdouttrace branch from 01eff24 to bfbff08 Compare August 29, 2025 00:08
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.

2 participants