From 23f7b41e0acefe0f371eb2202f03a53fe22327cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 13 Sep 2024 06:35:01 +0200 Subject: [PATCH] log: Introduce EnabledParameters (#5791) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://github.com/open-telemetry/opentelemetry-go/issues/5769 Related spec PR: https://github.com/open-telemetry/opentelemetry-specification/pull/4203 Remark: A follow-up in contrib is required and afterwards here to fix the `example/dice`. Benchstat results for `sdk/log` (`log` has no benchmarks related to Enabled): ``` goos: linux goarch: amd64 pkg: go.opentelemetry.io/otel/sdk/log cpu: Intel(R) Core(TM) i9-10885H CPU @ 2.40GHz │ old.txt │ new.txt │ │ sec/op │ sec/op vs base │ BatchProcessorOnEmit-16 398.8n ± 10% 395.6n ± 12% ~ (p=0.971 n=10) Processor/Simple-16 882.2n ± 8% 869.8n ± 9% ~ (p=0.811 n=10) Processor/Batch-16 1.478µ ± 3% 1.485µ ± 5% ~ (p=0.646 n=10) Processor/SetTimestampSimple-16 847.8n ± 1% 844.6n ± 3% ~ (p=0.247 n=10) Processor/SetTimestampBatch-16 1.480µ ± 3% 1.473µ ± 4% ~ (p=0.700 n=10) Processor/AddAttributesSimple-16 930.0n ± 1% 933.8n ± 1% ~ (p=0.172 n=10) Processor/AddAttributesBatch-16 1.624µ ± 2% 1.639µ ± 2% ~ (p=0.839 n=10) Processor/SetAttributesSimple-16 903.4n ± 1% 895.1n ± 1% ~ (p=0.190 n=10) Processor/SetAttributesBatch-16 1.554µ ± 4% 1.529µ ± 3% ~ (p=0.159 n=10) LoggerNewRecord/5_attributes-16 346.0n ± 2% 343.3n ± 2% ~ (p=0.448 n=10) LoggerNewRecord/10_attributes-16 1.608µ ± 6% 1.503µ ± 2% -6.53% (p=0.007 n=10) LoggerEnabled-16 34.305n ± 8% 6.706n ± 1% -80.45% (p=0.000 n=10) LoggerProviderLogger-16 636.9n ± 10% 605.8n ± 3% ~ (p=0.105 n=10) WalkAttributes/1_attributes-16 5.363n ± 3% 4.540n ± 14% -15.34% (p=0.002 n=10) WalkAttributes/10_attributes-16 5.436n ± 7% 4.461n ± 2% -17.95% (p=0.000 n=10) WalkAttributes/100_attributes-16 5.126n ± 9% 4.465n ± 1% -12.90% (p=0.000 n=10) WalkAttributes/1000_attributes-16 5.316n ± 9% 4.502n ± 5% -15.32% (p=0.002 n=10) SetAddAttributes/SetAttributes-16 220.5n ± 18% 192.6n ± 11% -12.67% (p=0.007 n=10) SetAddAttributes/AddAttributes-16 165.3n ± 21% 127.3n ± 22% -22.96% (p=0.011 n=10) SimpleProcessorOnEmit-16 2.159n ± 9% 2.167n ± 9% ~ (p=0.739 n=10) geomean 178.3n 154.5n -13.31% │ old.txt │ new.txt │ │ B/s │ B/s vs base │ BatchProcessorOnEmit-16 76.52Mi ± 11% 77.14Mi ± 14% ~ (p=0.971 n=10) │ old.txt │ new.txt │ │ B/op │ B/op vs base │ BatchProcessorOnEmit-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Processor/Simple-16 417.0 ± 0% 417.0 ± 0% ~ (p=1.000 n=10) ¹ Processor/Batch-16 1.093Ki ± 1% 1.088Ki ± 1% ~ (p=0.254 n=10) Processor/SetTimestampSimple-16 417.0 ± 0% 417.0 ± 0% ~ (p=1.000 n=10) ¹ Processor/SetTimestampBatch-16 1.095Ki ± 1% 1.084Ki ± 2% ~ (p=0.361 n=10) Processor/AddAttributesSimple-16 417.0 ± 0% 417.0 ± 0% ~ (p=1.000 n=10) ¹ Processor/AddAttributesBatch-16 1.085Ki ± 1% 1.086Ki ± 1% ~ (p=1.000 n=10) Processor/SetAttributesSimple-16 465.0 ± 0% 465.0 ± 0% ~ (p=1.000 n=10) ¹ Processor/SetAttributesBatch-16 1.129Ki ± 1% 1.125Ki ± 1% ~ (p=0.084 n=10) LoggerNewRecord/5_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ LoggerNewRecord/10_attributes-16 610.0 ± 0% 610.0 ± 0% ~ (p=1.000 n=10) ¹ LoggerEnabled-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ LoggerProviderLogger-16 359.0 ± 6% 346.0 ± 3% ~ (p=0.117 n=10) WalkAttributes/1_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ WalkAttributes/10_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ WalkAttributes/100_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ WalkAttributes/1000_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ SetAddAttributes/SetAttributes-16 48.00 ± 0% 48.00 ± 0% ~ (p=1.000 n=10) ¹ SetAddAttributes/AddAttributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ SimpleProcessorOnEmit-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ geomean ² -0.27% ² ¹ all samples are equal ² summaries must be >0 to compute geomean │ old.txt │ new.txt │ │ allocs/op │ allocs/op vs base │ BatchProcessorOnEmit-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ Processor/Simple-16 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ Processor/Batch-16 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ Processor/SetTimestampSimple-16 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ Processor/SetTimestampBatch-16 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ Processor/AddAttributesSimple-16 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ Processor/AddAttributesBatch-16 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ Processor/SetAttributesSimple-16 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=10) ¹ Processor/SetAttributesBatch-16 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=10) ¹ LoggerNewRecord/5_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ LoggerNewRecord/10_attributes-16 4.000 ± 0% 4.000 ± 0% ~ (p=1.000 n=10) ¹ LoggerEnabled-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ LoggerProviderLogger-16 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ WalkAttributes/1_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ WalkAttributes/10_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ WalkAttributes/100_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ WalkAttributes/1000_attributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ SetAddAttributes/SetAttributes-16 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ SetAddAttributes/AddAttributes-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ SimpleProcessorOnEmit-16 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ geomean ² +0.00% ² ¹ all samples are equal ² summaries must be >0 to compute geomean ``` --- CHANGELOG.md | 2 ++ example/dice/go.mod | 1 - example/dice/go.sum | 2 -- example/dice/rolldice.go | 7 +++++-- log/DESIGN.md | 17 +++++++++++++++++ log/internal/global/log.go | 4 ++-- log/internal/global/log_test.go | 8 +++++--- log/logger.go | 30 ++++++++++++++++++++++++------ log/logtest/recorder.go | 12 ++++++------ log/logtest/recorder_test.go | 24 ++++++++++++------------ log/noop/noop.go | 2 +- sdk/log/example_test.go | 11 +++-------- sdk/log/internal/x/x.go | 19 ++++++++++--------- sdk/log/logger.go | 15 +++++++-------- sdk/log/logger_test.go | 8 ++++---- sdk/log/provider_test.go | 4 ++-- 16 files changed, 100 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9f3ba2b6..8261f72ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### 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) diff --git a/example/dice/go.mod b/example/dice/go.mod index d4d0bfcdf..e4d557fcb 100644 --- a/example/dice/go.mod +++ b/example/dice/go.mod @@ -3,7 +3,6 @@ module go.opentelemetry.io/otel/example/dice go 1.22 require ( - go.opentelemetry.io/contrib/bridges/otelslog v0.5.0 go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.55.0 go.opentelemetry.io/otel v1.30.0 go.opentelemetry.io/otel/exporters/stdout/stdoutlog v0.6.0 diff --git a/example/dice/go.sum b/example/dice/go.sum index c493a7eb7..aa8744703 100644 --- a/example/dice/go.sum +++ b/example/dice/go.sum @@ -15,8 +15,6 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= -go.opentelemetry.io/contrib/bridges/otelslog v0.5.0 h1:lU3F57OSLK5mQ1PDBVAfDDaKCPv37MrEbCfTzsF4bz0= -go.opentelemetry.io/contrib/bridges/otelslog v0.5.0/go.mod h1:I84u06zJFr8T5D73fslEUbnRBimVVSBhuVw8L8I92AU= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.55.0 h1:ZIg3ZT/aQ7AfKqdwp7ECpOK6vHqquXXuyTjIO8ZdmPs= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.55.0/go.mod h1:DQAwmETtZV00skUwgD6+0U89g80NKsJE3DCKeLLPQMI= golang.org/x/sys v0.25.0 h1:r+8e+loiHxRqhXVl6ML1nO3l1+oFoWbnlu2Ehimmi34= diff --git a/example/dice/rolldice.go b/example/dice/rolldice.go index 1869b5183..ffc509cb2 100644 --- a/example/dice/rolldice.go +++ b/example/dice/rolldice.go @@ -6,11 +6,14 @@ package main import ( "fmt" "io" + "log/slog" "math/rand" "net/http" + "os" "strconv" - "go.opentelemetry.io/contrib/bridges/otelslog" + // TODO: https://github.com/open-telemetry/opentelemetry-go/issues/5801 + // "go.opentelemetry.io/contrib/bridges/otelslog". "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" @@ -21,7 +24,7 @@ const name = "go.opentelemetry.io/otel/example/dice" var ( tracer = otel.Tracer(name) meter = otel.Meter(name) - logger = otelslog.NewLogger(name) + logger = slog.New(slog.NewJSONHandler(os.Stdout, nil)) // TODO: logger = otelslog.NewLogger(name). rollCnt metric.Int64Counter ) diff --git a/log/DESIGN.md b/log/DESIGN.md index 7f4873835..2d0c27b1e 100644 --- a/log/DESIGN.md +++ b/log/DESIGN.md @@ -253,6 +253,23 @@ Rejected alternatives: - [Add XYZ method to Logger](#add-xyz-method-to-logger) - [Rename KeyValue to Attr](#rename-keyvalue-to-attr) +### Logger.Enabled + +The `Enabled` method implements the [`Enabled` operation](https://opentelemetry.io/docs/specs/otel/logs/bridge-api/#enabled). + +[`Context` associated with the `LogRecord`](https://opentelemetry.io/docs/specs/otel/context/) +is accepted as a `context.Context` method argument. + +Calls to `Enabled` are supposed to be on the hot path and the list of arguments +can be extendend in future. Therefore, in order to reduce the number of heap +allocations and make it possible to handle new arguments, `Enabled` accepts +a `EnabledParameters` struct, defined in [logger.go](logger.go), as the second +method argument. + +The `EnabledParameters` getters are returning values using the `(value, ok)` +idiom in order to indicate if the values were actually set by the caller or if +there are unspecified. + ### noop package The `go.opentelemetry.io/otel/log/noop` package provides diff --git a/log/internal/global/log.go b/log/internal/global/log.go index acd333e8b..8a27358d4 100644 --- a/log/internal/global/log.go +++ b/log/internal/global/log.go @@ -87,10 +87,10 @@ func (l *logger) Emit(ctx context.Context, r log.Record) { } } -func (l *logger) Enabled(ctx context.Context, r log.Record) bool { +func (l *logger) Enabled(ctx context.Context, param log.EnabledParameters) bool { var enabled bool if del, ok := l.delegate.Load().(log.Logger); ok { - enabled = del.Enabled(ctx, r) + enabled = del.Enabled(ctx, param) } return enabled } diff --git a/log/internal/global/log_test.go b/log/internal/global/log_test.go index f465abc90..a27f83211 100644 --- a/log/internal/global/log_test.go +++ b/log/internal/global/log_test.go @@ -51,11 +51,12 @@ func TestLoggerConcurrentSafe(t *testing.T) { ctx := context.Background() var r log.Record + var param log.EnabledParameters var enabled bool for { l.Emit(ctx, r) - enabled = l.Enabled(ctx, r) + enabled = l.Enabled(ctx, param) select { case <-stop: @@ -103,16 +104,17 @@ type testLogger struct { } func (l *testLogger) Emit(context.Context, log.Record) { l.emitN++ } -func (l *testLogger) Enabled(context.Context, log.Record) bool { +func (l *testLogger) Enabled(context.Context, log.EnabledParameters) bool { l.enabledN++ return true } func emitRecord(l log.Logger) { ctx := context.Background() + var param log.EnabledParameters var r log.Record - _ = l.Enabled(ctx, r) + _ = l.Enabled(ctx, param) l.Emit(ctx, r) } diff --git a/log/logger.go b/log/logger.go index df2e88ea6..985d0494f 100644 --- a/log/logger.go +++ b/log/logger.go @@ -31,26 +31,26 @@ type Logger interface { Emit(ctx context.Context, record Record) // Enabled returns whether the Logger emits for the given context and - // record. + // param. // - // 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 + // The passed param is likely to be a partial record with only the + // bridge-relevant information being provided (e.g a param 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 Logger will emit for the - // provided context and record, and will be false if the Logger will not + // provided context and param, and will be false if the Logger will not // emit. 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 record should not be held by the implementation. A copy should be + // The param should not be held by the implementation. A copy should be // made if the record needs to be held after the call returns. // // Implementations of this method need to be safe for a user to call // concurrently. - Enabled(ctx context.Context, record Record) bool + Enabled(ctx context.Context, param EnabledParameters) bool } // LoggerOption applies configuration options to a [Logger]. @@ -129,3 +129,21 @@ func WithSchemaURL(schemaURL string) LoggerOption { return config }) } + +// EnabledParameters represents payload for [Logger]'s Enabled method. +type EnabledParameters struct { + severity Severity + severitySet bool +} + +// 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 Severity, ok bool) { + return r.severity, r.severitySet +} + +// SetSeverity sets the [Severity] level. +func (r *EnabledParameters) SetSeverity(level Severity) { + r.severity = level + r.severitySet = true +} diff --git a/log/logtest/recorder.go b/log/logtest/recorder.go index b3d45d647..84449b143 100644 --- a/log/logtest/recorder.go +++ b/log/logtest/recorder.go @@ -11,9 +11,9 @@ import ( "go.opentelemetry.io/otel/log/embedded" ) -type enabledFn func(context.Context, log.Record) bool +type enabledFn func(context.Context, log.EnabledParameters) bool -var defaultEnabledFunc = func(context.Context, log.Record) bool { +var defaultEnabledFunc = func(context.Context, log.EnabledParameters) bool { return true } @@ -42,7 +42,7 @@ func (f optFunc) apply(c config) config { return f(c) } // WithEnabledFunc allows configuring whether the [Recorder] is enabled for specific log entries or not. // // By default, the Recorder is enabled for every log entry. -func WithEnabledFunc(fn func(context.Context, log.Record) bool) Option { +func WithEnabledFunc(fn func(context.Context, log.EnabledParameters) bool) Option { return optFunc(func(c config) config { c.enabledFn = fn return c @@ -155,12 +155,12 @@ type logger struct { } // Enabled indicates whether a specific record should be stored. -func (l *logger) Enabled(ctx context.Context, record log.Record) bool { +func (l *logger) Enabled(ctx context.Context, opts log.EnabledParameters) bool { if l.enabledFn == nil { - return defaultEnabledFunc(ctx, record) + return defaultEnabledFunc(ctx, opts) } - return l.enabledFn(ctx, record) + return l.enabledFn(ctx, opts) } // Emit stores the log record. diff --git a/log/logtest/recorder_test.go b/log/logtest/recorder_test.go index 15f67d0e6..7d413ff84 100644 --- a/log/logtest/recorder_test.go +++ b/log/logtest/recorder_test.go @@ -65,18 +65,18 @@ func TestRecorderLoggerCreatesNewStruct(t *testing.T) { func TestLoggerEnabled(t *testing.T) { for _, tt := range []struct { - name string - options []Option - ctx context.Context - buildRecord func() log.Record + name string + options []Option + ctx context.Context + buildEnabledParameters func() log.EnabledParameters isEnabled bool }{ { name: "the default option enables every log entry", ctx: context.Background(), - buildRecord: func() log.Record { - return log.Record{} + buildEnabledParameters: func() log.EnabledParameters { + return log.EnabledParameters{} }, isEnabled: true, @@ -84,20 +84,20 @@ func TestLoggerEnabled(t *testing.T) { { name: "with everything disabled", options: []Option{ - WithEnabledFunc(func(context.Context, log.Record) bool { + WithEnabledFunc(func(context.Context, log.EnabledParameters) bool { return false }), }, ctx: context.Background(), - buildRecord: func() log.Record { - return log.Record{} + buildEnabledParameters: func() log.EnabledParameters { + return log.EnabledParameters{} }, isEnabled: false, }, } { t.Run(tt.name, func(t *testing.T) { - e := NewRecorder(tt.options...).Logger("test").Enabled(tt.ctx, tt.buildRecord()) + e := NewRecorder(tt.options...).Logger("test").Enabled(tt.ctx, tt.buildEnabledParameters()) assert.Equal(t, tt.isEnabled, e) }) } @@ -105,7 +105,7 @@ func TestLoggerEnabled(t *testing.T) { func TestLoggerEnabledFnUnset(t *testing.T) { r := &logger{} - assert.True(t, r.Enabled(context.Background(), log.Record{})) + assert.True(t, r.Enabled(context.Background(), log.EnabledParameters{})) } func TestRecorderEmitAndReset(t *testing.T) { @@ -158,7 +158,7 @@ func TestRecorderConcurrentSafe(t *testing.T) { defer wg.Done() nr := r.Logger("test") - nr.Enabled(context.Background(), log.Record{}) + nr.Enabled(context.Background(), log.EnabledParameters{}) nr.Emit(context.Background(), log.Record{}) r.Result() diff --git a/log/noop/noop.go b/log/noop/noop.go index d2e21edba..f45a7c7e0 100644 --- a/log/noop/noop.go +++ b/log/noop/noop.go @@ -47,4 +47,4 @@ type Logger struct{ embedded.Logger } func (Logger) Emit(context.Context, log.Record) {} // Enabled returns false. No log records are ever emitted. -func (Logger) Enabled(context.Context, log.Record) bool { return false } +func (Logger) Enabled(context.Context, log.EnabledParameters) bool { return false } diff --git a/sdk/log/example_test.go b/sdk/log/example_test.go index be92d2677..8070beef7 100644 --- a/sdk/log/example_test.go +++ b/sdk/log/example_test.go @@ -90,7 +90,7 @@ type ContextFilterProcessor struct { } type filter interface { - Enabled(ctx context.Context, record log.Record) bool + Enabled(ctx context.Context, param logapi.EnabledParameters) bool } func (p *ContextFilterProcessor) OnEmit(ctx context.Context, record *log.Record) error { @@ -100,13 +100,13 @@ 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 { +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, record)) + return !ignoreLogs(ctx) && (p.filter == nil || p.filter.Enabled(ctx, param)) } func ignoreLogs(ctx context.Context) bool { @@ -147,11 +147,6 @@ func (p *RedactTokensProcessor) OnEmit(ctx context.Context, record *log.Record) return nil } -// Enabled returns true. -func (p *RedactTokensProcessor) Enabled(context.Context, log.Record) bool { - return true -} - // Shutdown returns nil. func (p *RedactTokensProcessor) Shutdown(ctx context.Context) error { return nil diff --git a/sdk/log/internal/x/x.go b/sdk/log/internal/x/x.go index 9b3f8b7b0..ca78d1097 100644 --- a/sdk/log/internal/x/x.go +++ b/sdk/log/internal/x/x.go @@ -10,37 +10,38 @@ import ( "go.opentelemetry.io/otel/log" ) -// FilterProcessor is a [Processor] that knows, and can identify, what -// [log.Record] it will process or drop when it is passed to OnEmit. +// 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 +// 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 [Processor]s that can make reliable +// 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 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 + // 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 record. - Enabled(ctx context.Context, record log.Record) bool + // 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 db41c0570..d6ca2ea41 100644 --- a/sdk/log/logger.go +++ b/sdk/log/logger.go @@ -44,24 +44,23 @@ 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 the record for the provided context. +// that created the logger will process param for the provided context and param. // -// If it is not possible to definitively determine the record will be +// 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 the -// record. -func (l *logger) Enabled(ctx context.Context, record log.Record) bool { +// 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, record, fltrs) + return len(l.provider.processors) > len(fltrs) || anyEnabled(ctx, param, fltrs) } -func anyEnabled(ctx context.Context, r log.Record, fltrs []x.FilterProcessor) bool { +func anyEnabled(ctx context.Context, param log.EnabledParameters, fltrs []x.FilterProcessor) bool { for _, f := range fltrs { - if f.Enabled(ctx, r) { + if f.Enabled(ctx, param) { // At least one Processor will process the Record. return true } diff --git a/sdk/log/logger_test.go b/sdk/log/logger_test.go index bfa967232..0c2f793db 100644 --- a/sdk/log/logger_test.go +++ b/sdk/log/logger_test.go @@ -270,7 +270,7 @@ func TestLoggerEnabled(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.expected, tc.logger.Enabled(tc.ctx, log.Record{})) + assert.Equal(t, tc.expected, tc.logger.Enabled(tc.ctx, log.EnabledParameters{})) }) } } @@ -281,8 +281,8 @@ func BenchmarkLoggerEnabled(b *testing.B) { WithProcessor(newFltrProcessor("1", true)), ) logger := provider.Logger("BenchmarkLoggerEnabled") - ctx, r := context.Background(), log.Record{} - r.SetSeverityText("test") + ctx, param := context.Background(), log.EnabledParameters{} + param.SetSeverity(log.SeverityDebug) var enabled bool @@ -290,7 +290,7 @@ func BenchmarkLoggerEnabled(b *testing.B) { b.ResetTimer() for n := 0; n < b.N; n++ { - enabled = logger.Enabled(ctx, r) + enabled = logger.Enabled(ctx, param) } _ = enabled diff --git a/sdk/log/provider_test.go b/sdk/log/provider_test.go index cbe355c23..b9374e9d9 100644 --- a/sdk/log/provider_test.go +++ b/sdk/log/provider_test.go @@ -76,7 +76,7 @@ func newFltrProcessor(name string, enabled bool) *fltrProcessor { } } -func (p *fltrProcessor) Enabled(context.Context, log.Record) bool { +func (p *fltrProcessor) Enabled(context.Context, log.EnabledParameters) bool { return p.enabled } @@ -384,5 +384,5 @@ func BenchmarkLoggerProviderLogger(b *testing.B) { } b.StopTimer() - loggers[0].Enabled(context.Background(), log.Record{}) + loggers[0].Enabled(context.Background(), log.EnabledParameters{}) }