-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Added the internal/observ package to stdoutlog
#7735
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?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7735 +/- ##
======================================
Coverage 86.2% 86.3%
======================================
Files 302 306 +4
Lines 21991 22139 +148
======================================
+ Hits 18968 19113 +145
- Misses 2642 2644 +2
- Partials 381 382 +1
🚀 New features to boost your workflow:
|
internal/observ package to stdout loginternal/observ package to stdoutlog
671f7c2 to
ebffa09
Compare
flc1125
left a comment
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.
Overall it looks good.
| - ./internal/version.go | ||
| go.opentelemetry.io/otel/exporters/stdout/stdoutlog: | ||
| version-refs: | ||
| - ./internal/version.go |
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.
| - ./internal/version.go | |
| - ./internal/version.go | |
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.
Pull request overview
This PR adds the internal/observ package to the stdoutlog exporter as part of issue #7020, introducing experimental observability metrics for the exporter. The implementation includes a feature flag system to enable/disable the observability functionality via environment variables.
Key Changes
- Introduces
internal/observpackage with instrumentation for tracking log export metrics (inflight, exported, and operation duration) - Adds
internal/xpackage for experimental feature flag management, specifically theObservabilityfeature flag - Implements partial success error handling with proper error formatting
- Updates module dependencies to include metric SDK support
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| versions.yaml | Adds stdoutlog exporter to version tracking |
| exporters/stdout/stdoutlog/internal/x/x.go | Implements feature flag framework for experimental features |
| exporters/stdout/stdoutlog/internal/x/x_test.go | Tests for feature flag functionality |
| exporters/stdout/stdoutlog/internal/x/features.go | Defines Observability feature flag |
| exporters/stdout/stdoutlog/internal/x/features_test.go | Tests for Observability feature |
| exporters/stdout/stdoutlog/internal/x/README.md | Documents experimental observability feature usage |
| exporters/stdout/stdoutlog/internal/version.go | Defines version constant for the exporter |
| exporters/stdout/stdoutlog/internal/partialsuccess.go | Implements partial success error type |
| exporters/stdout/stdoutlog/internal/partialsuccess_test.go | Tests for partial success error formatting |
| exporters/stdout/stdoutlog/internal/observ/instrumentation.go | Core instrumentation logic for metrics collection |
| exporters/stdout/stdoutlog/internal/observ/instrumentation_test.go | Comprehensive tests including benchmarks for instrumentation |
| exporters/stdout/stdoutlog/internal/gen.go | Code generation directives for template-based code |
| exporters/stdout/stdoutlog/go.mod | Adds metric and sdk/metric dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| attrs := make([]attribute.KeyValue, 0, 2) | ||
| attrs = append(attrs, | ||
| semconv.OTelComponentName(GetComponentName(id)), | ||
| semconv.OTelComponentNameKey.String(ComponentType)) |
Copilot
AI
Jan 7, 2026
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.
The attribute key used here appears incorrect. The code uses semconv.OTelComponentNameKey.String(ComponentType) which creates an attribute with key "otel.component.name" and value ComponentType. However, based on the semantic conventions, there should be a "otel.component.type" attribute. This line should likely use semconv.OTelComponentType(ComponentType) instead of semconv.OTelComponentNameKey.String(ComponentType) to properly set the component type.
| semconv.OTelComponentNameKey.String(ComponentType)) | |
| semconv.OTelComponentType(ComponentType)) |
| func set(err error) attribute.Set { | ||
| attrs := []attribute.KeyValue{ | ||
| semconv.OTelComponentName(GetComponentName(ID)), | ||
| semconv.OTelComponentNameKey.String(ComponentType), |
Copilot
AI
Jan 7, 2026
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.
The attribute key used here appears incorrect. The code uses semconv.OTelComponentNameKey.String(ComponentType) which creates an attribute with key "otel.component.name" and value ComponentType. However, based on the semantic conventions, there should be a "otel.component.type" attribute. This line should likely use semconv.OTelComponentType(ComponentType) instead of semconv.OTelComponentNameKey.String(ComponentType) to properly set the component type.
| semconv.OTelComponentNameKey.String(ComponentType), | |
| semconv.OTelComponentType(ComponentType), |
| // source: internal/shared/x/x_text.go.tmpl | ||
|
|
||
| // Copyright The OpenTelemetry Authors |
Copilot
AI
Jan 7, 2026
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.
The source template filename appears to have a typo. It references "x_text.go.tmpl" but should likely be "x_test.go.tmpl" based on the file being generated (x_test.go).
| // source: internal/shared/x/x_text.go.tmpl | |
| // Copyright The OpenTelemetry Authors | |
| // source: internal/shared/x/x_test.go.tmpl | |
| Copyright The OpenTelemetry Authors |
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.
This might require another PR to fix it.
| e = fmt.Errorf("failed to create the inflight metric %w", e) | ||
| err = errors.Join(err, e) | ||
| } | ||
| inst.inflight = inflight.Inst() | ||
|
|
||
| exported, e := otelconv.NewSDKExporterLogExported(m) | ||
| if e != nil { | ||
| e = fmt.Errorf("failed to create the exported metric %w", e) | ||
| err = errors.Join(err, e) | ||
| } | ||
| inst.exported = exported.Inst() | ||
|
|
||
| duration, e := otelconv.NewSDKExporterOperationDuration(m) | ||
| if e != nil { | ||
| e = fmt.Errorf("failed to create the duration metric %w", e) |
Copilot
AI
Jan 7, 2026
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.
The error message is missing a colon separator between the message text and the wrapped error. This should be "failed to create the inflight metric: %w" to properly format the error chain.
| e = fmt.Errorf("failed to create the inflight metric %w", e) | |
| err = errors.Join(err, e) | |
| } | |
| inst.inflight = inflight.Inst() | |
| exported, e := otelconv.NewSDKExporterLogExported(m) | |
| if e != nil { | |
| e = fmt.Errorf("failed to create the exported metric %w", e) | |
| err = errors.Join(err, e) | |
| } | |
| inst.exported = exported.Inst() | |
| duration, e := otelconv.NewSDKExporterOperationDuration(m) | |
| if e != nil { | |
| e = fmt.Errorf("failed to create the duration metric %w", e) | |
| e = fmt.Errorf("failed to create the inflight metric: %w", e) | |
| err = errors.Join(err, e) | |
| } | |
| inst.inflight = inflight.Inst() | |
| exported, e := otelconv.NewSDKExporterLogExported(m) | |
| if e != nil { | |
| e = fmt.Errorf("failed to create the exported metric: %w", e) | |
| err = errors.Join(err, e) | |
| } | |
| inst.exported = exported.Inst() | |
| duration, e := otelconv.NewSDKExporterOperationDuration(m) | |
| if e != nil { | |
| e = fmt.Errorf("failed to create the duration metric: %w", e) |
|
|
||
| exported, e := otelconv.NewSDKExporterLogExported(m) | ||
| if e != nil { | ||
| e = fmt.Errorf("failed to create the exported metric %w", e) |
Copilot
AI
Jan 7, 2026
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.
The error message is missing a colon separator between the message text and the wrapped error. This should be "failed to create the exported metric: %w" to properly format the error chain.
| e = fmt.Errorf("failed to create the exported metric %w", e) | |
| e = fmt.Errorf("failed to create the exported metric: %w", e) |
|
|
||
| duration, e := otelconv.NewSDKExporterOperationDuration(m) | ||
| if e != nil { | ||
| e = fmt.Errorf("failed to create the duration metric %w", e) |
Copilot
AI
Jan 7, 2026
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.
The error message is missing a colon separator between the message text and the wrapped error. This should be "failed to create the duration metric: %w" to properly format the error chain.
| e = fmt.Errorf("failed to create the duration metric %w", e) | |
| e = fmt.Errorf("failed to create the duration metric: %w", e) |
| if errors.As(err, ps) { | ||
| return min(max(ps.RejectedItems, 0), n) | ||
| } | ||
| // all logs exporter |
Copilot
AI
Jan 7, 2026
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.
The comment contains a typo: "exporter" should be "exported". The comment should read "// all logs exported".
| // all logs exporter | |
| // all logs exported |
| }, | ||
| } | ||
|
|
||
| // rejectedCount returns how many out of the n logs exporter were rejected based on |
Copilot
AI
Jan 7, 2026
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.
The comment contains a typo: "exporter" should be "exported". The comment should read "// rejectedCount returns how many out of the n logs exported were rejected based on".
a part of #7020
goos: windows goarch: amd64 pkg: go.opentelemetry.io/otel/exporters/stdout/stdoutlog/internal/observ cpu: Intel(R) Core(TM) i7-14700 │ result.txt │ │ sec/op │ InstrumentationExportLogs/NoError-28 47.68n ± 5% InstrumentationExportLogs/PartialError-28 471.6n ± 2% InstrumentationExportLogs/FullError-28 471.9n ± 10% geomean 219.7n │ result.txt │ │ B/op │ InstrumentationExportLogs/NoError-28 0.000 ± 0% InstrumentationExportLogs/PartialError-28 305.0 ± 0% InstrumentationExportLogs/FullError-28 305.0 ± 0% geomean ¹ ¹ summaries must be >0 to compute geomean │ result.txt │ │ allocs/op │ InstrumentationExportLogs/NoError-28 0.000 ± 0% InstrumentationExportLogs/PartialError-28 4.000 ± 0% InstrumentationExportLogs/FullError-28 4.000 ± 0% geomean ¹ ¹ summaries must be >0 to compute geomean