From 51dca45ff0b7dd079ec39f465f5a66a43e773534 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 29 Nov 2024 13:22:27 +0100 Subject: [PATCH 1/7] log: Change EnabledParameters to have fields instead of methods --- log/logger.go | 15 +-------------- sdk/log/logger_test.go | 7 +++---- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/log/logger.go b/log/logger.go index fe826819d8b..0773a49b608 100644 --- a/log/logger.go +++ b/log/logger.go @@ -138,18 +138,5 @@ func WithSchemaURL(schemaURL string) LoggerOption { // 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 + Severity Severity } diff --git a/sdk/log/logger_test.go b/sdk/log/logger_test.go index 0c2f793db97..b8da0750bad 100644 --- a/sdk/log/logger_test.go +++ b/sdk/log/logger_test.go @@ -280,10 +280,9 @@ func BenchmarkLoggerEnabled(b *testing.B) { WithProcessor(newFltrProcessor("0", false)), WithProcessor(newFltrProcessor("1", true)), ) - logger := provider.Logger("BenchmarkLoggerEnabled") - ctx, param := context.Background(), log.EnabledParameters{} - param.SetSeverity(log.SeverityDebug) - + logger := provider.Logger(b.Name()) + ctx := context.Background() + param := log.EnabledParameters{Severity: log.SeverityDebug} var enabled bool b.ReportAllocs() From f8b0b2bf9cdd279fd63d04a4abfe1c5785d8004d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 29 Nov 2024 13:38:56 +0100 Subject: [PATCH 2/7] log: Rename EnabledParameters to EnabledParams --- log/internal/global/log.go | 2 +- log/internal/global/log_test.go | 6 +++--- log/logger.go | 6 +++--- log/logtest/recorder.go | 8 ++++---- log/logtest/recorder_test.go | 16 ++++++++-------- log/noop/noop.go | 2 +- sdk/log/example_test.go | 4 ++-- sdk/log/internal/x/x.go | 2 +- sdk/log/logger.go | 4 ++-- sdk/log/logger_test.go | 4 ++-- sdk/log/provider_test.go | 4 ++-- 11 files changed, 29 insertions(+), 29 deletions(-) diff --git a/log/internal/global/log.go b/log/internal/global/log.go index d97ee966350..dd593c289ee 100644 --- a/log/internal/global/log.go +++ b/log/internal/global/log.go @@ -94,7 +94,7 @@ func (l *logger) Emit(ctx context.Context, r log.Record) { } } -func (l *logger) Enabled(ctx context.Context, param log.EnabledParameters) bool { +func (l *logger) Enabled(ctx context.Context, param log.EnabledParams) bool { var enabled bool if del, ok := l.delegate.Load().(log.Logger); ok { enabled = del.Enabled(ctx, param) diff --git a/log/internal/global/log_test.go b/log/internal/global/log_test.go index ae2c5b2a6dd..9b7fb3b5981 100644 --- a/log/internal/global/log_test.go +++ b/log/internal/global/log_test.go @@ -52,7 +52,7 @@ func TestLoggerConcurrentSafe(t *testing.T) { ctx := context.Background() var r log.Record - var param log.EnabledParameters + var param log.EnabledParams var enabled bool for { @@ -105,14 +105,14 @@ type testLogger struct { } func (l *testLogger) Emit(context.Context, log.Record) { l.emitN++ } -func (l *testLogger) Enabled(context.Context, log.EnabledParameters) bool { +func (l *testLogger) Enabled(context.Context, log.EnabledParams) bool { l.enabledN++ return true } func emitRecord(l log.Logger) { ctx := context.Background() - var param log.EnabledParameters + var param log.EnabledParams var r log.Record _ = l.Enabled(ctx, param) diff --git a/log/logger.go b/log/logger.go index 0773a49b608..83fa3f3a16e 100644 --- a/log/logger.go +++ b/log/logger.go @@ -56,7 +56,7 @@ type Logger interface { // // Notice: Enabled is intended to be used by log bridges. // Is should not be used for writing instrumentation. - Enabled(ctx context.Context, param EnabledParameters) bool + Enabled(ctx context.Context, param EnabledParams) bool } // LoggerOption applies configuration options to a [Logger]. @@ -136,7 +136,7 @@ func WithSchemaURL(schemaURL string) LoggerOption { }) } -// EnabledParameters represents payload for [Logger]'s Enabled method. -type EnabledParameters struct { +// EnabledParams represents payload for [Logger]'s Enabled method. +type EnabledParams struct { Severity Severity } diff --git a/log/logtest/recorder.go b/log/logtest/recorder.go index fd986c9afc4..af7c55af77e 100644 --- a/log/logtest/recorder.go +++ b/log/logtest/recorder.go @@ -12,9 +12,9 @@ import ( "go.opentelemetry.io/otel/log/embedded" ) -type enabledFn func(context.Context, log.EnabledParameters) bool +type enabledFn func(context.Context, log.EnabledParams) bool -var defaultEnabledFunc = func(context.Context, log.EnabledParameters) bool { +var defaultEnabledFunc = func(context.Context, log.EnabledParams) bool { return true } @@ -43,7 +43,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.EnabledParameters) bool) Option { +func WithEnabledFunc(fn func(context.Context, log.EnabledParams) bool) Option { return optFunc(func(c config) config { c.enabledFn = fn return c @@ -159,7 +159,7 @@ type logger struct { } // Enabled indicates whether a specific record should be stored. -func (l *logger) Enabled(ctx context.Context, opts log.EnabledParameters) bool { +func (l *logger) Enabled(ctx context.Context, opts log.EnabledParams) bool { if l.enabledFn == nil { return defaultEnabledFunc(ctx, opts) } diff --git a/log/logtest/recorder_test.go b/log/logtest/recorder_test.go index 4efab499504..22cfbfbb6c5 100644 --- a/log/logtest/recorder_test.go +++ b/log/logtest/recorder_test.go @@ -71,15 +71,15 @@ func TestLoggerEnabled(t *testing.T) { name string options []Option ctx context.Context - buildEnabledParameters func() log.EnabledParameters + buildEnabledParameters func() log.EnabledParams isEnabled bool }{ { name: "the default option enables every log entry", ctx: context.Background(), - buildEnabledParameters: func() log.EnabledParameters { - return log.EnabledParameters{} + buildEnabledParameters: func() log.EnabledParams { + return log.EnabledParams{} }, isEnabled: true, @@ -87,13 +87,13 @@ func TestLoggerEnabled(t *testing.T) { { name: "with everything disabled", options: []Option{ - WithEnabledFunc(func(context.Context, log.EnabledParameters) bool { + WithEnabledFunc(func(context.Context, log.EnabledParams) bool { return false }), }, ctx: context.Background(), - buildEnabledParameters: func() log.EnabledParameters { - return log.EnabledParameters{} + buildEnabledParameters: func() log.EnabledParams { + return log.EnabledParams{} }, isEnabled: false, @@ -108,7 +108,7 @@ func TestLoggerEnabled(t *testing.T) { func TestLoggerEnabledFnUnset(t *testing.T) { r := &logger{} - assert.True(t, r.Enabled(context.Background(), log.EnabledParameters{})) + assert.True(t, r.Enabled(context.Background(), log.EnabledParams{})) } func TestRecorderEmitAndReset(t *testing.T) { @@ -161,7 +161,7 @@ func TestRecorderConcurrentSafe(t *testing.T) { defer wg.Done() nr := r.Logger("test") - nr.Enabled(context.Background(), log.EnabledParameters{}) + nr.Enabled(context.Background(), log.EnabledParams{}) nr.Emit(context.Background(), log.Record{}) r.Result() diff --git a/log/noop/noop.go b/log/noop/noop.go index f45a7c7e0b3..d1e792e517e 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.EnabledParameters) bool { return false } +func (Logger) Enabled(context.Context, log.EnabledParams) bool { return false } diff --git a/sdk/log/example_test.go b/sdk/log/example_test.go index 8070beef771..2b3bb2c3b34 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, param logapi.EnabledParameters) bool + Enabled(ctx context.Context, param logapi.EnabledParams) bool } func (p *ContextFilterProcessor) OnEmit(ctx context.Context, record *log.Record) error { @@ -100,7 +100,7 @@ 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 { +func (p *ContextFilterProcessor) Enabled(ctx context.Context, param logapi.EnabledParams) bool { p.lazyFilter.Do(func() { if f, ok := p.Processor.(filter); ok { p.filter = f diff --git a/sdk/log/internal/x/x.go b/sdk/log/internal/x/x.go index ca78d109778..3a876e1369f 100644 --- a/sdk/log/internal/x/x.go +++ b/sdk/log/internal/x/x.go @@ -43,5 +43,5 @@ type FilterProcessor interface { // indeterminate state. // // Implementations should not modify the param. - Enabled(ctx context.Context, param log.EnabledParameters) bool + Enabled(ctx context.Context, param log.EnabledParams) bool } diff --git a/sdk/log/logger.go b/sdk/log/logger.go index d6ca2ea41aa..43c65d06922 100644 --- a/sdk/log/logger.go +++ b/sdk/log/logger.go @@ -49,7 +49,7 @@ func (l *logger) Emit(ctx context.Context, r log.Record) { // 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 { +func (l *logger) Enabled(ctx context.Context, param log.EnabledParams) 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. @@ -58,7 +58,7 @@ func (l *logger) Enabled(ctx context.Context, param log.EnabledParameters) bool return len(l.provider.processors) > len(fltrs) || anyEnabled(ctx, param, fltrs) } -func anyEnabled(ctx context.Context, param log.EnabledParameters, fltrs []x.FilterProcessor) bool { +func anyEnabled(ctx context.Context, param log.EnabledParams, fltrs []x.FilterProcessor) bool { for _, f := range fltrs { if f.Enabled(ctx, param) { // At least one Processor will process the Record. diff --git a/sdk/log/logger_test.go b/sdk/log/logger_test.go index b8da0750bad..27bcfc3fe41 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.EnabledParameters{})) + assert.Equal(t, tc.expected, tc.logger.Enabled(tc.ctx, log.EnabledParams{})) }) } } @@ -282,7 +282,7 @@ func BenchmarkLoggerEnabled(b *testing.B) { ) logger := provider.Logger(b.Name()) ctx := context.Background() - param := log.EnabledParameters{Severity: log.SeverityDebug} + param := log.EnabledParams{Severity: log.SeverityDebug} var enabled bool b.ReportAllocs() diff --git a/sdk/log/provider_test.go b/sdk/log/provider_test.go index 133e752896b..5dfcc488795 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.EnabledParameters) bool { +func (p *fltrProcessor) Enabled(context.Context, log.EnabledParams) bool { return p.enabled } @@ -388,5 +388,5 @@ func BenchmarkLoggerProviderLogger(b *testing.B) { } b.StopTimer() - loggers[0].Enabled(context.Background(), log.EnabledParameters{}) + loggers[0].Enabled(context.Background(), log.EnabledParams{}) } From 1061bb603771a35a6dda6f0d689ea9ce58485af0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 29 Nov 2024 13:39:50 +0100 Subject: [PATCH 3/7] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a19036caee0..0c13546982e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Propagate non-retryable error messages to client in `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#5929) - Propagate non-retryable error messages to client in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#5929) - Propagate non-retryable error messages to client in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#5929) +- Change `EnabledParameters` to simple `EnabledParams` in `go.opentelemetry.io/otel/log`. (#6009) ### Fixed From 7fb4f3cf4324864cb3237b00d4dc7729313672fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 29 Nov 2024 13:43:52 +0100 Subject: [PATCH 4/7] Refactor tests --- log/logtest/recorder_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/log/logtest/recorder_test.go b/log/logtest/recorder_test.go index 22cfbfbb6c5..8e87cea9e24 100644 --- a/log/logtest/recorder_test.go +++ b/log/logtest/recorder_test.go @@ -68,17 +68,17 @@ func TestRecorderLoggerCreatesNewStruct(t *testing.T) { func TestLoggerEnabled(t *testing.T) { for _, tt := range []struct { - name string - options []Option - ctx context.Context - buildEnabledParameters func() log.EnabledParams + name string + options []Option + ctx context.Context + buildEnabledParams func() log.EnabledParams isEnabled bool }{ { name: "the default option enables every log entry", ctx: context.Background(), - buildEnabledParameters: func() log.EnabledParams { + buildEnabledParams: func() log.EnabledParams { return log.EnabledParams{} }, @@ -92,7 +92,7 @@ func TestLoggerEnabled(t *testing.T) { }), }, ctx: context.Background(), - buildEnabledParameters: func() log.EnabledParams { + buildEnabledParams: func() log.EnabledParams { return log.EnabledParams{} }, @@ -100,7 +100,7 @@ func TestLoggerEnabled(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - e := NewRecorder(tt.options...).Logger("test").Enabled(tt.ctx, tt.buildEnabledParameters()) + e := NewRecorder(tt.options...).Logger("test").Enabled(tt.ctx, tt.buildEnabledParams()) assert.Equal(t, tt.isEnabled, e) }) } From bf846bfd1a076f853b10fdad2669a78e98cab4e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 29 Nov 2024 13:43:59 +0100 Subject: [PATCH 5/7] Update design doc --- log/DESIGN.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/log/DESIGN.md b/log/DESIGN.md index da1865191a1..d4db065e38c 100644 --- a/log/DESIGN.md +++ b/log/DESIGN.md @@ -261,12 +261,12 @@ 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 +a `EnabledParams` 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. +The `EnabledParams` uses fields, instead of getters and setters, to allow +simpler usage which allows configuring the `EnabledParams` in the same line +where `Enabled` is called. ### noop package From ac62cf9e6a6f19079c0d9d861c5c7e9044c6b72d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 3 Dec 2024 08:53:28 +0100 Subject: [PATCH 6/7] Revert the rename --- CHANGELOG.md | 2 +- log/DESIGN.md | 6 +++--- log/internal/global/log.go | 2 +- log/internal/global/log_test.go | 6 +++--- log/logger.go | 6 +++--- log/logtest/recorder.go | 8 ++++---- log/logtest/recorder_test.go | 16 ++++++++-------- log/noop/noop.go | 2 +- sdk/log/example_test.go | 4 ++-- sdk/log/internal/x/x.go | 2 +- sdk/log/logger.go | 4 ++-- sdk/log/logger_test.go | 4 ++-- sdk/log/provider_test.go | 4 ++-- 13 files changed, 33 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c13546982e..358954028b5 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 - Propagate non-retryable error messages to client in `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#5929) - Propagate non-retryable error messages to client in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#5929) - Propagate non-retryable error messages to client in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#5929) -- Change `EnabledParameters` to simple `EnabledParams` in `go.opentelemetry.io/otel/log`. (#6009) +- Change `EnabledParameters` to have a `Severity` field instead of a getter and setter in `go.opentelemetry.io/otel/log`. (#6009) ### Fixed diff --git a/log/DESIGN.md b/log/DESIGN.md index d4db065e38c..568df49d96e 100644 --- a/log/DESIGN.md +++ b/log/DESIGN.md @@ -261,11 +261,11 @@ 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 `EnabledParams` struct, defined in [logger.go](logger.go), as the second +a `EnabledParameters` struct, defined in [logger.go](logger.go), as the second method argument. -The `EnabledParams` uses fields, instead of getters and setters, to allow -simpler usage which allows configuring the `EnabledParams` in the same line +The `EnabledParameters` uses fields, instead of getters and setters, to allow +simpler usage which allows configuring the `EnabledParameters` in the same line where `Enabled` is called. ### noop package diff --git a/log/internal/global/log.go b/log/internal/global/log.go index dd593c289ee..d97ee966350 100644 --- a/log/internal/global/log.go +++ b/log/internal/global/log.go @@ -94,7 +94,7 @@ func (l *logger) Emit(ctx context.Context, r log.Record) { } } -func (l *logger) Enabled(ctx context.Context, param log.EnabledParams) 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, param) diff --git a/log/internal/global/log_test.go b/log/internal/global/log_test.go index 9b7fb3b5981..ae2c5b2a6dd 100644 --- a/log/internal/global/log_test.go +++ b/log/internal/global/log_test.go @@ -52,7 +52,7 @@ func TestLoggerConcurrentSafe(t *testing.T) { ctx := context.Background() var r log.Record - var param log.EnabledParams + var param log.EnabledParameters var enabled bool for { @@ -105,14 +105,14 @@ type testLogger struct { } func (l *testLogger) Emit(context.Context, log.Record) { l.emitN++ } -func (l *testLogger) Enabled(context.Context, log.EnabledParams) 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.EnabledParams + var param log.EnabledParameters var r log.Record _ = l.Enabled(ctx, param) diff --git a/log/logger.go b/log/logger.go index 83fa3f3a16e..0773a49b608 100644 --- a/log/logger.go +++ b/log/logger.go @@ -56,7 +56,7 @@ type Logger interface { // // Notice: Enabled is intended to be used by log bridges. // Is should not be used for writing instrumentation. - Enabled(ctx context.Context, param EnabledParams) bool + Enabled(ctx context.Context, param EnabledParameters) bool } // LoggerOption applies configuration options to a [Logger]. @@ -136,7 +136,7 @@ func WithSchemaURL(schemaURL string) LoggerOption { }) } -// EnabledParams represents payload for [Logger]'s Enabled method. -type EnabledParams struct { +// EnabledParameters represents payload for [Logger]'s Enabled method. +type EnabledParameters struct { Severity Severity } diff --git a/log/logtest/recorder.go b/log/logtest/recorder.go index af7c55af77e..fd986c9afc4 100644 --- a/log/logtest/recorder.go +++ b/log/logtest/recorder.go @@ -12,9 +12,9 @@ import ( "go.opentelemetry.io/otel/log/embedded" ) -type enabledFn func(context.Context, log.EnabledParams) bool +type enabledFn func(context.Context, log.EnabledParameters) bool -var defaultEnabledFunc = func(context.Context, log.EnabledParams) bool { +var defaultEnabledFunc = func(context.Context, log.EnabledParameters) bool { return true } @@ -43,7 +43,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.EnabledParams) bool) Option { +func WithEnabledFunc(fn func(context.Context, log.EnabledParameters) bool) Option { return optFunc(func(c config) config { c.enabledFn = fn return c @@ -159,7 +159,7 @@ type logger struct { } // Enabled indicates whether a specific record should be stored. -func (l *logger) Enabled(ctx context.Context, opts log.EnabledParams) bool { +func (l *logger) Enabled(ctx context.Context, opts log.EnabledParameters) bool { if l.enabledFn == nil { return defaultEnabledFunc(ctx, opts) } diff --git a/log/logtest/recorder_test.go b/log/logtest/recorder_test.go index 8e87cea9e24..eb24266082d 100644 --- a/log/logtest/recorder_test.go +++ b/log/logtest/recorder_test.go @@ -71,15 +71,15 @@ func TestLoggerEnabled(t *testing.T) { name string options []Option ctx context.Context - buildEnabledParams func() log.EnabledParams + buildEnabledParams func() log.EnabledParameters isEnabled bool }{ { name: "the default option enables every log entry", ctx: context.Background(), - buildEnabledParams: func() log.EnabledParams { - return log.EnabledParams{} + buildEnabledParams: func() log.EnabledParameters { + return log.EnabledParameters{} }, isEnabled: true, @@ -87,13 +87,13 @@ func TestLoggerEnabled(t *testing.T) { { name: "with everything disabled", options: []Option{ - WithEnabledFunc(func(context.Context, log.EnabledParams) bool { + WithEnabledFunc(func(context.Context, log.EnabledParameters) bool { return false }), }, ctx: context.Background(), - buildEnabledParams: func() log.EnabledParams { - return log.EnabledParams{} + buildEnabledParams: func() log.EnabledParameters { + return log.EnabledParameters{} }, isEnabled: false, @@ -108,7 +108,7 @@ func TestLoggerEnabled(t *testing.T) { func TestLoggerEnabledFnUnset(t *testing.T) { r := &logger{} - assert.True(t, r.Enabled(context.Background(), log.EnabledParams{})) + assert.True(t, r.Enabled(context.Background(), log.EnabledParameters{})) } func TestRecorderEmitAndReset(t *testing.T) { @@ -161,7 +161,7 @@ func TestRecorderConcurrentSafe(t *testing.T) { defer wg.Done() nr := r.Logger("test") - nr.Enabled(context.Background(), log.EnabledParams{}) + 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 d1e792e517e..f45a7c7e0b3 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.EnabledParams) 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 2b3bb2c3b34..8070beef771 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, param logapi.EnabledParams) bool + Enabled(ctx context.Context, param logapi.EnabledParameters) bool } func (p *ContextFilterProcessor) OnEmit(ctx context.Context, record *log.Record) error { @@ -100,7 +100,7 @@ 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.EnabledParams) 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 diff --git a/sdk/log/internal/x/x.go b/sdk/log/internal/x/x.go index 3a876e1369f..ca78d109778 100644 --- a/sdk/log/internal/x/x.go +++ b/sdk/log/internal/x/x.go @@ -43,5 +43,5 @@ type FilterProcessor interface { // indeterminate state. // // Implementations should not modify the param. - Enabled(ctx context.Context, param log.EnabledParams) bool + Enabled(ctx context.Context, param log.EnabledParameters) bool } diff --git a/sdk/log/logger.go b/sdk/log/logger.go index 43c65d06922..d6ca2ea41aa 100644 --- a/sdk/log/logger.go +++ b/sdk/log/logger.go @@ -49,7 +49,7 @@ func (l *logger) Emit(ctx context.Context, r log.Record) { // 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.EnabledParams) bool { +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. @@ -58,7 +58,7 @@ func (l *logger) Enabled(ctx context.Context, param log.EnabledParams) bool { return len(l.provider.processors) > len(fltrs) || anyEnabled(ctx, param, fltrs) } -func anyEnabled(ctx context.Context, param log.EnabledParams, fltrs []x.FilterProcessor) bool { +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. diff --git a/sdk/log/logger_test.go b/sdk/log/logger_test.go index 27bcfc3fe41..b8da0750bad 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.EnabledParams{})) + assert.Equal(t, tc.expected, tc.logger.Enabled(tc.ctx, log.EnabledParameters{})) }) } } @@ -282,7 +282,7 @@ func BenchmarkLoggerEnabled(b *testing.B) { ) logger := provider.Logger(b.Name()) ctx := context.Background() - param := log.EnabledParams{Severity: log.SeverityDebug} + param := log.EnabledParameters{Severity: log.SeverityDebug} var enabled bool b.ReportAllocs() diff --git a/sdk/log/provider_test.go b/sdk/log/provider_test.go index 5dfcc488795..133e752896b 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.EnabledParams) bool { +func (p *fltrProcessor) Enabled(context.Context, log.EnabledParameters) bool { return p.enabled } @@ -388,5 +388,5 @@ func BenchmarkLoggerProviderLogger(b *testing.B) { } b.StopTimer() - loggers[0].Enabled(context.Background(), log.EnabledParams{}) + loggers[0].Enabled(context.Background(), log.EnabledParameters{}) } From a3f93f168432d9d9ee6dabc210e7f3d79dfd06b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 3 Dec 2024 17:26:02 +0100 Subject: [PATCH 7/7] Refactor TestLoggerEnabled --- log/logtest/recorder_test.go | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/log/logtest/recorder_test.go b/log/logtest/recorder_test.go index eb24266082d..28d814528e2 100644 --- a/log/logtest/recorder_test.go +++ b/log/logtest/recorder_test.go @@ -68,21 +68,16 @@ func TestRecorderLoggerCreatesNewStruct(t *testing.T) { func TestLoggerEnabled(t *testing.T) { for _, tt := range []struct { - name string - options []Option - ctx context.Context - buildEnabledParams func() log.EnabledParameters - - isEnabled bool + name string + options []Option + ctx context.Context + enabledParams log.EnabledParameters + want bool }{ { name: "the default option enables every log entry", ctx: context.Background(), - buildEnabledParams: func() log.EnabledParameters { - return log.EnabledParameters{} - }, - - isEnabled: true, + want: true, }, { name: "with everything disabled", @@ -91,17 +86,13 @@ func TestLoggerEnabled(t *testing.T) { return false }), }, - ctx: context.Background(), - buildEnabledParams: func() log.EnabledParameters { - return log.EnabledParameters{} - }, - - isEnabled: false, + ctx: context.Background(), + want: false, }, } { t.Run(tt.name, func(t *testing.T) { - e := NewRecorder(tt.options...).Logger("test").Enabled(tt.ctx, tt.buildEnabledParams()) - assert.Equal(t, tt.isEnabled, e) + e := NewRecorder(tt.options...).Logger("test").Enabled(tt.ctx, tt.enabledParams) + assert.Equal(t, tt.want, e) }) } }