-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Bind stdouttrace
self-observability instruments
#7226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Bind stdouttrace
self-observability instruments
#7226
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ 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
🚀 New features to boost your workflow:
|
64e4484
to
221ad42
Compare
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.
221ad42
to
e72bf43
Compare
If we refer to the 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. |
1885081
to
01eff24
Compare
stdouttrace
self-observability instrumentsstdouttrace
self-observability instruments
01eff24
to
bfbff08
Compare
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