-
Notifications
You must be signed in to change notification settings - Fork 1.3k
sdk/log: self observability: batch log processor metrics #7124
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?
Changes from all commits
93e2fec
9de991b
8d1edbf
ec0b55e
385fd9f
5c405d7
1366caa
b2dd092
4f24dff
c4bf81a
4cbaff1
9d42e5c
3a35f8e
96de62a
001ddb6
9371ede
f3a214a
a931308
df7b8f0
7c2fd2b
7238dd4
3f2c046
a3d80da
bff6fee
5d57d29
a12f99f
d7fcefa
055f5ec
eebda4b
84e81c4
68025cc
c82c8db
81872b0
52a7300
170e336
b41cc36
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,10 @@ import ( | |
| "sync/atomic" | ||
| "time" | ||
|
|
||
| "go.opentelemetry.io/otel" | ||
| "go.opentelemetry.io/otel/internal/global" | ||
| "go.opentelemetry.io/otel/sdk/log/internal/counter" | ||
| "go.opentelemetry.io/otel/sdk/log/internal/observ" | ||
| ) | ||
|
|
||
| const ( | ||
|
|
@@ -98,6 +101,9 @@ type BatchProcessor struct { | |
| // stopped holds the stopped state of the BatchProcessor. | ||
| stopped atomic.Bool | ||
|
|
||
| // inst is the instrumentation for observability (nil when disabled). | ||
| inst *observ.BLP | ||
|
|
||
| noCmp [0]func() //nolint: unused // This is indeed used. | ||
| } | ||
|
|
||
|
|
@@ -111,6 +117,31 @@ func NewBatchProcessor(exporter Exporter, opts ...BatchProcessorOption) *BatchPr | |
| // Do not panic on nil export. | ||
| exporter = defaultNoopExporter | ||
| } | ||
|
|
||
| b := &BatchProcessor{ | ||
| q: newQueue(cfg.maxQSize.Value), | ||
| batchSize: cfg.expMaxBatchSize.Value, | ||
| pollTrigger: make(chan struct{}, 1), | ||
| pollKill: make(chan struct{}), | ||
| } | ||
|
|
||
| var err error | ||
| b.inst, err = observ.NewBLP( | ||
| counter.NextExporterID(), | ||
| func() int64 { return int64(b.q.Len()) }, | ||
| int64(cfg.maxQSize.Value), | ||
| ) | ||
| if err != nil { | ||
| otel.Handle(err) | ||
| } | ||
|
|
||
| // Wrap exporter with metrics recording if observability is enabled. | ||
| // This must be the innermost wrapper (closest to user exporter) to record | ||
| // metrics just before calling the actual exporter. | ||
| if b.inst != nil { | ||
| exporter = newMetricsExporter(exporter, b.inst) | ||
| } | ||
|
Comment on lines
+128
to
+143
|
||
|
|
||
| // Order is important here. Wrap the timeoutExporter with the chunkExporter | ||
| // to ensure each export completes in timeout (instead of all chunked | ||
| // exports). | ||
|
|
@@ -119,15 +150,9 @@ func NewBatchProcessor(exporter Exporter, opts ...BatchProcessorOption) *BatchPr | |
| // appropriately on export. | ||
| exporter = newChunkExporter(exporter, cfg.expMaxBatchSize.Value) | ||
|
|
||
| b := &BatchProcessor{ | ||
| exporter: newBufferExporter(exporter, cfg.expBufferSize.Value), | ||
|
|
||
| q: newQueue(cfg.maxQSize.Value), | ||
| batchSize: cfg.expMaxBatchSize.Value, | ||
| pollTrigger: make(chan struct{}, 1), | ||
| pollKill: make(chan struct{}), | ||
| } | ||
| b.exporter = newBufferExporter(exporter, cfg.expBufferSize.Value) | ||
| b.pollDone = b.poll(cfg.expInterval.Value) | ||
|
|
||
| return b | ||
| } | ||
|
|
||
|
|
@@ -143,6 +168,8 @@ func (b *BatchProcessor) poll(interval time.Duration) (done chan struct{}) { | |
| defer close(done) | ||
| defer ticker.Stop() | ||
|
|
||
| ctx := context.Background() | ||
|
|
||
| for { | ||
| select { | ||
| case <-ticker.C: | ||
|
|
@@ -153,6 +180,9 @@ func (b *BatchProcessor) poll(interval time.Duration) (done chan struct{}) { | |
| } | ||
|
|
||
| if d := b.q.Dropped(); d > 0 { | ||
| if b.inst != nil { | ||
| b.inst.ProcessedQueueFull(ctx, int64(d)) //nolint: gosec | ||
| } | ||
|
pellared marked this conversation as resolved.
|
||
| global.Warn("dropped log records", "dropped", d) | ||
| } | ||
|
|
||
|
|
@@ -225,6 +255,9 @@ func (b *BatchProcessor) Shutdown(ctx context.Context) error { | |
|
|
||
| // Flush remaining queued before exporter shutdown. | ||
| err := b.exporter.Export(ctx, b.q.Flush()) | ||
| if b.inst != nil { | ||
| err = errors.Join(err, b.inst.Shutdown()) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When records have overflowed the ring buffer since the last poll iteration, I think that addressing this would be very hard and I would rather create an issue for tracking this.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| } | ||
| return errors.Join(err, b.exporter.Shutdown(ctx)) | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,8 +20,18 @@ import ( | |||||||||
| "github.com/stretchr/testify/require" | ||||||||||
|
|
||||||||||
| "go.opentelemetry.io/otel" | ||||||||||
| "go.opentelemetry.io/otel/attribute" | ||||||||||
| "go.opentelemetry.io/otel/internal/global" | ||||||||||
| "go.opentelemetry.io/otel/log" | ||||||||||
| "go.opentelemetry.io/otel/sdk" | ||||||||||
| "go.opentelemetry.io/otel/sdk/instrumentation" | ||||||||||
| "go.opentelemetry.io/otel/sdk/log/internal/counter" | ||||||||||
| "go.opentelemetry.io/otel/sdk/log/internal/observ" | ||||||||||
| sdkmetric "go.opentelemetry.io/otel/sdk/metric" | ||||||||||
| "go.opentelemetry.io/otel/sdk/metric/metricdata" | ||||||||||
| "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" | ||||||||||
| semconv "go.opentelemetry.io/otel/semconv/v1.40.0" | ||||||||||
| "go.opentelemetry.io/otel/semconv/v1.40.0/otelconv" | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| type concurrentBuffer struct { | ||||||||||
|
|
@@ -673,3 +683,199 @@ func BenchmarkBatchProcessorOnEmit(b *testing.B) { | |||||||||
| _ = err | ||||||||||
| }) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const blpComponentID int64 = 0 | ||||||||||
|
|
||||||||||
| func TestBatchProcessorMetricsDisabled(t *testing.T) { | ||||||||||
| t.Setenv("OTEL_GO_X_OBSERVABILITY", "false") | ||||||||||
|
|
||||||||||
|
||||||||||
| origExporterID := counter.ExporterID() | |
| t.Cleanup(func() { counter.SetExporterID(origExporterID) }) |
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.
similar pattern as other package tests, setting to 0
Copilot
AI
Apr 16, 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.
This test mutates the global exporter ID counter via counter.SetExporterID(...) but does not restore the previous value, which can leak global state into other tests in the package depending on execution order. Consider saving the previous counter value and restoring it in t.Cleanup(...).
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.
same as above
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
counter.NextExporterID()is evaluated unconditionally beforeobserv.NewBLP(...)can check whether observability is enabled, so the global ID counter is incremented (atomic op) even whenOTEL_GO_X_OBSERVABILITYis disabled. This adds avoidable overhead in the default (disabled) path and makes component IDs depend on how many batch processors were created while disabled. Consider gating the ID generation andNewBLPcall behind the feature-flag check (or moving ID generation insideNewBLPafter it confirms observability is enabled).