From 8b5aa7511073649630ef8ce5b6fbac2746e97cd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 6 Sep 2024 08:59:20 +0200 Subject: [PATCH 01/12] [Spike] log: Introduce EnabledParam --- example/dice/go.mod | 1 - example/dice/go.sum | 2 -- example/dice/rolldice.go | 7 +++++-- log/enabled.go | 19 +++++++++++++++++++ log/internal/global/log.go | 4 ++-- log/internal/global/log_test.go | 8 +++++--- log/logger.go | 10 +++++----- log/logtest/recorder.go | 12 ++++++------ log/logtest/recorder_test.go | 24 ++++++++++++------------ log/noop/noop.go | 2 +- sdk/log/internal/x/x.go | 10 +++++----- sdk/log/logger.go | 8 ++++---- sdk/log/logger_test.go | 8 ++++---- sdk/log/provider_test.go | 4 ++-- 14 files changed, 70 insertions(+), 49 deletions(-) create mode 100644 log/enabled.go diff --git a/example/dice/go.mod b/example/dice/go.mod index 76b81b37c67..b2301c27741 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.4.0 go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.54.0 go.opentelemetry.io/otel v1.29.0 go.opentelemetry.io/otel/exporters/stdout/stdoutlog v0.5.0 diff --git a/example/dice/go.sum b/example/dice/go.sum index 3912716f243..867f758b0b0 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.4.0 h1:i66F95zqmrf3EyN5gu0E2pjTvCRZo/p8XIYidG3vOP8= -go.opentelemetry.io/contrib/bridges/otelslog v0.4.0/go.mod h1:JuCiVizZ6ovLZLnYk1nGRUEAnmRJLKGh5v8DmwiKlhY= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.54.0 h1:TT4fX+nBOA/+LUkobKGW1ydGcn+G3vRw9+g5HwCphpk= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.54.0/go.mod h1:L7UH0GbB0p47T4Rri3uHjbpCFYrVrwc1I25QhNPiGK8= 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 186771df25e..ca6e039826e 100644 --- a/example/dice/rolldice.go +++ b/example/dice/rolldice.go @@ -4,7 +4,10 @@ package main import ( - "go.opentelemetry.io/contrib/bridges/otelslog" + // TODO: "go.opentelemetry.io/contrib/bridges/otelslog". + "log/slog" + "os" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/metric" ) @@ -14,7 +17,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/enabled.go b/log/enabled.go new file mode 100644 index 00000000000..103be11eed6 --- /dev/null +++ b/log/enabled.go @@ -0,0 +1,19 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package log // import "go.opentelemetry.io/otel/log" + +// EnabledParam represents payload for [Logger]'s Enabled method. +type EnabledParam struct { + severity Severity +} + +// Severity returns the [Severity] level. +func (r *EnabledParam) Severity() Severity { + return r.severity +} + +// SetSeverity sets the [Severity] level. +func (r *EnabledParam) SetSeverity(level Severity) { + r.severity = level +} diff --git a/log/internal/global/log.go b/log/internal/global/log.go index acd333e8b19..6ef716f735a 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.EnabledParam) 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 f465abc9011..79e4cb39103 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 p log.EnabledParam var enabled bool for { l.Emit(ctx, r) - enabled = l.Enabled(ctx, r) + enabled = l.Enabled(ctx, p) 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.EnabledParam) bool { l.enabledN++ return true } func emitRecord(l log.Logger) { ctx := context.Background() + var p log.EnabledParam var r log.Record - _ = l.Enabled(ctx, r) + _ = l.Enabled(ctx, p) l.Emit(ctx, r) } diff --git a/log/logger.go b/log/logger.go index df2e88ea6b2..a4117082466 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 + // 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 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 EnabledParam) bool } // LoggerOption applies configuration options to a [Logger]. diff --git a/log/logtest/recorder.go b/log/logtest/recorder.go index b3d45d647ae..05785a9dd1d 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.EnabledParam) bool -var defaultEnabledFunc = func(context.Context, log.Record) bool { +var defaultEnabledFunc = func(context.Context, log.EnabledParam) 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.EnabledParam) 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, param log.EnabledParam) bool { if l.enabledFn == nil { - return defaultEnabledFunc(ctx, record) + return defaultEnabledFunc(ctx, param) } - return l.enabledFn(ctx, record) + return l.enabledFn(ctx, param) } // Emit stores the log record. diff --git a/log/logtest/recorder_test.go b/log/logtest/recorder_test.go index 15f67d0e6de..649d9e85ac7 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 + buildEnabledParam func() log.EnabledParam isEnabled bool }{ { name: "the default option enables every log entry", ctx: context.Background(), - buildRecord: func() log.Record { - return log.Record{} + buildEnabledParam: func() log.EnabledParam { + return log.EnabledParam{} }, 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.EnabledParam) bool { return false }), }, ctx: context.Background(), - buildRecord: func() log.Record { - return log.Record{} + buildEnabledParam: func() log.EnabledParam { + return log.EnabledParam{} }, 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.buildEnabledParam()) 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.EnabledParam{})) } 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.EnabledParam{}) nr.Emit(context.Background(), log.Record{}) r.Result() diff --git a/log/noop/noop.go b/log/noop/noop.go index d2e21edba66..4569005d5e2 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.EnabledParam) bool { return false } diff --git a/sdk/log/internal/x/x.go b/sdk/log/internal/x/x.go index 9b3f8b7b069..b1f9395329b 100644 --- a/sdk/log/internal/x/x.go +++ b/sdk/log/internal/x/x.go @@ -29,18 +29,18 @@ import ( // [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.EnabledParam) bool } diff --git a/sdk/log/logger.go b/sdk/log/logger.go index db41c057005..6a6ff84e8a6 100644 --- a/sdk/log/logger.go +++ b/sdk/log/logger.go @@ -50,18 +50,18 @@ func (l *logger) Emit(ctx context.Context, r log.Record) { // 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 { +func (l *logger) Enabled(ctx context.Context, param log.EnabledParam) 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.EnabledParam, 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 bfa967232ec..b7f529e7e9c 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.EnabledParam{})) }) } } @@ -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.EnabledParam{} + 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 4dd9cf327e9..18c68e82c15 100644 --- a/sdk/log/provider_test.go +++ b/sdk/log/provider_test.go @@ -72,7 +72,7 @@ func newFltrProcessor(name string, enabled bool) *fltrProcessor { } } -func (p *fltrProcessor) Enabled(context.Context, log.Record) bool { +func (p *fltrProcessor) Enabled(context.Context, log.EnabledParam) bool { return p.enabled } @@ -321,5 +321,5 @@ func BenchmarkLoggerProviderLogger(b *testing.B) { } b.StopTimer() - loggers[0].Enabled(context.Background(), log.Record{}) + loggers[0].Enabled(context.Background(), log.EnabledParam{}) } From 85ee299324216c2e430c4dd908ed73243d428a4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 6 Sep 2024 09:07:43 +0200 Subject: [PATCH 02/12] Fix doc comment --- log/logger.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/log/logger.go b/log/logger.go index a4117082466..e84bd35d31a 100644 --- a/log/logger.go +++ b/log/logger.go @@ -34,7 +34,7 @@ type Logger interface { // 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 + // 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). // From 2924ccb03c989b794d112362b257cfe4a206300d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Mon, 9 Sep 2024 10:32:27 +0200 Subject: [PATCH 03/12] Update DESIGN.md --- log/DESIGN.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/log/DESIGN.md b/log/DESIGN.md index 2bb8c3a643f..9768066d9af 100644 --- a/log/DESIGN.md +++ b/log/DESIGN.md @@ -253,6 +253,19 @@ 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 `EnabledParam` struct, defined in [enabled.go](enabled.go), as the second +method argument. + ### noop package The `go.opentelemetry.io/otel/log/noop` package provides From 35944b75f26aabfebc5d7cf2e74a524827a92399 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Mon, 9 Sep 2024 10:57:24 +0200 Subject: [PATCH 04/12] Update examples --- sdk/log/example_test.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/sdk/log/example_test.go b/sdk/log/example_test.go index be92d2677c4..587fbd738db 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.EnabledParam) 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.EnabledParam) 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 From 450fc7a0c7eb084f8f7b39ee0a3b194f7ee254c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Mon, 9 Sep 2024 17:22:42 +0200 Subject: [PATCH 05/12] Rename EnabledParam to EnabledOpts --- log/DESIGN.md | 2 +- log/enabled.go | 8 ++++---- log/internal/global/log.go | 4 ++-- log/internal/global/log_test.go | 8 ++++---- log/logger.go | 12 ++++++------ log/logtest/recorder.go | 12 ++++++------ log/logtest/recorder_test.go | 24 ++++++++++++------------ log/noop/noop.go | 2 +- sdk/log/example_test.go | 6 +++--- sdk/log/internal/x/x.go | 19 ++++++++++--------- sdk/log/logger.go | 13 ++++++------- sdk/log/logger_test.go | 4 ++-- sdk/log/provider_test.go | 4 ++-- 13 files changed, 59 insertions(+), 59 deletions(-) diff --git a/log/DESIGN.md b/log/DESIGN.md index 84e432dede7..e1ae576b637 100644 --- a/log/DESIGN.md +++ b/log/DESIGN.md @@ -263,7 +263,7 @@ 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 `EnabledParam` struct, defined in [enabled.go](enabled.go), as the second +a `EnabledOpts` struct, defined in [enabled.go](enabled.go), as the second method argument. ### noop package diff --git a/log/enabled.go b/log/enabled.go index 103be11eed6..115fcbf6ce1 100644 --- a/log/enabled.go +++ b/log/enabled.go @@ -3,17 +3,17 @@ package log // import "go.opentelemetry.io/otel/log" -// EnabledParam represents payload for [Logger]'s Enabled method. -type EnabledParam struct { +// EnabledOpts represents payload for [Logger]'s Enabled method. +type EnabledOpts struct { severity Severity } // Severity returns the [Severity] level. -func (r *EnabledParam) Severity() Severity { +func (r *EnabledOpts) Severity() Severity { return r.severity } // SetSeverity sets the [Severity] level. -func (r *EnabledParam) SetSeverity(level Severity) { +func (r *EnabledOpts) SetSeverity(level Severity) { r.severity = level } diff --git a/log/internal/global/log.go b/log/internal/global/log.go index 6ef716f735a..0190f449ba1 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, param log.EnabledParam) bool { +func (l *logger) Enabled(ctx context.Context, opts log.EnabledOpts) bool { var enabled bool if del, ok := l.delegate.Load().(log.Logger); ok { - enabled = del.Enabled(ctx, param) + enabled = del.Enabled(ctx, opts) } return enabled } diff --git a/log/internal/global/log_test.go b/log/internal/global/log_test.go index 79e4cb39103..8ca07b1e258 100644 --- a/log/internal/global/log_test.go +++ b/log/internal/global/log_test.go @@ -51,12 +51,12 @@ func TestLoggerConcurrentSafe(t *testing.T) { ctx := context.Background() var r log.Record - var p log.EnabledParam + var opts log.EnabledOpts var enabled bool for { l.Emit(ctx, r) - enabled = l.Enabled(ctx, p) + enabled = l.Enabled(ctx, opts) select { case <-stop: @@ -104,14 +104,14 @@ type testLogger struct { } func (l *testLogger) Emit(context.Context, log.Record) { l.emitN++ } -func (l *testLogger) Enabled(context.Context, log.EnabledParam) bool { +func (l *testLogger) Enabled(context.Context, log.EnabledOpts) bool { l.enabledN++ return true } func emitRecord(l log.Logger) { ctx := context.Background() - var p log.EnabledParam + var p log.EnabledOpts var r log.Record _ = l.Enabled(ctx, p) diff --git a/log/logger.go b/log/logger.go index e84bd35d31a..ffe20a24fc4 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 - // param. + // opts. // - // 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 + // The passed opts is likely to be a partial record with only the + // bridge-relevant information being provided (e.g a opts 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 param, and will be false if the Logger will not + // provided context and opts, 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 param should not be held by the implementation. A copy should be + // The opts 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, param EnabledParam) bool + Enabled(ctx context.Context, opts EnabledOpts) bool } // LoggerOption applies configuration options to a [Logger]. diff --git a/log/logtest/recorder.go b/log/logtest/recorder.go index 05785a9dd1d..716e4d84a3e 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.EnabledParam) bool +type enabledFn func(context.Context, log.EnabledOpts) bool -var defaultEnabledFunc = func(context.Context, log.EnabledParam) bool { +var defaultEnabledFunc = func(context.Context, log.EnabledOpts) 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.EnabledParam) bool) Option { +func WithEnabledFunc(fn func(context.Context, log.EnabledOpts) 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, param log.EnabledParam) bool { +func (l *logger) Enabled(ctx context.Context, opts log.EnabledOpts) bool { if l.enabledFn == nil { - return defaultEnabledFunc(ctx, param) + return defaultEnabledFunc(ctx, opts) } - return l.enabledFn(ctx, param) + 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 649d9e85ac7..2928472c879 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 - buildEnabledParam func() log.EnabledParam + name string + options []Option + ctx context.Context + buildEnabledOpts func() log.EnabledOpts isEnabled bool }{ { name: "the default option enables every log entry", ctx: context.Background(), - buildEnabledParam: func() log.EnabledParam { - return log.EnabledParam{} + buildEnabledOpts: func() log.EnabledOpts { + return log.EnabledOpts{} }, isEnabled: true, @@ -84,20 +84,20 @@ func TestLoggerEnabled(t *testing.T) { { name: "with everything disabled", options: []Option{ - WithEnabledFunc(func(context.Context, log.EnabledParam) bool { + WithEnabledFunc(func(context.Context, log.EnabledOpts) bool { return false }), }, ctx: context.Background(), - buildEnabledParam: func() log.EnabledParam { - return log.EnabledParam{} + buildEnabledOpts: func() log.EnabledOpts { + return log.EnabledOpts{} }, isEnabled: false, }, } { t.Run(tt.name, func(t *testing.T) { - e := NewRecorder(tt.options...).Logger("test").Enabled(tt.ctx, tt.buildEnabledParam()) + e := NewRecorder(tt.options...).Logger("test").Enabled(tt.ctx, tt.buildEnabledOpts()) 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.EnabledParam{})) + assert.True(t, r.Enabled(context.Background(), log.EnabledOpts{})) } 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.EnabledParam{}) + nr.Enabled(context.Background(), log.EnabledOpts{}) nr.Emit(context.Background(), log.Record{}) r.Result() diff --git a/log/noop/noop.go b/log/noop/noop.go index 4569005d5e2..3d05c82b031 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.EnabledParam) bool { return false } +func (Logger) Enabled(context.Context, log.EnabledOpts) bool { return false } diff --git a/sdk/log/example_test.go b/sdk/log/example_test.go index 587fbd738db..964b57e9050 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.EnabledParam) bool + Enabled(ctx context.Context, opts logapi.EnabledOpts) 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, param logapi.EnabledParam) bool { +func (p *ContextFilterProcessor) Enabled(ctx context.Context, opts logapi.EnabledOpts) 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)) + return !ignoreLogs(ctx) && (p.filter == nil || p.filter.Enabled(ctx, opts)) } func ignoreLogs(ctx context.Context) bool { diff --git a/sdk/log/internal/x/x.go b/sdk/log/internal/x/x.go index b1f9395329b..c8e7d3e581a 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 param. + // and opts. // - // The passed param is likely to be a partial record with only the + // The passed opts 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 + // provided context and opts, 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.EnabledParam) bool + // Implementations should not modify the opts. + Enabled(ctx context.Context, opts log.EnabledOpts) bool } diff --git a/sdk/log/logger.go b/sdk/log/logger.go index 6a6ff84e8a6..be8c2dc0450 100644 --- a/sdk/log/logger.go +++ b/sdk/log/logger.go @@ -44,22 +44,21 @@ 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 opts for the provided context and opts. // -// If it is not possible to definitively determine the record will be +// If it is not possible to definitively determine the opts 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, param log.EnabledParam) bool { +// returned if it can be positively verified that no Processor will process. +func (l *logger) Enabled(ctx context.Context, opts log.EnabledOpts) 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) + return len(l.provider.processors) > len(fltrs) || anyEnabled(ctx, opts, fltrs) } -func anyEnabled(ctx context.Context, param log.EnabledParam, fltrs []x.FilterProcessor) bool { +func anyEnabled(ctx context.Context, param log.EnabledOpts, 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 b7f529e7e9c..d854af58444 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.EnabledParam{})) + assert.Equal(t, tc.expected, tc.logger.Enabled(tc.ctx, log.EnabledOpts{})) }) } } @@ -281,7 +281,7 @@ func BenchmarkLoggerEnabled(b *testing.B) { WithProcessor(newFltrProcessor("1", true)), ) logger := provider.Logger("BenchmarkLoggerEnabled") - ctx, param := context.Background(), log.EnabledParam{} + ctx, param := context.Background(), log.EnabledOpts{} param.SetSeverity(log.SeverityDebug) var enabled bool diff --git a/sdk/log/provider_test.go b/sdk/log/provider_test.go index 664d3c64ec7..24d0293906b 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.EnabledParam) bool { +func (p *fltrProcessor) Enabled(context.Context, log.EnabledOpts) bool { return p.enabled } @@ -384,5 +384,5 @@ func BenchmarkLoggerProviderLogger(b *testing.B) { } b.StopTimer() - loggers[0].Enabled(context.Background(), log.EnabledParam{}) + loggers[0].Enabled(context.Background(), log.EnabledOpts{}) } From 6dff94eb8ebe7d61072d89b5f4545a223fd12331 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Mon, 9 Sep 2024 17:22:54 +0200 Subject: [PATCH 06/12] Update changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 016f9e3fb8c..7d6525ce810 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `WithResource` option for `NewMeterProvider` now merges the provided resources with the ones from environment variables. (#5773) - The `WithResource` option for `NewLoggerProvider` now merges the provided resources with the ones from environment variables. (#5773) +### Changed + +- `Logger.Enabled` in `go.opentelemetry.io/otel/log` now accepts a newly introduced `EnabledOpts` type instead of `Record`. (#5791) +- `FilterProcessor.Enabled` in `go.opentelemetry.io/otel/sdk/log/internal/x` now accepts `EnabledOpts` instead of `Record`. (#5791) + ### Fixed - Fix memory leak in the global `MeterProvider` when identical instruments are repeatedly created. (#5754) From b9a617178a9204a79551088b51a780eef9f57668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 10 Sep 2024 06:56:29 +0200 Subject: [PATCH 07/12] Rename EnabledOpts to EnabledParameters --- CHANGELOG.md | 4 ++-- log/DESIGN.md | 2 +- log/enabled.go | 8 ++++---- log/internal/global/log.go | 4 ++-- log/internal/global/log_test.go | 10 +++++----- log/logger.go | 12 ++++++------ log/logtest/recorder.go | 8 ++++---- log/logtest/recorder_test.go | 24 ++++++++++++------------ log/noop/noop.go | 2 +- sdk/log/example_test.go | 6 +++--- sdk/log/internal/x/x.go | 10 +++++----- sdk/log/logger.go | 10 +++++----- sdk/log/logger_test.go | 4 ++-- sdk/log/provider_test.go | 4 ++-- 14 files changed, 54 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 610d9bb7148..fb9fbc96d4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,8 +17,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Changed -- `Logger.Enabled` in `go.opentelemetry.io/otel/log` now accepts a newly introduced `EnabledOpts` type instead of `Record`. (#5791) -- `FilterProcessor.Enabled` in `go.opentelemetry.io/otel/sdk/log/internal/x` now accepts `EnabledOpts` instead of `Record`. (#5791) +- `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) ### Fixed diff --git a/log/DESIGN.md b/log/DESIGN.md index e1ae576b637..f8908cee5ec 100644 --- a/log/DESIGN.md +++ b/log/DESIGN.md @@ -263,7 +263,7 @@ 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 `EnabledOpts` struct, defined in [enabled.go](enabled.go), as the second +a `EnabledParameters` struct, defined in [enabled.go](enabled.go), as the second method argument. ### noop package diff --git a/log/enabled.go b/log/enabled.go index 115fcbf6ce1..f5c0e67df73 100644 --- a/log/enabled.go +++ b/log/enabled.go @@ -3,17 +3,17 @@ package log // import "go.opentelemetry.io/otel/log" -// EnabledOpts represents payload for [Logger]'s Enabled method. -type EnabledOpts struct { +// EnabledParameters represents payload for [Logger]'s Enabled method. +type EnabledParameters struct { severity Severity } // Severity returns the [Severity] level. -func (r *EnabledOpts) Severity() Severity { +func (r *EnabledParameters) Severity() Severity { return r.severity } // SetSeverity sets the [Severity] level. -func (r *EnabledOpts) SetSeverity(level Severity) { +func (r *EnabledParameters) SetSeverity(level Severity) { r.severity = level } diff --git a/log/internal/global/log.go b/log/internal/global/log.go index 0190f449ba1..8a27358d4ba 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, opts log.EnabledOpts) 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, opts) + enabled = del.Enabled(ctx, param) } return enabled } diff --git a/log/internal/global/log_test.go b/log/internal/global/log_test.go index 8ca07b1e258..a27f832113d 100644 --- a/log/internal/global/log_test.go +++ b/log/internal/global/log_test.go @@ -51,12 +51,12 @@ func TestLoggerConcurrentSafe(t *testing.T) { ctx := context.Background() var r log.Record - var opts log.EnabledOpts + var param log.EnabledParameters var enabled bool for { l.Emit(ctx, r) - enabled = l.Enabled(ctx, opts) + enabled = l.Enabled(ctx, param) select { case <-stop: @@ -104,17 +104,17 @@ type testLogger struct { } func (l *testLogger) Emit(context.Context, log.Record) { l.emitN++ } -func (l *testLogger) Enabled(context.Context, log.EnabledOpts) bool { +func (l *testLogger) Enabled(context.Context, log.EnabledParameters) bool { l.enabledN++ return true } func emitRecord(l log.Logger) { ctx := context.Background() - var p log.EnabledOpts + var param log.EnabledParameters var r log.Record - _ = l.Enabled(ctx, p) + _ = l.Enabled(ctx, param) l.Emit(ctx, r) } diff --git a/log/logger.go b/log/logger.go index ffe20a24fc4..90dfc791490 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 - // opts. + // param. // - // The passed opts is likely to be a partial record with only the - // bridge-relevant information being provided (e.g a opts 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 opts, 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 opts 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, opts EnabledOpts) bool + Enabled(ctx context.Context, param EnabledParameters) bool } // LoggerOption applies configuration options to a [Logger]. diff --git a/log/logtest/recorder.go b/log/logtest/recorder.go index 716e4d84a3e..84449b14377 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.EnabledOpts) bool +type enabledFn func(context.Context, log.EnabledParameters) bool -var defaultEnabledFunc = func(context.Context, log.EnabledOpts) 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.EnabledOpts) bool) Option { +func WithEnabledFunc(fn func(context.Context, log.EnabledParameters) bool) Option { return optFunc(func(c config) config { c.enabledFn = fn return c @@ -155,7 +155,7 @@ type logger struct { } // Enabled indicates whether a specific record should be stored. -func (l *logger) Enabled(ctx context.Context, opts log.EnabledOpts) 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 2928472c879..7d413ff841b 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 - buildEnabledOpts func() log.EnabledOpts + name string + options []Option + ctx context.Context + buildEnabledParameters func() log.EnabledParameters isEnabled bool }{ { name: "the default option enables every log entry", ctx: context.Background(), - buildEnabledOpts: func() log.EnabledOpts { - return log.EnabledOpts{} + 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.EnabledOpts) bool { + WithEnabledFunc(func(context.Context, log.EnabledParameters) bool { return false }), }, ctx: context.Background(), - buildEnabledOpts: func() log.EnabledOpts { - return log.EnabledOpts{} + 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.buildEnabledOpts()) + 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.EnabledOpts{})) + 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.EnabledOpts{}) + 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 3d05c82b031..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.EnabledOpts) 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 964b57e9050..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, opts logapi.EnabledOpts) 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, opts logapi.EnabledOpts) 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, opts)) + return !ignoreLogs(ctx) && (p.filter == nil || p.filter.Enabled(ctx, param)) } func ignoreLogs(ctx context.Context) bool { diff --git a/sdk/log/internal/x/x.go b/sdk/log/internal/x/x.go index c8e7d3e581a..ca78d109778 100644 --- a/sdk/log/internal/x/x.go +++ b/sdk/log/internal/x/x.go @@ -30,18 +30,18 @@ import ( // [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 opts. + // and param. // - // The passed opts 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 opts, 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 opts. - Enabled(ctx context.Context, opts log.EnabledOpts) 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 be8c2dc0450..d6ca2ea41aa 100644 --- a/sdk/log/logger.go +++ b/sdk/log/logger.go @@ -44,21 +44,21 @@ 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 opts for the provided context and opts. +// that created the logger will process param for the provided context and param. // -// If it is not possible to definitively determine the opts 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. -func (l *logger) Enabled(ctx context.Context, opts log.EnabledOpts) 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. // // If all Processors are FilterProcessors, check if any is enabled. - return len(l.provider.processors) > len(fltrs) || anyEnabled(ctx, opts, fltrs) + return len(l.provider.processors) > len(fltrs) || anyEnabled(ctx, param, fltrs) } -func anyEnabled(ctx context.Context, param log.EnabledOpts, 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 d854af58444..0c2f793db97 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.EnabledOpts{})) + assert.Equal(t, tc.expected, tc.logger.Enabled(tc.ctx, log.EnabledParameters{})) }) } } @@ -281,7 +281,7 @@ func BenchmarkLoggerEnabled(b *testing.B) { WithProcessor(newFltrProcessor("1", true)), ) logger := provider.Logger("BenchmarkLoggerEnabled") - ctx, param := context.Background(), log.EnabledOpts{} + ctx, param := context.Background(), log.EnabledParameters{} param.SetSeverity(log.SeverityDebug) var enabled bool diff --git a/sdk/log/provider_test.go b/sdk/log/provider_test.go index 24d0293906b..b9374e9d934 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.EnabledOpts) 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.EnabledOpts{}) + loggers[0].Enabled(context.Background(), log.EnabledParameters{}) } From ae37bbad440ff8403c24fd6ff112801fe4450ba8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 10 Sep 2024 18:08:47 +0200 Subject: [PATCH 08/12] Update rolldice.go --- example/dice/rolldice.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/example/dice/rolldice.go b/example/dice/rolldice.go index ca6e039826e..c7aa8070ce9 100644 --- a/example/dice/rolldice.go +++ b/example/dice/rolldice.go @@ -4,7 +4,8 @@ package main import ( - // TODO: "go.opentelemetry.io/contrib/bridges/otelslog". + // TODO: https://github.com/open-telemetry/opentelemetry-go/issues/5801 + // "go.opentelemetry.io/contrib/bridges/otelslog". "log/slog" "os" From 9db67acf75c934899a286d017b8b44b538013679 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 11 Sep 2024 18:53:59 +0200 Subject: [PATCH 09/12] Refactor EnabledParameters --- log/enabled.go | 19 ------------------- log/logger.go | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+), 19 deletions(-) delete mode 100644 log/enabled.go diff --git a/log/enabled.go b/log/enabled.go deleted file mode 100644 index f5c0e67df73..00000000000 --- a/log/enabled.go +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package log // import "go.opentelemetry.io/otel/log" - -// EnabledParameters represents payload for [Logger]'s Enabled method. -type EnabledParameters struct { - severity Severity -} - -// Severity returns the [Severity] level. -func (r *EnabledParameters) Severity() Severity { - return r.severity -} - -// SetSeverity sets the [Severity] level. -func (r *EnabledParameters) SetSeverity(level Severity) { - r.severity = level -} diff --git a/log/logger.go b/log/logger.go index 90dfc791490..985d0494f4a 100644 --- a/log/logger.go +++ b/log/logger.go @@ -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 +} From 0d8a4d79f05e8877795deb2f5a82d1bb6795c957 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 11 Sep 2024 18:55:36 +0200 Subject: [PATCH 10/12] Fix changelog --- CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ca5f292e68..52981b1d4b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Changed + +- `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) + @@ -20,11 +25,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `WithResource` option for `NewLoggerProvider` now merges the provided resources with the ones from environment variables. (#5773) - Add UTF-8 support to `go.opentelemetry.io/otel/exporters/prometheus`. (#5755) -### Changed - -- `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) - ### Fixed - Fix memory leak in the global `MeterProvider` when identical instruments are repeatedly created. (#5754) From 682eecc2b36d68994cbf5f15d4581da0098ad8b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 11 Sep 2024 18:56:16 +0200 Subject: [PATCH 11/12] Update hyperlink --- log/DESIGN.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/log/DESIGN.md b/log/DESIGN.md index f8908cee5ec..822c544f913 100644 --- a/log/DESIGN.md +++ b/log/DESIGN.md @@ -263,7 +263,7 @@ 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 [enabled.go](enabled.go), as the second +a `EnabledParameters` struct, defined in [logger.go](logger.go), as the second method argument. ### noop package From 57ee0509524331d0b041fedc6e2593f3f02e03a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 11 Sep 2024 18:58:57 +0200 Subject: [PATCH 12/12] Update design doc --- log/DESIGN.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/log/DESIGN.md b/log/DESIGN.md index 822c544f913..2d0c27b1e25 100644 --- a/log/DESIGN.md +++ b/log/DESIGN.md @@ -266,6 +266,10 @@ 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