From 91010b0dc02c3fc34440c0f2831c8fa006d70e80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 13 Sep 2024 07:10:10 +0200 Subject: [PATCH 01/11] Revert "Move `log.Processor.Enabled` to independent `FilterProcessor` interfaced type (#5692)" This reverts commit 002c0a4c0352a56ebebc13f3ec20f73c23b348f6. --- CHANGELOG.md | 2 -- sdk/log/batch.go | 5 ++++ sdk/log/batch_test.go | 9 +++++++ sdk/log/doc.go | 3 --- sdk/log/example_test.go | 25 ++++++------------- sdk/log/internal/x/README.md | 35 --------------------------- sdk/log/internal/x/x.go | 47 ------------------------------------ sdk/log/logger.go | 32 ++++++------------------ sdk/log/logger_test.go | 26 ++------------------ sdk/log/processor.go | 24 +++++++++++++++--- sdk/log/provider.go | 15 ------------ sdk/log/provider_test.go | 27 +++++---------------- sdk/log/simple.go | 7 +++++- sdk/log/simple_test.go | 8 ++++++ 14 files changed, 72 insertions(+), 193 deletions(-) delete mode 100644 sdk/log/internal/x/README.md delete mode 100644 sdk/log/internal/x/x.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 8261f72ca3c..5f320cfd198 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,8 +65,6 @@ The next release will require at least [Go 1.22]. - `SimpleProcessor.Enabled` in `go.opentelemetry.io/otel/sdk/log` now returns `false` if the exporter is `nil`. (#5665) - Update the concurrency requirements of `Exporter` in `go.opentelemetry.io/otel/sdk/log`. (#5666) - `SimpleProcessor` in `go.opentelemetry.io/otel/sdk/log` synchronizes `OnEmit` calls. (#5666) -- The `Processor` interface in `go.opentelemetry.io/otel/sdk/log` no longer includes the `Enabled` method. - See the `FilterProcessor` interface type added in `go.opentelemetry.io/otel/sdk/log/internal/x` to continue providing this functionality. (#5692) - The `SimpleProcessor` type in `go.opentelemetry.io/otel/sdk/log` is no longer comparable. (#5693) - The `BatchProcessor` type in `go.opentelemetry.io/otel/sdk/log` is no longer comparable. (#5693) diff --git a/sdk/log/batch.go b/sdk/log/batch.go index 197fcbad43d..3faf4d1d4ea 100644 --- a/sdk/log/batch.go +++ b/sdk/log/batch.go @@ -196,6 +196,11 @@ func (b *BatchProcessor) OnEmit(_ context.Context, r *Record) error { return nil } +// Enabled returns if b is enabled. +func (b *BatchProcessor) Enabled(context.Context, Record) bool { + return !b.stopped.Load() && b.q != nil +} + // Shutdown flushes queued log records and shuts down the decorated exporter. func (b *BatchProcessor) Shutdown(ctx context.Context) error { if b.stopped.Swap(true) || b.q == nil { diff --git a/sdk/log/batch_test.go b/sdk/log/batch_test.go index a4c1b5f094f..0b9ece02a3d 100644 --- a/sdk/log/batch_test.go +++ b/sdk/log/batch_test.go @@ -47,6 +47,7 @@ func TestEmptyBatchConfig(t *testing.T) { ctx := context.Background() record := new(Record) assert.NoError(t, bp.OnEmit(ctx, record), "OnEmit") + assert.False(t, bp.Enabled(ctx, *record), "Enabled") assert.NoError(t, bp.ForceFlush(ctx), "ForceFlush") assert.NoError(t, bp.Shutdown(ctx), "Shutdown") }) @@ -269,6 +270,14 @@ func TestBatchProcessor(t *testing.T) { assert.Equal(t, 3, e.ExportN()) }) + t.Run("Enabled", func(t *testing.T) { + b := NewBatchProcessor(defaultNoopExporter) + assert.True(t, b.Enabled(ctx, Record{})) + + _ = b.Shutdown(ctx) + assert.False(t, b.Enabled(ctx, Record{})) + }) + t.Run("Shutdown", func(t *testing.T) { t.Run("Error", func(t *testing.T) { e := newTestExporter(assert.AnError) diff --git a/sdk/log/doc.go b/sdk/log/doc.go index 14a581db6b6..6a1f1b0e915 100644 --- a/sdk/log/doc.go +++ b/sdk/log/doc.go @@ -32,8 +32,5 @@ at a single endpoint their origin is decipherable. See [go.opentelemetry.io/otel/log] for more information about the OpenTelemetry Logs Bridge API. - -See [go.opentelemetry.io/otel/sdk/log/internal/x] for information about the -experimental features. */ package log // import "go.opentelemetry.io/otel/sdk/log" diff --git a/sdk/log/example_test.go b/sdk/log/example_test.go index 8070beef771..68449a9b315 100644 --- a/sdk/log/example_test.go +++ b/sdk/log/example_test.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "strings" - "sync" logapi "go.opentelemetry.io/otel/log" "go.opentelemetry.io/otel/log/global" @@ -59,7 +58,7 @@ func ExampleProcessor_filtering() { // Wrap the processor so that it ignores processing log records // when a context deriving from WithIgnoreLogs is passed // to the logging methods. - processor = &ContextFilterProcessor{Processor: processor} + processor = &ContextFilterProcessor{processor} // The created processor can then be registered with // the OpenTelemetry Logs SDK using the WithProcessor option. @@ -82,15 +81,6 @@ func WithIgnoreLogs(ctx context.Context) context.Context { // [WithIgnoreLogs] is passed to its methods. type ContextFilterProcessor struct { log.Processor - - lazyFilter sync.Once - // Use the experimental FilterProcessor interface - // (go.opentelemetry.io/otel/sdk/log/internal/x). - filter filter -} - -type filter interface { - Enabled(ctx context.Context, param logapi.EnabledParameters) bool } func (p *ContextFilterProcessor) OnEmit(ctx context.Context, record *log.Record) error { @@ -100,13 +90,8 @@ func (p *ContextFilterProcessor) OnEmit(ctx context.Context, record *log.Record) return p.Processor.OnEmit(ctx, record) } -func (p *ContextFilterProcessor) Enabled(ctx context.Context, param logapi.EnabledParameters) bool { - p.lazyFilter.Do(func() { - if f, ok := p.Processor.(filter); ok { - p.filter = f - } - }) - return !ignoreLogs(ctx) && (p.filter == nil || p.filter.Enabled(ctx, param)) +func (p *ContextFilterProcessor) Enabled(ctx context.Context, record log.Record) bool { + return !ignoreLogs(ctx) && p.Processor.Enabled(ctx, record) } func ignoreLogs(ctx context.Context) bool { @@ -135,6 +120,10 @@ func ExampleProcessor_redact() { // from attributes containing "token" in the key. type RedactTokensProcessor struct{} +func (p *RedactTokensProcessor) Enabled(ctx context.Context, record log.Record) bool { + return true +} + // OnEmit redacts values from attributes containing "token" in the key // by replacing them with a REDACTED value. func (p *RedactTokensProcessor) OnEmit(ctx context.Context, record *log.Record) error { diff --git a/sdk/log/internal/x/README.md b/sdk/log/internal/x/README.md deleted file mode 100644 index 73f4db626af..00000000000 --- a/sdk/log/internal/x/README.md +++ /dev/null @@ -1,35 +0,0 @@ -# Experimental Features - -The Logs SDK contains features that have not yet stabilized. -These features are added to the OpenTelemetry Go Logs SDK prior to stabilization so that users can start experimenting with them and provide feedback. - -These feature may change in backwards incompatible ways as feedback is applied. -See the [Compatibility and Stability](#compatibility-and-stability) section for more information. - -## Features - -- [Filter Processors](#filter-processor) - -### Filter Processor - -Users of logging libraries often want to know if a log `Record` will be processed or dropped before they perform complex operations to construct the `Record`. -The [`Logger`] in the Logs Bridge API provides the `Enabled` method for just this use-case. -In order for the Logs Bridge SDK to effectively implement this API, it needs to be known if the registered [`Processor`]s are enabled for the `Record` within a context. -A [`Processor`] that knows, and can identify, what `Record` it will process or drop when it is passed to `OnEmit` can communicate this to the SDK `Logger` by implementing the `FilterProcessor`. - -By default, the SDK `Logger.Enabled` will return true when called. -Only if all the registered [`Processor`]s implement `FilterProcessor` and they all return `false` will `Logger.Enabled` return `false`. - -See the [`minsev`] [`Processor`] for an example use-case. -It is used to filter `Record`s out that a have a `Severity` below a threshold. - -[`Logger`]: https://pkg.go.dev/go.opentelemetry.io/otel/log#Logger -[`Processor`]: https://pkg.go.dev/go.opentelemetry.io/otel/sdk/log#Processor -[`minsev`]: https://pkg.go.dev/go.opentelemetry.io/contrib/processors/minsev - -## Compatibility and Stability - -Experimental features do not fall within the scope of the OpenTelemetry Go versioning and stability [policy](../../../../VERSIONING.md). -These features may be removed or modified in successive version releases, including patch versions. - -When an experimental feature is promoted to a stable feature, a migration path will be included in the changelog entry of the release. diff --git a/sdk/log/internal/x/x.go b/sdk/log/internal/x/x.go deleted file mode 100644 index ca78d109778..00000000000 --- a/sdk/log/internal/x/x.go +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -// Package x contains support for Logs SDK experimental features. -package x // import "go.opentelemetry.io/otel/sdk/log/internal/x" - -import ( - "context" - - "go.opentelemetry.io/otel/log" -) - -// FilterProcessor is a [go.opentelemetry.io/otel/sdk/log.Processor] that knows, -// and can identify, what [log.Record] it will process or drop when it is -// passed to OnEmit. -// -// This is useful for users of logging libraries that want to know if a [log.Record] -// will be processed or dropped before they perform complex operations to -// construct the [log.Record]. -// -// Processor implementations that choose to support this by satisfying this -// interface are expected to re-evaluate the [log.Record]s passed to OnEmit, it is -// not expected that the caller to OnEmit will use the functionality from this -// interface prior to calling OnEmit. -// -// This should only be implemented for Processors that can make reliable -// enough determination of this prior to processing a [log.Record] and where -// the result is dynamic. -// -// [Processor]: https://pkg.go.dev/go.opentelemetry.io/otel/sdk/log#Processor -type FilterProcessor interface { - // Enabled returns whether the Processor will process for the given context - // and param. - // - // The passed param is likely to be a partial record with only the - // bridge-relevant information being provided (e.g a record with only the - // Severity set). If a Logger needs more information than is provided, it - // is said to be in an indeterminate state (see below). - // - // The returned value will be true when the Processor will process for the - // provided context and param, and will be false if the Processor will not - // process. An implementation should default to returning true for an - // indeterminate state. - // - // Implementations should not modify the param. - Enabled(ctx context.Context, param log.EnabledParameters) bool -} diff --git a/sdk/log/logger.go b/sdk/log/logger.go index d6ca2ea41aa..030102aacb5 100644 --- a/sdk/log/logger.go +++ b/sdk/log/logger.go @@ -11,7 +11,6 @@ import ( "go.opentelemetry.io/otel/log" "go.opentelemetry.io/otel/log/embedded" "go.opentelemetry.io/otel/sdk/instrumentation" - "go.opentelemetry.io/otel/sdk/log/internal/x" "go.opentelemetry.io/otel/trace" ) @@ -43,29 +42,14 @@ func (l *logger) Emit(ctx context.Context, r log.Record) { } } -// Enabled returns true if at least one Processor held by the LoggerProvider -// that created the logger will process param for the provided context and param. -// -// If it is not possible to definitively determine the param will be -// processed, true will be returned by default. A value of false will only be -// returned if it can be positively verified that no Processor will process. -func (l *logger) Enabled(ctx context.Context, param log.EnabledParameters) bool { - fltrs := l.provider.filterProcessors() - // If there are more Processors than FilterProcessors we cannot be sure - // that all Processors will drop the record. Therefore, return true. - // - // If all Processors are FilterProcessors, check if any is enabled. - return len(l.provider.processors) > len(fltrs) || anyEnabled(ctx, param, fltrs) -} - -func anyEnabled(ctx context.Context, param log.EnabledParameters, fltrs []x.FilterProcessor) bool { - for _, f := range fltrs { - if f.Enabled(ctx, param) { - // At least one Processor will process the Record. - return true - } - } - // No Processor will process the record +func (l *logger) Enabled(ctx context.Context, r log.EnabledParameters) bool { + // TODO + // newRecord := l.newRecord(ctx, r) + // for _, p := range l.provider.processors { + // if enabled := p.Enabled(ctx, newRecord); enabled { + // return true + // } + // } return false } diff --git a/sdk/log/logger_test.go b/sdk/log/logger_test.go index 0c2f793db97..890eb32a9a3 100644 --- a/sdk/log/logger_test.go +++ b/sdk/log/logger_test.go @@ -215,9 +215,8 @@ func TestLoggerEmit(t *testing.T) { } func TestLoggerEnabled(t *testing.T) { - p0 := newFltrProcessor("0", true) - p1 := newFltrProcessor("1", true) - p2WithDisabled := newFltrProcessor("2", false) + p0, p1, p2WithDisabled := newProcessor("0"), newProcessor("1"), newProcessor("2") + p2WithDisabled.enabled = false testCases := []struct { name string @@ -274,24 +273,3 @@ func TestLoggerEnabled(t *testing.T) { }) } } - -func BenchmarkLoggerEnabled(b *testing.B) { - provider := NewLoggerProvider( - WithProcessor(newFltrProcessor("0", false)), - WithProcessor(newFltrProcessor("1", true)), - ) - logger := provider.Logger("BenchmarkLoggerEnabled") - ctx, param := context.Background(), log.EnabledParameters{} - param.SetSeverity(log.SeverityDebug) - - var enabled bool - - b.ReportAllocs() - b.ResetTimer() - - for n := 0; n < b.N; n++ { - enabled = logger.Enabled(ctx, param) - } - - _ = enabled -} diff --git a/sdk/log/processor.go b/sdk/log/processor.go index fcab34c7a48..937f69f2597 100644 --- a/sdk/log/processor.go +++ b/sdk/log/processor.go @@ -12,9 +12,6 @@ import ( // Any of the Processor's methods may be called concurrently with itself // or with other methods. It is the responsibility of the Processor to manage // this concurrency. -// -// See [go.opentelemetry.io/otel/sdk/log/internal/x] for information about how -// a Processor can be extended to support experimental features. type Processor interface { // OnEmit is called when a Record is emitted. // @@ -38,6 +35,27 @@ type Processor interface { // to create a copy that shares no state with the original. OnEmit(ctx context.Context, record *Record) error + // Enabled returns whether the Processor will process for the given context + // and record. + // + // The passed record is likely to be a partial record with only the + // bridge-relevant information being provided (e.g a record with only the + // Severity set). If a Logger needs more information than is provided, it + // is said to be in an indeterminate state (see below). + // + // The returned value will be true when the Processor will process for the + // provided context and record, and will be false if the Processor will not + // process. The returned value may be true or false in an indeterminate + // state. An implementation should default to returning true for an + // indeterminate state, but may return false if valid reasons in particular + // circumstances exist (e.g. performance, correctness). + // + // The SDK invokes the processors sequentially in the same order as + // they were registered using [WithProcessor] until any processor returns true. + // + // Implementations should not modify the record. + Enabled(ctx context.Context, record Record) bool + // Shutdown is called when the SDK shuts down. Any cleanup or release of // resources held by the exporter should be done in this call. // diff --git a/sdk/log/provider.go b/sdk/log/provider.go index 14084ed99a8..726406d014e 100644 --- a/sdk/log/provider.go +++ b/sdk/log/provider.go @@ -15,7 +15,6 @@ import ( "go.opentelemetry.io/otel/log/embedded" "go.opentelemetry.io/otel/log/noop" "go.opentelemetry.io/otel/sdk/instrumentation" - "go.opentelemetry.io/otel/sdk/log/internal/x" "go.opentelemetry.io/otel/sdk/resource" ) @@ -67,9 +66,6 @@ type LoggerProvider struct { attributeCountLimit int attributeValueLengthLimit int - fltrProcessorsOnce sync.Once - fltrProcessors []x.FilterProcessor - loggersMu sync.Mutex loggers map[instrumentation.Scope]*logger @@ -97,17 +93,6 @@ func NewLoggerProvider(opts ...LoggerProviderOption) *LoggerProvider { } } -func (p *LoggerProvider) filterProcessors() []x.FilterProcessor { - p.fltrProcessorsOnce.Do(func() { - for _, proc := range p.processors { - if f, ok := proc.(x.FilterProcessor); ok { - p.fltrProcessors = append(p.fltrProcessors, f) - } - } - }) - return p.fltrProcessors -} - // Logger returns a new [log.Logger] with the provided name and configuration. // // If p is shut down, a [noop.Logger] instance is returned. diff --git a/sdk/log/provider_test.go b/sdk/log/provider_test.go index b9374e9d934..1dca7128f41 100644 --- a/sdk/log/provider_test.go +++ b/sdk/log/provider_test.go @@ -22,7 +22,6 @@ import ( "go.opentelemetry.io/otel/log" "go.opentelemetry.io/otel/log/noop" ottest "go.opentelemetry.io/otel/sdk/internal/internaltest" - "go.opentelemetry.io/otel/sdk/log/internal/x" "go.opentelemetry.io/otel/sdk/resource" ) @@ -36,10 +35,11 @@ type processor struct { forceFlushCalls int records []Record + enabled bool } func newProcessor(name string) *processor { - return &processor{Name: name} + return &processor{Name: name, enabled: true} } func (p *processor) OnEmit(ctx context.Context, r *Record) error { @@ -51,6 +51,10 @@ func (p *processor) OnEmit(ctx context.Context, r *Record) error { return nil } +func (p *processor) Enabled(context.Context, Record) bool { + return p.enabled +} + func (p *processor) Shutdown(context.Context) error { p.shutdownCalls++ return p.Err @@ -61,25 +65,6 @@ func (p *processor) ForceFlush(context.Context) error { return p.Err } -type fltrProcessor struct { - *processor - - enabled bool -} - -var _ x.FilterProcessor = (*fltrProcessor)(nil) - -func newFltrProcessor(name string, enabled bool) *fltrProcessor { - return &fltrProcessor{ - processor: newProcessor(name), - enabled: enabled, - } -} - -func (p *fltrProcessor) Enabled(context.Context, log.EnabledParameters) bool { - return p.enabled -} - func TestNewLoggerProviderConfiguration(t *testing.T) { t.Cleanup(func(orig otel.ErrorHandler) func() { otel.SetErrorHandler(otel.ErrorHandlerFunc(func(err error) { diff --git a/sdk/log/simple.go b/sdk/log/simple.go index 002e52cae66..b426e414fff 100644 --- a/sdk/log/simple.go +++ b/sdk/log/simple.go @@ -58,7 +58,12 @@ func (s *SimpleProcessor) OnEmit(ctx context.Context, r *Record) error { return s.exporter.Export(ctx, *records) } -// Shutdown shuts down the exporter. +// Enabled returns true if the exporter is not nil. +func (s *SimpleProcessor) Enabled(context.Context, Record) bool { + return s.exporter != nil +} + +// Shutdown shuts down the expoter. func (s *SimpleProcessor) Shutdown(ctx context.Context) error { if s.exporter == nil { return nil diff --git a/sdk/log/simple_test.go b/sdk/log/simple_test.go index f8719bfe551..cc71360f8fc 100644 --- a/sdk/log/simple_test.go +++ b/sdk/log/simple_test.go @@ -52,6 +52,12 @@ func TestSimpleProcessorOnEmit(t *testing.T) { assert.Equal(t, []log.Record{*r}, e.records) } +func TestSimpleProcessorEnabled(t *testing.T) { + e := new(exporter) + s := log.NewSimpleProcessor(e) + assert.True(t, s.Enabled(context.Background(), log.Record{})) +} + func TestSimpleProcessorShutdown(t *testing.T) { e := new(exporter) s := log.NewSimpleProcessor(e) @@ -91,6 +97,7 @@ func TestSimpleProcessorEmpty(t *testing.T) { ctx := context.Background() record := new(log.Record) assert.NoError(t, s.OnEmit(ctx, record), "OnEmit") + assert.False(t, s.Enabled(ctx, *record), "Enabled") assert.NoError(t, s.ForceFlush(ctx), "ForceFlush") assert.NoError(t, s.Shutdown(ctx), "Shutdown") }) @@ -112,6 +119,7 @@ func TestSimpleProcessorConcurrentSafe(t *testing.T) { defer wg.Done() _ = s.OnEmit(ctx, r) + _ = s.Enabled(ctx, *r) _ = s.Shutdown(ctx) _ = s.ForceFlush(ctx) }() From 012b907d78a1e8a7d86e8e4f7e2805ffa27e079c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 13 Sep 2024 07:19:56 +0200 Subject: [PATCH 02/11] sdk/log: Spike for EnabledParamerters --- sdk/log/batch.go | 2 +- sdk/log/batch_test.go | 6 +-- sdk/log/bench_test.go | 6 +-- sdk/log/example_test.go | 6 +-- sdk/log/logger.go | 92 +++++++++++++++++++++++++++++++++++++--- sdk/log/processor.go | 2 +- sdk/log/provider_test.go | 2 +- sdk/log/simple.go | 2 +- sdk/log/simple_test.go | 7 +-- 9 files changed, 102 insertions(+), 23 deletions(-) diff --git a/sdk/log/batch.go b/sdk/log/batch.go index 3faf4d1d4ea..d0bdabc80e8 100644 --- a/sdk/log/batch.go +++ b/sdk/log/batch.go @@ -197,7 +197,7 @@ func (b *BatchProcessor) OnEmit(_ context.Context, r *Record) error { } // Enabled returns if b is enabled. -func (b *BatchProcessor) Enabled(context.Context, Record) bool { +func (b *BatchProcessor) Enabled(context.Context, EnabledParameters) bool { return !b.stopped.Load() && b.q != nil } diff --git a/sdk/log/batch_test.go b/sdk/log/batch_test.go index 0b9ece02a3d..0d06407ff69 100644 --- a/sdk/log/batch_test.go +++ b/sdk/log/batch_test.go @@ -47,7 +47,7 @@ func TestEmptyBatchConfig(t *testing.T) { ctx := context.Background() record := new(Record) assert.NoError(t, bp.OnEmit(ctx, record), "OnEmit") - assert.False(t, bp.Enabled(ctx, *record), "Enabled") + assert.False(t, bp.Enabled(ctx, EnabledParameters{}), "Enabled") assert.NoError(t, bp.ForceFlush(ctx), "ForceFlush") assert.NoError(t, bp.Shutdown(ctx), "Shutdown") }) @@ -272,10 +272,10 @@ func TestBatchProcessor(t *testing.T) { t.Run("Enabled", func(t *testing.T) { b := NewBatchProcessor(defaultNoopExporter) - assert.True(t, b.Enabled(ctx, Record{})) + assert.True(t, b.Enabled(ctx, EnabledParameters{})) _ = b.Shutdown(ctx) - assert.False(t, b.Enabled(ctx, Record{})) + assert.False(t, b.Enabled(ctx, EnabledParameters{})) }) t.Run("Shutdown", func(t *testing.T) { diff --git a/sdk/log/bench_test.go b/sdk/log/bench_test.go index 835f68c7aba..fe3094ff994 100644 --- a/sdk/log/bench_test.go +++ b/sdk/log/bench_test.go @@ -116,7 +116,7 @@ func (p timestampProcessor) OnEmit(ctx context.Context, r *Record) error { return nil } -func (p timestampProcessor) Enabled(context.Context, Record) bool { +func (p timestampProcessor) Enabled(context.Context, EnabledParameters) bool { return true } @@ -135,7 +135,7 @@ func (p attrAddProcessor) OnEmit(ctx context.Context, r *Record) error { return nil } -func (p attrAddProcessor) Enabled(context.Context, Record) bool { +func (p attrAddProcessor) Enabled(context.Context, EnabledParameters) bool { return true } @@ -154,7 +154,7 @@ func (p attrSetDecorator) OnEmit(ctx context.Context, r *Record) error { return nil } -func (p attrSetDecorator) Enabled(context.Context, Record) bool { +func (p attrSetDecorator) Enabled(context.Context, EnabledParameters) bool { return true } diff --git a/sdk/log/example_test.go b/sdk/log/example_test.go index 68449a9b315..fbdd48b7e8e 100644 --- a/sdk/log/example_test.go +++ b/sdk/log/example_test.go @@ -90,8 +90,8 @@ func (p *ContextFilterProcessor) OnEmit(ctx context.Context, record *log.Record) return p.Processor.OnEmit(ctx, record) } -func (p *ContextFilterProcessor) Enabled(ctx context.Context, record log.Record) bool { - return !ignoreLogs(ctx) && p.Processor.Enabled(ctx, record) +func (p *ContextFilterProcessor) Enabled(ctx context.Context, param log.EnabledParameters) bool { + return !ignoreLogs(ctx) && p.Processor.Enabled(ctx, param) } func ignoreLogs(ctx context.Context) bool { @@ -120,7 +120,7 @@ func ExampleProcessor_redact() { // from attributes containing "token" in the key. type RedactTokensProcessor struct{} -func (p *RedactTokensProcessor) Enabled(ctx context.Context, record log.Record) bool { +func (p *RedactTokensProcessor) Enabled(context.Context, log.EnabledParameters) bool { return true } diff --git a/sdk/log/logger.go b/sdk/log/logger.go index 030102aacb5..47a67054dda 100644 --- a/sdk/log/logger.go +++ b/sdk/log/logger.go @@ -11,6 +11,7 @@ import ( "go.opentelemetry.io/otel/log" "go.opentelemetry.io/otel/log/embedded" "go.opentelemetry.io/otel/sdk/instrumentation" + "go.opentelemetry.io/otel/sdk/resource" "go.opentelemetry.io/otel/trace" ) @@ -43,13 +44,12 @@ func (l *logger) Emit(ctx context.Context, r log.Record) { } func (l *logger) Enabled(ctx context.Context, r log.EnabledParameters) bool { - // TODO - // newRecord := l.newRecord(ctx, r) - // for _, p := range l.provider.processors { - // if enabled := p.Enabled(ctx, newRecord); enabled { - // return true - // } - // } + newParam := l.newEnabledParameters(ctx, r) + for _, p := range l.provider.processors { + if enabled := p.Enabled(ctx, newParam); enabled { + return true + } + } return false } @@ -85,3 +85,81 @@ func (l *logger) newRecord(ctx context.Context, r log.Record) Record { return newRecord } + +func (l *logger) newEnabledParameters(ctx context.Context, param log.EnabledParameters) EnabledParameters { + sc := trace.SpanContextFromContext(ctx) + + newParam := EnabledParameters{ + traceID: sc.TraceID(), + spanID: sc.SpanID(), + traceFlags: sc.TraceFlags(), + + resource: l.provider.resource, + scope: &l.instrumentationScope, + } + + if v, ok := param.Severity(); ok { + newParam.setSeverity(v) + } + + return newParam +} + +// EnabledParameters represents payload for [Logger]'s Enabled method. +type EnabledParameters struct { + severity log.Severity + severitySet bool + + traceID trace.TraceID + spanID trace.SpanID + traceFlags trace.TraceFlags + + // resource represents the entity that collected the log. + resource *resource.Resource + + // scope is the Scope that the Logger was created with. + scope *instrumentation.Scope +} + +// Severity returns the [Severity] level value, or [SeverityUndefined] if no value was set. +// The ok result indicates whether the value was set. +func (r *EnabledParameters) Severity() (value log.Severity, ok bool) { + return r.severity, r.severitySet +} + +// setSeverity sets the [Severity] level. +func (r *EnabledParameters) setSeverity(level log.Severity) { + r.severity = level + r.severitySet = true +} + +// TraceID returns the trace ID or empty array. +func (r *EnabledParameters) TraceID() trace.TraceID { + return r.traceID +} + +// SpanID returns the span ID or empty array. +func (r *EnabledParameters) SpanID() trace.SpanID { + return r.spanID +} + +// TraceFlags returns the trace flags. +func (r *EnabledParameters) TraceFlags() trace.TraceFlags { + return r.traceFlags +} + +// Resource returns the entity that collected the log. +func (r *EnabledParameters) Resource() resource.Resource { + if r.resource == nil { + return *resource.Empty() + } + return *r.resource +} + +// InstrumentationScope returns the scope that the Logger was created with. +func (r *EnabledParameters) InstrumentationScope() instrumentation.Scope { + if r.scope == nil { + return instrumentation.Scope{} + } + return *r.scope +} diff --git a/sdk/log/processor.go b/sdk/log/processor.go index 937f69f2597..83afc1682f1 100644 --- a/sdk/log/processor.go +++ b/sdk/log/processor.go @@ -54,7 +54,7 @@ type Processor interface { // they were registered using [WithProcessor] until any processor returns true. // // Implementations should not modify the record. - Enabled(ctx context.Context, record Record) bool + Enabled(ctx context.Context, param EnabledParameters) bool // Shutdown is called when the SDK shuts down. Any cleanup or release of // resources held by the exporter should be done in this call. diff --git a/sdk/log/provider_test.go b/sdk/log/provider_test.go index 1dca7128f41..e24a413a87c 100644 --- a/sdk/log/provider_test.go +++ b/sdk/log/provider_test.go @@ -51,7 +51,7 @@ func (p *processor) OnEmit(ctx context.Context, r *Record) error { return nil } -func (p *processor) Enabled(context.Context, Record) bool { +func (p *processor) Enabled(context.Context, EnabledParameters) bool { return p.enabled } diff --git a/sdk/log/simple.go b/sdk/log/simple.go index b426e414fff..a907fbf21be 100644 --- a/sdk/log/simple.go +++ b/sdk/log/simple.go @@ -59,7 +59,7 @@ func (s *SimpleProcessor) OnEmit(ctx context.Context, r *Record) error { } // Enabled returns true if the exporter is not nil. -func (s *SimpleProcessor) Enabled(context.Context, Record) bool { +func (s *SimpleProcessor) Enabled(context.Context, EnabledParameters) bool { return s.exporter != nil } diff --git a/sdk/log/simple_test.go b/sdk/log/simple_test.go index cc71360f8fc..da01349ab60 100644 --- a/sdk/log/simple_test.go +++ b/sdk/log/simple_test.go @@ -55,7 +55,7 @@ func TestSimpleProcessorOnEmit(t *testing.T) { func TestSimpleProcessorEnabled(t *testing.T) { e := new(exporter) s := log.NewSimpleProcessor(e) - assert.True(t, s.Enabled(context.Background(), log.Record{})) + assert.True(t, s.Enabled(context.Background(), log.EnabledParameters{})) } func TestSimpleProcessorShutdown(t *testing.T) { @@ -97,7 +97,7 @@ func TestSimpleProcessorEmpty(t *testing.T) { ctx := context.Background() record := new(log.Record) assert.NoError(t, s.OnEmit(ctx, record), "OnEmit") - assert.False(t, s.Enabled(ctx, *record), "Enabled") + assert.False(t, s.Enabled(ctx, log.EnabledParameters{}), "Enabled") assert.NoError(t, s.ForceFlush(ctx), "ForceFlush") assert.NoError(t, s.Shutdown(ctx), "Shutdown") }) @@ -110,6 +110,7 @@ func TestSimpleProcessorConcurrentSafe(t *testing.T) { wg.Add(goRoutineN) r := new(log.Record) + param := log.EnabledParameters{} r.SetSeverityText("test") ctx := context.Background() e := &writerExporter{new(strings.Builder)} @@ -119,7 +120,7 @@ func TestSimpleProcessorConcurrentSafe(t *testing.T) { defer wg.Done() _ = s.OnEmit(ctx, r) - _ = s.Enabled(ctx, *r) + _ = s.Enabled(ctx, param) _ = s.Shutdown(ctx) _ = s.ForceFlush(ctx) }() From bb9ff834dbba824b0ac17b5841ba0d7543803367 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 13 Sep 2024 07:21:22 +0200 Subject: [PATCH 03/11] Revert changelog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f320cfd198..8261f72ca3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,8 @@ The next release will require at least [Go 1.22]. - `SimpleProcessor.Enabled` in `go.opentelemetry.io/otel/sdk/log` now returns `false` if the exporter is `nil`. (#5665) - Update the concurrency requirements of `Exporter` in `go.opentelemetry.io/otel/sdk/log`. (#5666) - `SimpleProcessor` in `go.opentelemetry.io/otel/sdk/log` synchronizes `OnEmit` calls. (#5666) +- The `Processor` interface in `go.opentelemetry.io/otel/sdk/log` no longer includes the `Enabled` method. + See the `FilterProcessor` interface type added in `go.opentelemetry.io/otel/sdk/log/internal/x` to continue providing this functionality. (#5692) - The `SimpleProcessor` type in `go.opentelemetry.io/otel/sdk/log` is no longer comparable. (#5693) - The `BatchProcessor` type in `go.opentelemetry.io/otel/sdk/log` is no longer comparable. (#5693) From 34b6f4ba141fb575423b1b5450d389115bd520ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 13 Sep 2024 07:27:24 +0200 Subject: [PATCH 04/11] Revert BenchmarkLoggerEnabled --- sdk/log/logger_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/sdk/log/logger_test.go b/sdk/log/logger_test.go index 890eb32a9a3..44ed94dfc50 100644 --- a/sdk/log/logger_test.go +++ b/sdk/log/logger_test.go @@ -273,3 +273,28 @@ func TestLoggerEnabled(t *testing.T) { }) } } + +func BenchmarkLoggerEnabled(b *testing.B) { + p0, p1, p2WithDisabled := newProcessor("0"), newProcessor("1"), newProcessor("2") + p2WithDisabled.enabled = false + + provider := NewLoggerProvider( + WithProcessor(p0), + WithProcessor(p1), + WithProcessor(p2WithDisabled), + ) + logger := provider.Logger("BenchmarkLoggerEnabled") + ctx, param := context.Background(), log.EnabledParameters{} + param.SetSeverity(log.SeverityDebug) + + var enabled bool + + b.ReportAllocs() + b.ResetTimer() + + for n := 0; n < b.N; n++ { + enabled = logger.Enabled(ctx, param) + } + + _ = enabled +} From 3ac329322dec68307cacb8c1d59555df57a1a50d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 13 Sep 2024 07:32:33 +0200 Subject: [PATCH 05/11] Update changelog --- CHANGELOG.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8261f72ca3c..5729f4db682 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,11 +8,18 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Added + +- Add `Enabled` method to `Processor` interface that accepts a newly introduced `EnabledParameters` type in `go.opentelemetry.io/otel/sdk/log`. (#5816) + ### Changed - Enable exemplars by default in `go.opentelemetry.io/otel/sdk/metric`. Exemplars can be disabled by setting `OTEL_METRICS_EXEMPLAR_FILTER=always_off` (#5778) - `Logger.Enabled` in `go.opentelemetry.io/otel/log` now accepts a newly introduced `EnabledParameters` type instead of `Record`. (#5791) -- `FilterProcessor.Enabled` in `go.opentelemetry.io/otel/sdk/log/internal/x` now accepts `EnabledParameters` instead of `Record`. (#5791) + +### Removed + +- Remove `go.opentelemetry.io/otel/sdk/log/internal/x` package. (#5816) From 7033738d58d2a77a83f1edaa9b474ba0dbe0a304 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 13 Sep 2024 07:36:47 +0200 Subject: [PATCH 06/11] Update doc comment --- sdk/log/processor.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/sdk/log/processor.go b/sdk/log/processor.go index 83afc1682f1..bfae25f51d8 100644 --- a/sdk/log/processor.go +++ b/sdk/log/processor.go @@ -36,24 +36,22 @@ type Processor interface { OnEmit(ctx context.Context, record *Record) error // Enabled returns whether the Processor will process for the given context - // and record. + // and param. // - // The passed record is likely to be a partial record with only the + // The passed param is likely to be a partial record with only the // bridge-relevant information being provided (e.g a record with only the // Severity set). If a Logger needs more information than is provided, it // is said to be in an indeterminate state (see below). // // The returned value will be true when the Processor will process for the - // provided context and record, and will be false if the Processor will not - // process. The returned value may be true or false in an indeterminate - // state. An implementation should default to returning true for an - // indeterminate state, but may return false if valid reasons in particular - // circumstances exist (e.g. performance, correctness). + // provided context and param, and will be false if the Processor will not + // process. An implementation should default to returning true for an + // indeterminate state. // // The SDK invokes the processors sequentially in the same order as // they were registered using [WithProcessor] until any processor returns true. // - // Implementations should not modify the record. + // Implementations should not modify the param. Enabled(ctx context.Context, param EnabledParameters) bool // Shutdown is called when the SDK shuts down. Any cleanup or release of From fbbb32d65935c4f6e251cb8357fd9ded2538cd11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 13 Sep 2024 08:00:01 +0200 Subject: [PATCH 07/11] Update changlog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5729f4db682..96d149ad6e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Removed -- Remove `go.opentelemetry.io/otel/sdk/log/internal/x` package. (#5816) +- Remove `go.opentelemetry.io/otel/sdk/log/internal/x` package. Filtering is now part of `Processor` interface in `go.opentelemetry.io/otel/sdk/log`. (#5816) From c2cea065f7d5e41c1182a2d45418af2ffcc5083d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 13 Sep 2024 08:03:15 +0200 Subject: [PATCH 08/11] Update EnabledParameters --- sdk/log/logger.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sdk/log/logger.go b/sdk/log/logger.go index 47a67054dda..10c5ac930fd 100644 --- a/sdk/log/logger.go +++ b/sdk/log/logger.go @@ -105,7 +105,7 @@ func (l *logger) newEnabledParameters(ctx context.Context, param log.EnabledPara return newParam } -// EnabledParameters represents payload for [Logger]'s Enabled method. +// EnabledParameters represent Enabled parameters. type EnabledParameters struct { severity log.Severity severitySet bool @@ -119,6 +119,8 @@ type EnabledParameters struct { // scope is the Scope that the Logger was created with. scope *instrumentation.Scope + + noCmp [0]func() //nolint: unused // This is indeed used. } // Severity returns the [Severity] level value, or [SeverityUndefined] if no value was set. From 1f4cecc063bb1779f236a210c43a5e504afac165 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 13 Sep 2024 08:20:46 +0200 Subject: [PATCH 09/11] Improve TestLoggerEnabled --- sdk/log/logger_test.go | 36 +++++++++++++++++++++++++++++++----- sdk/log/provider_test.go | 4 +++- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/sdk/log/logger_test.go b/sdk/log/logger_test.go index 44ed94dfc50..e31943f3c25 100644 --- a/sdk/log/logger_test.go +++ b/sdk/log/logger_test.go @@ -219,10 +219,11 @@ func TestLoggerEnabled(t *testing.T) { p2WithDisabled.enabled = false testCases := []struct { - name string - logger *logger - ctx context.Context - expected bool + name string + logger *logger + ctx context.Context + expected bool + expectedParams []EnabledParameters }{ { name: "NoProcessors", @@ -235,9 +236,16 @@ func TestLoggerEnabled(t *testing.T) { logger: newLogger(NewLoggerProvider( WithProcessor(p0), WithProcessor(p1), - ), instrumentation.Scope{}), + WithResource(resource.NewSchemaless(attribute.String("key", "value"))), + ), instrumentation.Scope{Name: "name"}), ctx: context.Background(), expected: true, + expectedParams: []EnabledParameters{ + { + scope: &instrumentation.Scope{Name: "name"}, + resource: resource.NewSchemaless(attribute.String("key", "value")), + }, + }, }, { name: "WithDisabledProcessors", @@ -252,24 +260,42 @@ func TestLoggerEnabled(t *testing.T) { logger: newLogger(NewLoggerProvider( WithProcessor(p2WithDisabled), WithProcessor(p0), + WithResource(resource.NewSchemaless(attribute.String("key", "value"))), ), instrumentation.Scope{}), ctx: context.Background(), expected: true, + expectedParams: []EnabledParameters{ + { + scope: &instrumentation.Scope{}, + resource: resource.NewSchemaless(attribute.String("key", "value")), + }, + }, }, { name: "WithNilContext", logger: newLogger(NewLoggerProvider( WithProcessor(p0), WithProcessor(p1), + WithResource(resource.NewSchemaless(attribute.String("key", "value"))), ), instrumentation.Scope{}), ctx: nil, expected: true, + expectedParams: []EnabledParameters{ + { + scope: &instrumentation.Scope{}, + resource: resource.NewSchemaless(attribute.String("key", "value")), + }, + }, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + // Clean up the EnabledParameters before the test. + p0.params = nil + assert.Equal(t, tc.expected, tc.logger.Enabled(tc.ctx, log.EnabledParameters{})) + assert.Equal(t, tc.expectedParams, p0.params) }) } } diff --git a/sdk/log/provider_test.go b/sdk/log/provider_test.go index e24a413a87c..7d160df581f 100644 --- a/sdk/log/provider_test.go +++ b/sdk/log/provider_test.go @@ -35,6 +35,7 @@ type processor struct { forceFlushCalls int records []Record + params []EnabledParameters enabled bool } @@ -51,7 +52,8 @@ func (p *processor) OnEmit(ctx context.Context, r *Record) error { return nil } -func (p *processor) Enabled(context.Context, EnabledParameters) bool { +func (p *processor) Enabled(_ context.Context, param EnabledParameters) bool { + p.params = append(p.params, param) return p.enabled } From 33d6ada01e4098553eec82b43b3703dfbabc22df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 13 Sep 2024 08:41:17 +0200 Subject: [PATCH 10/11] Improve changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96d149ad6e7..1f597f24e69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added -- Add `Enabled` method to `Processor` interface that accepts a newly introduced `EnabledParameters` type in `go.opentelemetry.io/otel/sdk/log`. (#5816) +- Add `Enabled` method to `Processor` interface in `go.opentelemetry.io/otel/sdk/log` that accepts a newly introduced `EnabledParameters` type. (#5816) ### Changed From 7e9e7ce281846a9734adcfe247bbac5b3def483f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 13 Sep 2024 17:59:45 +0200 Subject: [PATCH 11/11] Update processor.go --- sdk/log/processor.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/sdk/log/processor.go b/sdk/log/processor.go index bfae25f51d8..144d7a30c8a 100644 --- a/sdk/log/processor.go +++ b/sdk/log/processor.go @@ -50,8 +50,6 @@ type Processor interface { // // The SDK invokes the processors sequentially in the same order as // they were registered using [WithProcessor] until any processor returns true. - // - // Implementations should not modify the param. Enabled(ctx context.Context, param EnabledParameters) bool // Shutdown is called when the SDK shuts down. Any cleanup or release of