-
Notifications
You must be signed in to change notification settings - Fork 1.2k
stdoutmetric: self-observability: exporter metrics #7492
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?
stdoutmetric: self-observability: exporter metrics #7492
Conversation
…GO_X_SELF_OBSERVABILITY
…etrics 1. otel.sdk.exporter.metric_data_point.inflight 2. otel.sdk.exporter.metric_data_point.exported 3. otel.sdk.exporter.operation.duration
- use pool to amortize slice allocation - pass actual context - use t.Cleanup instead of defer in tests - improve readability by returning without using err var
- use metricdatatest for comparision in testcase
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7492 +/- ##
======================================
Coverage 86.3% 86.3%
======================================
Files 294 298 +4
Lines 25987 26129 +142
======================================
+ Hits 22431 22557 +126
- Misses 3183 3195 +12
- Partials 373 377 +4
🚀 New features to boost your workflow:
|
… `OTEL_GO_X_SELF_OBSERVABILITY` with `OTEL_GO_X_OBSERVABILITY`
Benchmarks exporter_1.txt (commit=c106988)
exporter_2.txt (commit=ff814ca)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be continued
// to the case-insensitive string value of "true" (i.e. "True" and "TRUE" | ||
// will also enable this). | ||
var Observability = newFeature( | ||
[]string{"OBSERVABILITY", "SELF_OBSERVABILITY"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[]string{"OBSERVABILITY", "SELF_OBSERVABILITY"}, | |
[]string{"OBSERVABILITY"}, |
|
||
const altKey = "OTEL_GO_X_SELF_OBSERVABILITY" | ||
require.Contains(t, Observability.Keys(), altKey) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const altKey = "OTEL_GO_X_SELF_OBSERVABILITY" | |
require.Contains(t, Observability.Keys(), altKey) |
Since it is a new component, we do not need to support SELF_OBSERVABILITY
.
mp := otel.GetMeterProvider() | ||
m := mp.Meter( | ||
scope, | ||
metric.WithInstrumentationVersion(sdk.Version()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a Version
that should follow the version of its parent module (go.opentelemetry.io/otel/exporters/stdout/stdoutmetric).
Some reference approaches:
const Version = "1.38.0" Version = internal.Version opentelemetry-go/versions.yaml
Lines 46 to 48 in 73cbc69
go.opentelemetry.io/otel/exporters/stdout/stdouttrace: version-refs: - ./exporters/stdout/stdouttrace/internal/version.go
return em, err | ||
} | ||
|
||
func (em *Instrumentation) TrackExport(ctx context.Context, count int64) func(err error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can refer to this PR and return a struct to improve performance:
- Optimize the return type of ExportSpans #7405
opentelemetry-go/exporters/stdout/stdouttrace/internal/observ/instrumentation.go
Lines 167 to 172 in 73cbc69
type ExportOp struct { ctx context.Context start time.Time nSpans int64 inst *Instrumentation }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think naming this file instrumentation.go
might be better, and the same applies to the unit tests.
func (e *exporter) Export(ctx context.Context, data *metricdata.ResourceMetrics) error { | ||
if err := ctx.Err(); err != nil { | ||
func (e *exporter) Export(ctx context.Context, data *metricdata.ResourceMetrics) (err error) { | ||
trackExportFunc := e.trackExport(ctx, countDataPoints(data)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we can directly use the following approach without defining a function (even if not enabled, it still incurs overhead because it returns an empty anonymous function).
if e.inst != nil {
// ...
}
em.inflight.Add(ctx, -count, em.addOpts...) | ||
if err == nil { // short circuit in case of success to avoid allocations | ||
em.exported.Int64Counter.Add(ctx, count, em.addOpts...) | ||
em.duration.Float64Histogram.Record(ctx, durationSeconds, em.recordOpts...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
em.duration.Float64Histogram.Record(ctx, durationSeconds, em.recordOpts...) | |
em.duration.Inst().Record(ctx, durationSeconds, em.recordOpts...) |
durationSeconds := time.Since(begin).Seconds() | ||
em.inflight.Add(ctx, -count, em.addOpts...) | ||
if err == nil { // short circuit in case of success to avoid allocations | ||
em.exported.Int64Counter.Add(ctx, count, em.addOpts...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
em.exported.Int64Counter.Add(ctx, count, em.addOpts...) | |
em.exported.Inst().Add(ctx, count, em.addOpts...) |
Fixes #7014
This PR adds support for below self observability metrics for stdoutmetric exporter
These metrics are experimental and hence behind a feature flag OTEL_GO_X_OBSERVABILITY.
Definition of above metrics is available at https://github.com/open-telemetry/semantic-conventions/blob/v1.36.0/docs/otel/sdk-metrics.md
Observability Implementation Checklist
Observability Implementation Checklist
Based on the project Observability guidelines, ensure the following are completed:
Environment Variable Activation
OTEL_GO_X_OBSERVABILITY
environment variablex.Observability.Enabled()
check 1Encapsulation
struct
(e.g.,Instrumentation
)Initialization
otel.GetMeterProvider()
)Version
)SchemaURL
)Performance
Attribute and Option Allocation Management
sync.Pool
for attribute slices and options with dynamic attributesCaching
Benchmarking
b.ReportAllocs()
in benchmarks)Error Handling and Robustness
otel.Handle()
otel.Handle()
only when component cannot report error to userContext Propagation
context.Background()
)Semantic Conventions Compliance
otelconv
convenience package for metric semantic conventionsTesting
t.Cleanup()
)t.Setenv()
for environment variable testingFootnotes
https://github.com/open-telemetry/opentelemetry-go/blob/e4ab3141123d0811125a69823dbbe4d9ec5a9b8f/exporters/stdout/stdouttrace/internal/observ/instrumentation.go#L101-L103 ↩
https://github.com/open-telemetry/opentelemetry-go/blob/e4ab3141123d0811125a69823dbbe4d9ec5a9b8f/exporters/stdout/stdouttrace/internal/x/x.go ↩
https://github.com/open-telemetry/opentelemetry-go/blob/e4ab3141123d0811125a69823dbbe4d9ec5a9b8f/sdk/internal/x/x.go ↩