From 2a7b54cfe560668e65b425b4073a6ec355b4bb2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 21 Feb 2024 09:48:14 +0100 Subject: [PATCH 01/30] Create Logs SDK design doc --- sdk/log/DESIGN.md | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 sdk/log/DESIGN.md diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md new file mode 100644 index 00000000000..01b2c068015 --- /dev/null +++ b/sdk/log/DESIGN.md @@ -0,0 +1,37 @@ +# Logs SDK + +## Abstract + +`go.opentelemetry.io/otel/sdk/log` provides Logs SDK compliant with the +[specification](https://opentelemetry.io/docs/specs/otel/logs/sdk/). + +The main and recommended use case is to configure the SDK to use an OTLP +exporter with a batch processor.[^1] Therefore, the design aims to be +high-performant in this scenario. + +The prototype was created in TODO. + +## Module structure + +The SDK is published as a single `go.opentelemetry.io/otel/sdk/log` Go module. + +The Go module consists of the following packages: + +- `go.opentelemetry.io/otel/sdk/log` +- `go.opentelemetry.io/otel/sdk/log/logtest` + +The exporters are published as following Go modules: + +- `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc` +- `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp` +- `go.opentelemetry.io/otel/exporters/stdout/stdoutlog` + +## Rejected alternatives + +## Open issues + +The Logs SDK NOT be released as stable before all issues below are closed: + +- TBD + +[^1]: [OpenTelemetry Logging](https://opentelemetry.io/docs/specs/otel/logs) From 7268786ad8ce30ce37a738af13fd0874c46d6774 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 21 Feb 2024 10:23:26 +0100 Subject: [PATCH 02/30] Update prototype PR hyperlink --- sdk/log/DESIGN.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 01b2c068015..903f2663e6e 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -9,7 +9,8 @@ The main and recommended use case is to configure the SDK to use an OTLP exporter with a batch processor.[^1] Therefore, the design aims to be high-performant in this scenario. -The prototype was created in TODO. +The prototype was created in +[#4955](https://github.com/open-telemetry/opentelemetry-go/pull/4955). ## Module structure From 45e6efe32672ede807e3f239579a8aaed2b7fa0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 21 Feb 2024 11:34:26 +0100 Subject: [PATCH 03/30] Fix typo --- sdk/log/DESIGN.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 903f2663e6e..14fc01191a2 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -12,7 +12,7 @@ high-performant in this scenario. The prototype was created in [#4955](https://github.com/open-telemetry/opentelemetry-go/pull/4955). -## Module structure +## Modules structure The SDK is published as a single `go.opentelemetry.io/otel/sdk/log` Go module. From 6eff65a6081e12654446960813f290348bee257c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 6 Mar 2024 15:39:41 +0100 Subject: [PATCH 04/30] Update DESIGN.md --- sdk/log/DESIGN.md | 165 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 164 insertions(+), 1 deletion(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 14fc01191a2..9ede3be571c 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -27,12 +27,175 @@ The exporters are published as following Go modules: - `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp` - `go.opentelemetry.io/otel/exporters/stdout/stdoutlog` +## LoggerProvider + +The [LoggerProvider](https://opentelemetry.io/docs/specs/otel/logs/sdk/#loggerprovider) +is defined as follows: + +```go +type LoggerProvider struct { + embedded.LoggerProvider +} + +func NewLoggerProvider(...Option) *LoggerProvider + +func (*LoggerProvider) Logger(name string, options ...log.LoggerOption) log.Logger + +type Option interface { /* ... */ } + +func WithResource(*resource.Resource) Option +``` + +## LogRecord limits + +The [LogRecord limits](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecord-limits) +are defined as follows: + +```go +func WithLimits(Limits) Option + +type Limits struct { + // AttributeValueLengthLimit is the maximum allowed attribute value length. + // + // This limit only applies to string and string slice attribute values. + // Any string longer than this value will be truncated to this length. + // + // Setting this to zero means to use the default setting. + // See also AttributeValueLengthLimitZero. + // + // Setting this to a negative value means no limit is applied. + AttributeValueLengthLimit int + + // AttributeValueLengthLimitZero is to set a zero value of AttributeValueLengthLimit. + // Setting this means string and string slice attribute values will be empty. + // It overrides any value set via AttributeValueLengthLimit. + AttributeValueLengthLimitZero bool + + // AttributeCountLimit is the maximum allowed span attribute count. Any + // attribute added to a span once this limit is reached will be dropped. + // + // Setting this to zero means to use the default setting. + // See also AttributeCountLimitZero. + // + // Setting this to a negative value means no limit is applied. + AttributeCountLimit int + + // AttributeCountLimitZero is to set a zero value of AttributeCountLimit. + // Setting this means no attributes will be recorded. + // It overrides any value set via AttributeCountLimit. + AttributeCountLimitZero bool +} +``` + +### LogRecordProcessor and LogRecordExporter + +Both [LogRecordProcessor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordprocessor) +and [LogRecordExporter](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordexporter) +are defined via an `Exporter` interface: + +```go +func WithExporter(Exporter) Option { + return nil +} + +type Exporter interface { + Export(ctx context.Context, records []*Record) error + Shutdown(ctx context.Context) error + ForceFlush(ctx context.Context) error +} +``` + +The `Record` struct represents the [ReadWriteLogRecord](https://opentelemetry.io/docs/specs/otel/logs/sdk/#readwritelogrecord). + +```go +type Record struct { /* ... */ } + +func (r *Record) Timestamp() + +func (r *Record) SetTimestamp(t time.Time) + +func (r *Record) ObservedTimestamp() time.Time + +func (r *Record) SetObservedTimestamp(t time.Time) + +func (r *Record) Severity() log.Severity + +func (r *Record) SetSeverity(level log.Severity) + +func (r *Record) SeverityText() string + +func (r *Record) SetSeverityText(text string) + +func (r *Record) Body() log.Value + +func (r *Record) SetBody(v log.Value) + +func (r *Record) WalkAttributes(f func(log.KeyValue) bool) + +// SetAttributes sets (and overrides) attributes to the log record. +func (r *Record) SetAttributes(attrs ...log.KeyValue) + +func (r *Record) TraceID() trace.TraceID + +func (r *Record) SpanID() trace.SpanID + +func (r *Record) TraceFlags() trace.TraceFlags + +func (r *Record) Resource() resource.Resource + +func (r *Record) InstrumentationScope() instrumentation.Scope + +func (r *Record) AttributeValueLengthLimit() int + +func (r *Record) AttributeCountLimit() int +``` + +The user can implement a custom [LogRecordProcessor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordprocessor) +by implementing a `Exporter` decorator. + +[Simple processor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#simple-processor) +is a acheived by simply passing a bare-exporter. + +[Batching processor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#batching-processor) +is a acheived by wrapping an expoter with `Batcher`: + +```go +type Batcher struct { /* ... */ } + +func NewBatchingExporter(exporter Exporter, opts ...BatchingOption) *Batcher + +func (b *Batcher) Export(ctx context.Context, records []*Record) + +func (b *Batcher) Shutdown(ctx context.Context) error + +func (b *Batcher) ForceFlush(ctx context.Context) error + +type BatchingOption interface { /* ... */ } + +func WithInterval(d time.Duration) BatchingOption + +func WithTimeout(d time.Duration) BatchingOption + +func WithQueueSize(max int) BatchingOption + +func WithBatchSize(max int) BatchingOption +``` + +## Benchmarking + +The benchmarks are supposed to test end-to-end scenarios. + +However, in order avoid I/O that could affect the stability of the results, +the benchmarks are using an stdout exporter using `io.Discard`. + +The benchmark results can be found in [the prototype](https://github.com/open-telemetry/opentelemetry-go/pull/4955). + ## Rejected alternatives ## Open issues The Logs SDK NOT be released as stable before all issues below are closed: -- TBD +- [Fix what can be modified via ReadWriteLogRecord](https://github.com/open-telemetry/opentelemetry-specification/pull/3907) [^1]: [OpenTelemetry Logging](https://opentelemetry.io/docs/specs/otel/logs) From bc90093d4ff6eb3f84fb5dd7a7c748a07f515703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 6 Mar 2024 15:48:46 +0100 Subject: [PATCH 05/30] Update DESIGN.md --- sdk/log/DESIGN.md | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 9ede3be571c..474e4e231d9 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -16,12 +16,7 @@ The prototype was created in The SDK is published as a single `go.opentelemetry.io/otel/sdk/log` Go module. -The Go module consists of the following packages: - -- `go.opentelemetry.io/otel/sdk/log` -- `go.opentelemetry.io/otel/sdk/log/logtest` - -The exporters are published as following Go modules: +The exporters are going to be published as following Go modules: - `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc` - `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp` From af0e84826ab919bfcb92ced52573fbb2e8b2e6c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 6 Mar 2024 15:53:15 +0100 Subject: [PATCH 06/30] Fix typos --- sdk/log/DESIGN.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 474e4e231d9..e44254eea34 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -149,10 +149,10 @@ The user can implement a custom [LogRecordProcessor](https://opentelemetry.io/do by implementing a `Exporter` decorator. [Simple processor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#simple-processor) -is a acheived by simply passing a bare-exporter. +is a achieved by simply passing a bare-exporter. [Batching processor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#batching-processor) -is a acheived by wrapping an expoter with `Batcher`: +is a achieved by wrapping an expoter with `Batcher`: ```go type Batcher struct { /* ... */ } From 8b4a204bc90b795a33a2dd6b3bc6185af131dfd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 6 Mar 2024 16:02:19 +0100 Subject: [PATCH 07/30] Document design choice --- sdk/log/DESIGN.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index e44254eea34..67017616124 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -148,6 +148,9 @@ func (r *Record) AttributeCountLimit() int The user can implement a custom [LogRecordProcessor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordprocessor) by implementing a `Exporter` decorator. +This is similar to the design of HTTP server middleware +which is a wrapper of `http.Handler`.[^2] + [Simple processor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#simple-processor) is a achieved by simply passing a bare-exporter. @@ -194,3 +197,4 @@ The Logs SDK NOT be released as stable before all issues below are closed: - [Fix what can be modified via ReadWriteLogRecord](https://github.com/open-telemetry/opentelemetry-specification/pull/3907) [^1]: [OpenTelemetry Logging](https://opentelemetry.io/docs/specs/otel/logs) +[^2]: [Middleware - Go Web Examples](https://gowebexamples.com/basic-middleware/) From f1ec9aa374c1788e2108d6f4ce5264bc1fc17d41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 6 Mar 2024 16:21:58 +0100 Subject: [PATCH 08/30] Update DESIGN.md --- sdk/log/DESIGN.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 67017616124..bd1f6d0a27e 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -127,7 +127,7 @@ func (r *Record) SetBody(v log.Value) func (r *Record) WalkAttributes(f func(log.KeyValue) bool) -// SetAttributes sets (and overrides) attributes to the log record. +// SetAttributes sets and overrides the attributes of the log record. func (r *Record) SetAttributes(attrs ...log.KeyValue) func (r *Record) TraceID() trace.TraceID From 2e54ea5f55b2ab10316810d2b50b3d5946d0ecba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 6 Mar 2024 18:37:43 +0100 Subject: [PATCH 09/30] Update limits --- sdk/log/DESIGN.md | 36 +++--------------------------------- 1 file changed, 3 insertions(+), 33 deletions(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index bd1f6d0a27e..14adc9387ed 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -47,39 +47,9 @@ The [LogRecord limits](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrec are defined as follows: ```go -func WithLimits(Limits) Option - -type Limits struct { - // AttributeValueLengthLimit is the maximum allowed attribute value length. - // - // This limit only applies to string and string slice attribute values. - // Any string longer than this value will be truncated to this length. - // - // Setting this to zero means to use the default setting. - // See also AttributeValueLengthLimitZero. - // - // Setting this to a negative value means no limit is applied. - AttributeValueLengthLimit int - - // AttributeValueLengthLimitZero is to set a zero value of AttributeValueLengthLimit. - // Setting this means string and string slice attribute values will be empty. - // It overrides any value set via AttributeValueLengthLimit. - AttributeValueLengthLimitZero bool - - // AttributeCountLimit is the maximum allowed span attribute count. Any - // attribute added to a span once this limit is reached will be dropped. - // - // Setting this to zero means to use the default setting. - // See also AttributeCountLimitZero. - // - // Setting this to a negative value means no limit is applied. - AttributeCountLimit int - - // AttributeCountLimitZero is to set a zero value of AttributeCountLimit. - // Setting this means no attributes will be recorded. - // It overrides any value set via AttributeCountLimit. - AttributeCountLimitZero bool -} +func WithAttributeCountLimit(limit int) Option + +func WithAttributeValueLengthLimit(limit int) Option ``` ### LogRecordProcessor and LogRecordExporter From 6a85c02fc7b8a72743d4968cbb639266157984b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 6 Mar 2024 22:20:03 +0100 Subject: [PATCH 10/30] Apply suggestions from code review Co-authored-by: Tyler Yahn --- sdk/log/DESIGN.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 14adc9387ed..a577a3d7e63 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -122,10 +122,10 @@ This is similar to the design of HTTP server middleware which is a wrapper of `http.Handler`.[^2] [Simple processor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#simple-processor) -is a achieved by simply passing a bare-exporter. +is achieved by simply passing a bare-exporter. [Batching processor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#batching-processor) -is a achieved by wrapping an expoter with `Batcher`: +is a achieved by wrapping an exporter with `Batcher`: ```go type Batcher struct { /* ... */ } From 3cb2da401efab3d55089f870e6049f4fbc237ed7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Thu, 7 Mar 2024 08:55:31 +0100 Subject: [PATCH 11/30] Update DESIGN.md --- sdk/log/DESIGN.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index a577a3d7e63..308a272996a 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -140,13 +140,13 @@ func (b *Batcher) ForceFlush(ctx context.Context) error type BatchingOption interface { /* ... */ } -func WithInterval(d time.Duration) BatchingOption +func WithMaxQueueSize(max int) BatchingOption -func WithTimeout(d time.Duration) BatchingOption +func WithExportInterval(d time.Duration) BatchingOption -func WithQueueSize(max int) BatchingOption +func WithExportTimeout(d time.Duration) BatchingOption -func WithBatchSize(max int) BatchingOption +func WithExportMaxBatchSize(max int) BatchingOption ``` ## Benchmarking From 0a474d24d9c94283c72086dabc2cc8f4b27d5012 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Thu, 7 Mar 2024 09:09:08 +0100 Subject: [PATCH 12/30] Update DESIGN.md --- sdk/log/DESIGN.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 308a272996a..91356f6cdd7 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -44,7 +44,7 @@ func WithResource(*resource.Resource) Option ## LogRecord limits The [LogRecord limits](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecord-limits) -are defined as follows: +can be configured using following options: ```go func WithAttributeCountLimit(limit int) Option @@ -52,6 +52,11 @@ func WithAttributeCountLimit(limit int) Option func WithAttributeValueLengthLimit(limit int) Option ``` +The limits can be also configured using the `OTEL_LOGRECORD_*` environment variables as +[defined by the specification](https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#logrecord-limits). + +The options take precedence over environmental variables. + ### LogRecordProcessor and LogRecordExporter Both [LogRecordProcessor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordprocessor) @@ -149,6 +154,11 @@ func WithExportTimeout(d time.Duration) BatchingOption func WithExportMaxBatchSize(max int) BatchingOption ``` +The `Batcher` can be also configured using the `OTEL_BLRP_*` environment variables as +[defined by the specification](https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#batch-logrecord-processor). + +The options take precedence over environmental variables. + ## Benchmarking The benchmarks are supposed to test end-to-end scenarios. From d657c99b902a908291131326d6e1cc9f0dfb2db3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Thu, 7 Mar 2024 10:38:32 +0100 Subject: [PATCH 13/30] Update DESIGN.md --- sdk/log/DESIGN.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 91356f6cdd7..b52caf4a4ee 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -61,7 +61,7 @@ The options take precedence over environmental variables. Both [LogRecordProcessor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordprocessor) and [LogRecordExporter](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordexporter) -are defined via an `Exporter` interface: +are defined via an `Exporter` interface:[^2] ```go func WithExporter(Exporter) Option { @@ -124,7 +124,7 @@ The user can implement a custom [LogRecordProcessor](https://opentelemetry.io/do by implementing a `Exporter` decorator. This is similar to the design of HTTP server middleware -which is a wrapper of `http.Handler`.[^2] +which is a wrapper of `http.Handler`.[^3] [Simple processor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#simple-processor) is achieved by simply passing a bare-exporter. @@ -177,4 +177,5 @@ The Logs SDK NOT be released as stable before all issues below are closed: - [Fix what can be modified via ReadWriteLogRecord](https://github.com/open-telemetry/opentelemetry-specification/pull/3907) [^1]: [OpenTelemetry Logging](https://opentelemetry.io/docs/specs/otel/logs) -[^2]: [Middleware - Go Web Examples](https://gowebexamples.com/basic-middleware/) +[^2]: [Conversation on representing LogRecordProcessor and LogRecordExporter via a single Expoter interface](https://github.com/open-telemetry/opentelemetry-go/pull/4954#discussion_r1515050480) +[^3]: [Middleware - Go Web Examples](https://gowebexamples.com/basic-middleware/) From b24439c17ac0639020656424bb0284802dfeb339 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Thu, 7 Mar 2024 10:42:16 +0100 Subject: [PATCH 14/30] Update DESIGN.md --- sdk/log/DESIGN.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index b52caf4a4ee..852912b2b06 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -174,7 +174,9 @@ The benchmark results can be found in [the prototype](https://github.com/open-te The Logs SDK NOT be released as stable before all issues below are closed: +- [Redefine ReadableLogRecord and ReadWriteLogRecord](https://github.com/open-telemetry/opentelemetry-specification/pull/3898) - [Fix what can be modified via ReadWriteLogRecord](https://github.com/open-telemetry/opentelemetry-specification/pull/3907) +- [Add an Enabled method to Logger](https://github.com/open-telemetry/opentelemetry-specification/issues/3917) [^1]: [OpenTelemetry Logging](https://opentelemetry.io/docs/specs/otel/logs) [^2]: [Conversation on representing LogRecordProcessor and LogRecordExporter via a single Expoter interface](https://github.com/open-telemetry/opentelemetry-go/pull/4954#discussion_r1515050480) From 93826b77d8ed86218993f61e79de75f3be9e782a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Thu, 7 Mar 2024 10:44:17 +0100 Subject: [PATCH 15/30] Update DESIGN.md --- sdk/log/DESIGN.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 852912b2b06..00486d1c257 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -179,5 +179,5 @@ The Logs SDK NOT be released as stable before all issues below are closed: - [Add an Enabled method to Logger](https://github.com/open-telemetry/opentelemetry-specification/issues/3917) [^1]: [OpenTelemetry Logging](https://opentelemetry.io/docs/specs/otel/logs) -[^2]: [Conversation on representing LogRecordProcessor and LogRecordExporter via a single Expoter interface](https://github.com/open-telemetry/opentelemetry-go/pull/4954#discussion_r1515050480) +[^2]: [Conversation on representing LogRecordProcessor and LogRecordExporter via a single Expoter interface](https://github.com/open-telemetry/opentelemetry-go/pull/4954#discussion_r1515050480) [^3]: [Middleware - Go Web Examples](https://gowebexamples.com/basic-middleware/) From 7c8e4c5b70213ceac6c7c84c59d42690b0537209 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Thu, 7 Mar 2024 17:40:52 +0100 Subject: [PATCH 16/30] Update sdk/log/DESIGN.md Co-authored-by: David Ashpole --- sdk/log/DESIGN.md | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 00486d1c257..31528a18902 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -34,6 +34,7 @@ type LoggerProvider struct { func NewLoggerProvider(...Option) *LoggerProvider +// Logger implements the log.LoggerProvider interface. func (*LoggerProvider) Logger(name string, options ...log.LoggerOption) log.Logger type Option interface { /* ... */ } From 8ba8991379955ffd14c65cf06decd5078286989a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 8 Mar 2024 09:28:52 +0100 Subject: [PATCH 17/30] Update DESIGN.md --- sdk/log/DESIGN.md | 46 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 31528a18902..362290376b1 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -65,13 +65,45 @@ and [LogRecordExporter](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logre are defined via an `Exporter` interface:[^2] ```go -func WithExporter(Exporter) Option { - return nil -} +func WithExporter(Exporter) Option +// Exporter handles the delivery of log records to external receivers. +// +// Any of the Exporter's methods may be called concurrently with itself +// or with other methods. It is the responsibility of the Exporter to manage +// this concurrency. type Exporter interface { + // Export transmits log records to a receiver. + // + // The deadline or cancellation of the passed context must be honored. An + // appropriate error should be returned in these situations. + // + // All retry logic must be contained in this function. The SDK does not + // implement any retry logic. All errors returned by this function are + // considered unrecoverable and will be reported to a configured error + // Handler. + // + // Implementations must not retain the records slice. + // + // Implementations should consider cloning the records before modifying + // them to avoid possible data races. Export(ctx context.Context, records []*Record) error + + // Shutdown is called when the SDK shuts down. Any cleanup or release of + // resources held by the exporter should be done in this call. + // + // The deadline or cancellation of the passed context must be honored. An + // appropriate error should be returned in these situations. + // + // After Shutdown is called, calls to Export, Shutdown, or ForceFlush + // should perform no operation and return nil error. Shutdown(ctx context.Context) error + + // ForceFlush exports log records to the configured Exporter that have not yet + // been exported. + // + // The deadline or cancellation of the passed context must be honored. An + // appropriate error should be returned in these situations. ForceFlush(ctx context.Context) error } ``` @@ -119,8 +151,16 @@ func (r *Record) InstrumentationScope() instrumentation.Scope func (r *Record) AttributeValueLengthLimit() int func (r *Record) AttributeCountLimit() int + +func (r *Record) Clone() *Record ``` +The slice passed to `Export` must not be retained by the implementation +(like e.g. [`io.Writer`](https://pkg.go.dev/io#Writer)) +so that the caller can reuse the passed slice +(e.g. using [`sync.Pool`](https://pkg.go.dev/sync#Pool)) +to avoid heap allocations on each call. + The user can implement a custom [LogRecordProcessor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordprocessor) by implementing a `Exporter` decorator. From c4b798b438bec797ec6a18da075f2fd89eacd604 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 8 Mar 2024 10:06:02 +0100 Subject: [PATCH 18/30] Add Record.AddAttributes --- sdk/log/DESIGN.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 362290376b1..58d7182e3a3 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -135,6 +135,8 @@ func (r *Record) SetBody(v log.Value) func (r *Record) WalkAttributes(f func(log.KeyValue) bool) +func (r *Record) AddAttributes(attrs ...log.KeyValue) + // SetAttributes sets and overrides the attributes of the log record. func (r *Record) SetAttributes(attrs ...log.KeyValue) From 3a8106806b5f59cd53c9f4ff37095a6271ee6967 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 8 Mar 2024 16:27:47 +0100 Subject: [PATCH 19/30] Update DESIGN.md --- sdk/log/DESIGN.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 58d7182e3a3..b677fa165e6 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -85,9 +85,9 @@ type Exporter interface { // // Implementations must not retain the records slice. // - // Implementations should consider cloning the records before modifying + // Implementations should clone the records before modifying // them to avoid possible data races. - Export(ctx context.Context, records []*Record) error + Export(ctx context.Context, records []Record) error // Shutdown is called when the SDK shuts down. Any cleanup or release of // resources held by the exporter should be done in this call. @@ -154,7 +154,7 @@ func (r *Record) AttributeValueLengthLimit() int func (r *Record) AttributeCountLimit() int -func (r *Record) Clone() *Record +func (r *Record) Clone() Record ``` The slice passed to `Export` must not be retained by the implementation @@ -180,7 +180,7 @@ type Batcher struct { /* ... */ } func NewBatchingExporter(exporter Exporter, opts ...BatchingOption) *Batcher -func (b *Batcher) Export(ctx context.Context, records []*Record) +func (b *Batcher) Export(ctx context.Context, records []Record) func (b *Batcher) Shutdown(ctx context.Context) error From 7d68029d70131433fd0ced8834a9f5ed310c25f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 8 Mar 2024 17:12:26 +0100 Subject: [PATCH 20/30] Update DESIGN.md --- sdk/log/DESIGN.md | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index b677fa165e6..bbdb36bab1f 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -219,6 +219,7 @@ The Logs SDK NOT be released as stable before all issues below are closed: - [Redefine ReadableLogRecord and ReadWriteLogRecord](https://github.com/open-telemetry/opentelemetry-specification/pull/3898) - [Fix what can be modified via ReadWriteLogRecord](https://github.com/open-telemetry/opentelemetry-specification/pull/3907) +- [logs: Allow duplicate keys](https://github.com/open-telemetry/opentelemetry-specification/issues/3931) - [Add an Enabled method to Logger](https://github.com/open-telemetry/opentelemetry-specification/issues/3917) [^1]: [OpenTelemetry Logging](https://opentelemetry.io/docs/specs/otel/logs) From f480907f5e067feab4b48b83f9b9465401f93187 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Mon, 11 Mar 2024 12:05:57 +0100 Subject: [PATCH 21/30] Add Processor --- sdk/log/DESIGN.md | 297 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 235 insertions(+), 62 deletions(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index bbdb36bab1f..c19caa6249b 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -32,14 +32,30 @@ type LoggerProvider struct { embedded.LoggerProvider } -func NewLoggerProvider(...Option) *LoggerProvider +// NewLoggerProvider returns a new and configured LoggerProvider. +// +// By default, the returned LoggerProvider is configured with the default +// Resource and no Processors. Processors cannot be added after a LoggerProvider is +// created. This means the returned LoggerProvider, one created with no +// Processors, will perform no operations. +func NewLoggerProvider(...LoggerProviderOption) *LoggerProvider +// Logger returns a new log.Logger with the provided name and configuration. +// +// This method can be called concurrently. +// // Logger implements the log.LoggerProvider interface. func (*LoggerProvider) Logger(name string, options ...log.LoggerOption) log.Logger -type Option interface { /* ... */ } +type LoggerProviderOption interface { /* ... */ } -func WithResource(*resource.Resource) Option +// WithResource associates a Resource with a LoggerProvider. This Resource +// represents the entity producing telemetry and is associated with all Loggers +// the LoggerProvider will create. +// +// By default, if this Option is not used, the default Resource from the +// go.opentelemetry.io/otel/sdk/resource package will be used. +func WithResource(*resource.Resource) LoggerProviderOption ``` ## LogRecord limits @@ -48,25 +64,184 @@ The [LogRecord limits](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrec can be configured using following options: ```go -func WithAttributeCountLimit(limit int) Option +// WithAttributeCountLimit sets the maximum allowed log record attribute count. +// Any attribute added to a log record once this limit is reached will be dropped. +// +// Setting this to zero means no attributes will be recorded. +// +// Setting this to a negative value means no limit is applied. +// +// If the OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT environment variable is set, +// and this option is not passed, that variable value will be used. +// If both are set, OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT will take precedence. +// +// By default, if an environment variable is not set, and this option is not +// passed, no limit 128 will be used. +func WithAttributeCountLimit(limit int) LoggerProviderOption -func WithAttributeValueLengthLimit(limit int) Option +// AttributeValueLengthLimit sets the maximum allowed attribute value length. +// +// This limit only applies to string and string slice attribute values. +// Any string longer than this value will be truncated to this length. +// +// Setting this to a negative value means no limit is applied. +// +// If the OTEL_LOGRECORD_ATTRIBUTE_VALUE_LENGTH_LIMIT environment variable is set, +// and this option is not passed, that variable value will be used. +// If both are set, OTEL_LOGRECORD_ATTRIBUTE_VALUE_LENGTH_LIMIT will take precedence. +// +// By default, if an environment variable is not set, and this option is not +// passed, no limit (-1) will be used. +func WithAttributeValueLengthLimit(limit int) LoggerProviderOption ``` The limits can be also configured using the `OTEL_LOGRECORD_*` environment variables as [defined by the specification](https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#logrecord-limits). -The options take precedence over environmental variables. +### Processor + +The [LogRecordProcessor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordprocessor) +is defined as follows: -### LogRecordProcessor and LogRecordExporter +```go +// WithProcessor associates Processor with a LoggerProvider. +// +// By default, if this option is not used, the LoggerProvider will perform no +// operations; no data will be exported without a processor. +// +// Each WithProcessor creates a separate pipeline. Use custom decorators +// for advanced scenarios such as enriching with attributes. +// +// Use NewBatchingProcessor to batch log records before they are exported. +// Use NewSimpleProcessor to synchronously export log records. +func WithProcessor(processor Processor) LoggerProviderOption -Both [LogRecordProcessor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordprocessor) -and [LogRecordExporter](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordexporter) -are defined via an `Exporter` interface:[^2] +// Processor handles the processing of log records. +// +// Any of the Exporter's methods may be called concurrently with itself +// or with other methods. It is the responsibility of the Exporter to manage +// this concurrency. +type Processor interface { + // OnEmit is called when a Record is emitted. + // + // The deadline or cancellation of the passed context must be honored. An + // appropriate error should be returned in these situations. + // + // All retry logic must be contained in this function. The SDK does not + // implement any retry logic. All errors returned by this function are + // considered unrecoverable and will be reported to a configured error + // Handler. + // + // Before modifying a Record, the implementation must use Record.Clone + // to create a copy that shares no state with the original. + OnEmit(ctx context.Context, record Record) error + + // Shutdown is called when the SDK shuts down. Any cleanup or release of + // resources held by the exporter should be done in this call. + // + // The deadline or cancellation of the passed context must be honored. An + // appropriate error should be returned in these situations. + // + // After Shutdown is called, calls to Export, Shutdown, or ForceFlush + // should perform no operation and return nil error. + Shutdown(ctx context.Context) error + + // ForceFlush exports log records to the configured Exporter that have not yet + // been exported. + // + // The deadline or cancellation of the passed context must be honored. An + // appropriate error should be returned in these situations. + ForceFlush(ctx context.Context) error +} +``` + +The user can configure custom processors and decorate built-in processors. + +### SimpleProcessor + +The [Simple processor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#simple-processor) +is defined as follows: + +```go +// SimpleProcessor implements Processor. +type SimpleProcessor struct { /* ... */ } + +// NewBatchingProcessor decorates the provided exporter +// so that the log records are batched before exporting. +func NewSimpleProcessor(exporter Exporter) *SimpleProcessor +``` + +### BatchingProcessor + +The [Batching processor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#batching-processor) +is defined as follows: ```go -func WithExporter(Exporter) Option +// BatchingProcessor implements Processor. +type BatchingProcessor struct { /* ... */ } + +// NewBatchingProcessor decorates the provided exporter +// so that the log records are batched before exporting. +func NewBatchingProcessor(exporter Exporter, opts ...BatchingOption) *BatchingProcessor + +// BatchingOption applies a configuration to a Batcher. +type BatchingOption interface { /* ... */ } +// WithMaxQueueSize sets the maximum queue size used by the Batcher. +// After the size is reached log records are dropped. +// +// If the OTEL_BLRP_MAX_QUEUE_SIZE environment variable is set, +// and this option is not passed, that variable value will be used. +// If both are set, OTEL_BLRP_MAX_QUEUE_SIZE will take precedence. +// +// By default, if an environment variable is not set, and this option is not +// passed, 2048 will be used. +// The default value is also used when the provided value is not a positive value. +func WithMaxQueueSize(max int) BatchingOption + +// WithExportInterval sets the maximum duration between batched exports. +// +// If the OTEL_BSP_SCHEDULE_DELAY environment variable is set, +// and this option is not passed, that variable value will be used. +// If both are set, OTEL_BSP_SCHEDULE_DELAY will take precedence. +// +// By default, if an environment variable is not set, and this option is not +// passed, 1s will be used. +// The default value is also used when the provided value is not a positive value. +func WithExportInterval(d time.Duration) BatchingOption + +// WithExportTimeout sets the duration after which a batched export is canceled. +// +// If the OTEL_BSP_EXPORT_TIMEOUT environment variable is set, +// and this option is not passed, that variable value will be used. +// If both are set, OTEL_BSP_EXPORT_TIMEOUT will take precedence. +// +// By default, if an environment variable is not set, and this option is not +// passed, 30s will be used. +// The default value is also used when the provided value is not a positive value. +func WithExportTimeout(d time.Duration) BatchingOption + +// WithExportMaxBatchSize sets the maximum batch size of every export. +// +// If the OTEL_BSP_MAX_EXPORT_BATCH_SIZE environment variable is set, +// and this option is not passed, that variable value will be used. +// If both are set, OTEL_BSP_MAX_EXPORT_BATCH_SIZE will take precedence. +// +// By default, if an environment variable is not set, and this option is not +// passed, 512 will be used. +// The default value is also used when the provided value is not a positive value. +func WithExportMaxBatchSize(max int) BatchingOption +``` + +The `Batcher` can be also configured using the `OTEL_BLRP_*` environment variables as +[defined by the specification](https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#batch-logrecord-processor). + +### Exporter + +The [LogRecordExporter](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordexporter) +is defined as follows: + +```go // Exporter handles the delivery of log records to external receivers. // // Any of the Exporter's methods may be called concurrently with itself @@ -85,8 +260,8 @@ type Exporter interface { // // Implementations must not retain the records slice. // - // Implementations should clone the records before modifying - // them to avoid possible data races. + // Before modifying a Record, the implementation must use Record.Clone + // to create a copy that shares no state with the original. Export(ctx context.Context, records []Record) error // Shutdown is called when the SDK shuts down. Any cleanup or release of @@ -108,7 +283,16 @@ type Exporter interface { } ``` -The `Record` struct represents the [ReadWriteLogRecord](https://opentelemetry.io/docs/specs/otel/logs/sdk/#readwritelogrecord). +The slice passed to `Export` must not be retained by the implementation +(like e.g. [`io.Writer`](https://pkg.go.dev/io#Writer)) +so that the caller can reuse the passed slice +(e.g. using [`sync.Pool`](https://pkg.go.dev/sync#Pool)) +to avoid heap allocations on each call. + +### Record + +The [ReadWriteLogRecord](https://opentelemetry.io/docs/specs/otel/logs/sdk/#readwritelogrecord) +is defined as follows: ```go type Record struct { /* ... */ } @@ -146,73 +330,62 @@ func (r *Record) SpanID() trace.SpanID func (r *Record) TraceFlags() trace.TraceFlags +// Resource returns the entity that collected the log. func (r *Record) Resource() resource.Resource +// InstrumentationScope returns the scope that the Logger was created with. func (r *Record) InstrumentationScope() instrumentation.Scope +// AttributeValueLengthLimit is the maximum allowed attribute value length. +// +// This limit only applies to string and string slice attribute values. +// Any string longer than this value should be truncated to this length. +// +// Negative value means no limit should be applied. func (r *Record) AttributeValueLengthLimit() int +// AttributeCountLimit is the maximum allowed log record attribute count. Any +// attribute added to a log record once this limit is reached should be dropped. +// +// Zero means no attributes should be recorded. +// +// Negative value means no limit should be applied. func (r *Record) AttributeCountLimit() int +// Clone returns a copy of the record with no shared state. The original record +// and the clone can both be modified without interfering with each other. func (r *Record) Clone() Record ``` -The slice passed to `Export` must not be retained by the implementation -(like e.g. [`io.Writer`](https://pkg.go.dev/io#Writer)) -so that the caller can reuse the passed slice -(e.g. using [`sync.Pool`](https://pkg.go.dev/sync#Pool)) -to avoid heap allocations on each call. +The `Record` is designed similarly to [`log.Record`](https://pkg.go.dev/go.opentelemetry.io/otel/log#Record) +in order to reduce the number of heap allocations when processing attributes. -The user can implement a custom [LogRecordProcessor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordprocessor) -by implementing a `Exporter` decorator. - -This is similar to the design of HTTP server middleware -which is a wrapper of `http.Handler`.[^3] - -[Simple processor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#simple-processor) -is achieved by simply passing a bare-exporter. - -[Batching processor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#batching-processor) -is a achieved by wrapping an exporter with `Batcher`: - -```go -type Batcher struct { /* ... */ } - -func NewBatchingExporter(exporter Exporter, opts ...BatchingOption) *Batcher - -func (b *Batcher) Export(ctx context.Context, records []Record) - -func (b *Batcher) Shutdown(ctx context.Context) error - -func (b *Batcher) ForceFlush(ctx context.Context) error - -type BatchingOption interface { /* ... */ } - -func WithMaxQueueSize(max int) BatchingOption - -func WithExportInterval(d time.Duration) BatchingOption - -func WithExportTimeout(d time.Duration) BatchingOption - -func WithExportMaxBatchSize(max int) BatchingOption -``` - -The `Batcher` can be also configured using the `OTEL_BLRP_*` environment variables as -[defined by the specification](https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#batch-logrecord-processor). - -The options take precedence over environmental variables. +The SDK does have not have an additional definition of +[ReadableLogRecord](https://opentelemetry.io/docs/specs/otel/logs/sdk/#readablelogrecord) +as the specification does not say that the exporters must not be able to modify +the log records. It simply requires them to be able to read the log records. +Having less abstractions reduces the API surface and makes the design simpler. ## Benchmarking -The benchmarks are supposed to test end-to-end scenarios. - -However, in order avoid I/O that could affect the stability of the results, -the benchmarks are using an stdout exporter using `io.Discard`. +The benchmarks are supposed to test end-to-end scenarios +and avoid I/O that could affect the stability of the results, The benchmark results can be found in [the prototype](https://github.com/open-telemetry/opentelemetry-go/pull/4955). ## Rejected alternatives +### Represent both LogRecordProcessor and LogRecordExporter as Expoter + +Becuase the [LogRecordProcessor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordprocessor) +and the [LogRecordProcessor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordexporter) +abstractions are so similar there was a proposal to unify them under +single `Expoter` interface.[^2] + +However, introducing a `Processor` interface makes it easier +to create custom processor decorators[^3] +and makes the design more aligned with the specifiation. + ## Open issues The Logs SDK NOT be released as stable before all issues below are closed: @@ -224,4 +397,4 @@ The Logs SDK NOT be released as stable before all issues below are closed: [^1]: [OpenTelemetry Logging](https://opentelemetry.io/docs/specs/otel/logs) [^2]: [Conversation on representing LogRecordProcessor and LogRecordExporter via a single Expoter interface](https://github.com/open-telemetry/opentelemetry-go/pull/4954#discussion_r1515050480) -[^3]: [Middleware - Go Web Examples](https://gowebexamples.com/basic-middleware/) +[^3]: [Introduce Processor](https://github.com/pellared/opentelemetry-go/pull/9) From b1c39b51d804a873aa3591f3e9dcba1a2c69b0b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Mon, 11 Mar 2024 12:09:39 +0100 Subject: [PATCH 22/30] Fix typo --- sdk/log/DESIGN.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index c19caa6249b..d7dc7735f0d 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -377,7 +377,7 @@ The benchmark results can be found in [the prototype](https://github.com/open-te ### Represent both LogRecordProcessor and LogRecordExporter as Expoter -Becuase the [LogRecordProcessor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordprocessor) +Because the [LogRecordProcessor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordprocessor) and the [LogRecordProcessor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordexporter) abstractions are so similar there was a proposal to unify them under single `Expoter` interface.[^2] From 42eedc10ab7c8d38647b6317ad84b769209328cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Mon, 11 Mar 2024 12:19:22 +0100 Subject: [PATCH 23/30] Rejected proposal: Embedd log.Record --- sdk/log/DESIGN.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index d7dc7735f0d..bb61c52432d 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -379,13 +379,26 @@ The benchmark results can be found in [the prototype](https://github.com/open-te Because the [LogRecordProcessor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordprocessor) and the [LogRecordProcessor](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordexporter) -abstractions are so similar there was a proposal to unify them under +abstractions are so similar, there was a proposal to unify them under single `Expoter` interface.[^2] However, introducing a `Processor` interface makes it easier to create custom processor decorators[^3] and makes the design more aligned with the specifiation. +### Embedd log.Record + +Because [`Record`](#record) and [`log.Record`](https://pkg.go.dev/go.opentelemetry.io/otel/log#Record) +are very similar, there was a proposal to embedd `log.Record` in `Record` definition. + +[`log.Record`](https://pkg.go.dev/go.opentelemetry.io/otel/log#Record) +supports only adding attributes. +In the SDK, we also need to be able to modify the attributes (e.g. removal) +provided via API. + +Moreover it is safer to have these abstraction decoupled. +E.g. there can be a need for some fields that can be set via API and cannot be modified by the processors. + ## Open issues The Logs SDK NOT be released as stable before all issues below are closed: From 1bac33107c921e236246eb1f6e945c6091c8826c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Mon, 11 Mar 2024 13:25:20 +0100 Subject: [PATCH 24/30] Update DESIGN.md --- sdk/log/DESIGN.md | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index bb61c52432d..0fb87922925 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -166,8 +166,11 @@ is defined as follows: // SimpleProcessor implements Processor. type SimpleProcessor struct { /* ... */ } -// NewBatchingProcessor decorates the provided exporter -// so that the log records are batched before exporting. +// NewSimpleProcessor is a simple Processor adapter. +// +// Any of the Exporter's methods may be called concurrently with itself +// or with other methods. It is the responsibility of the Exporter to manage +// this concurrency. func NewSimpleProcessor(exporter Exporter) *SimpleProcessor ``` @@ -182,6 +185,10 @@ type BatchingProcessor struct { /* ... */ } // NewBatchingProcessor decorates the provided exporter // so that the log records are batched before exporting. +// +// All of the Exporter's methods are called from a single dedicated +// background goroutine. Therefore the Expoter does not need to +// be concurrent safe. func NewBatchingProcessor(exporter Exporter, opts ...BatchingOption) *BatchingProcessor // BatchingOption applies a configuration to a Batcher. @@ -243,10 +250,6 @@ is defined as follows: ```go // Exporter handles the delivery of log records to external receivers. -// -// Any of the Exporter's methods may be called concurrently with itself -// or with other methods. It is the responsibility of the Exporter to manage -// this concurrency. type Exporter interface { // Export transmits log records to a receiver. // From ab9e3ea45073246a39d6238ce15cb60a7b435fde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Mon, 11 Mar 2024 13:27:10 +0100 Subject: [PATCH 25/30] Update DESIGN.md --- sdk/log/DESIGN.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 0fb87922925..e1adb966ecf 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -187,7 +187,7 @@ type BatchingProcessor struct { /* ... */ } // so that the log records are batched before exporting. // // All of the Exporter's methods are called from a single dedicated -// background goroutine. Therefore the Expoter does not need to +// background goroutine. Therefore, the Expoter does not need to // be concurrent safe. func NewBatchingProcessor(exporter Exporter, opts ...BatchingOption) *BatchingProcessor From 86e63927e010d62a9c98c87b47ffb02aa04d35db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Mon, 11 Mar 2024 16:31:48 +0100 Subject: [PATCH 26/30] Update sdk/log/DESIGN.md Co-authored-by: Tyler Yahn --- sdk/log/DESIGN.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index e1adb966ecf..09115ac1844 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -363,7 +363,7 @@ func (r *Record) Clone() Record The `Record` is designed similarly to [`log.Record`](https://pkg.go.dev/go.opentelemetry.io/otel/log#Record) in order to reduce the number of heap allocations when processing attributes. -The SDK does have not have an additional definition of +The SDK does not have have an additional definition of [ReadableLogRecord](https://opentelemetry.io/docs/specs/otel/logs/sdk/#readablelogrecord) as the specification does not say that the exporters must not be able to modify the log records. It simply requires them to be able to read the log records. From 0d69655420f3c9983adfb3af9b8903f49bb5a200 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 12 Mar 2024 08:04:55 +0100 Subject: [PATCH 27/30] Apply suggestions from code review Co-authored-by: Sam Xie Co-authored-by: David Ashpole --- sdk/log/DESIGN.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 09115ac1844..05e47905f73 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -38,7 +38,7 @@ type LoggerProvider struct { // Resource and no Processors. Processors cannot be added after a LoggerProvider is // created. This means the returned LoggerProvider, one created with no // Processors, will perform no operations. -func NewLoggerProvider(...LoggerProviderOption) *LoggerProvider +func NewLoggerProvider(opts ...LoggerProviderOption) *LoggerProvider // Logger returns a new log.Logger with the provided name and configuration. // @@ -55,7 +55,7 @@ type LoggerProviderOption interface { /* ... */ } // // By default, if this Option is not used, the default Resource from the // go.opentelemetry.io/otel/sdk/resource package will be used. -func WithResource(*resource.Resource) LoggerProviderOption +func WithResource(res *resource.Resource) LoggerProviderOption ``` ## LogRecord limits @@ -118,8 +118,8 @@ func WithProcessor(processor Processor) LoggerProviderOption // Processor handles the processing of log records. // -// Any of the Exporter's methods may be called concurrently with itself -// or with other methods. It is the responsibility of the Exporter to manage +// Any of the Processor's methods may be called concurrently with itself +// or with other methods. It is the responsibility of the Processor to manage // this concurrency. type Processor interface { // OnEmit is called when a Record is emitted. From 2c8a57c968e631e6cbd3818e6d966f3f45e55f73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 12 Mar 2024 08:16:10 +0100 Subject: [PATCH 28/30] Update DESIGN.md --- sdk/log/DESIGN.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 05e47905f73..1bd10425927 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -168,8 +168,8 @@ type SimpleProcessor struct { /* ... */ } // NewSimpleProcessor is a simple Processor adapter. // -// Any of the Exporter's methods may be called concurrently with itself -// or with other methods. It is the responsibility of the Exporter to manage +// Any of the exporter's methods may be called concurrently with itself +// or with other methods. It is the responsibility of the exporter to manage // this concurrency. func NewSimpleProcessor(exporter Exporter) *SimpleProcessor ``` @@ -186,8 +186,8 @@ type BatchingProcessor struct { /* ... */ } // NewBatchingProcessor decorates the provided exporter // so that the log records are batched before exporting. // -// All of the Exporter's methods are called from a single dedicated -// background goroutine. Therefore, the Expoter does not need to +// All of the exporter's methods are called from a single dedicated +// background goroutine. Therefore, the expoter does not need to // be concurrent safe. func NewBatchingProcessor(exporter Exporter, opts ...BatchingOption) *BatchingProcessor From 716e7806f20429f085f048af6c1d3228ff02e1d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 12 Mar 2024 08:19:57 +0100 Subject: [PATCH 29/30] Update DESIGN.md --- sdk/log/DESIGN.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 1bd10425927..8ba2b4150c6 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -124,8 +124,8 @@ func WithProcessor(processor Processor) LoggerProviderOption type Processor interface { // OnEmit is called when a Record is emitted. // - // The deadline or cancellation of the passed context must be honored. An - // appropriate error should be returned in these situations. + // Implementation should not interrupt the record processing + // if the context is canceled. // // All retry logic must be contained in this function. The SDK does not // implement any retry logic. All errors returned by this function are From d96dbbbd64d4be28ed8868a07ddd018b8b2668a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 12 Mar 2024 15:54:40 +0100 Subject: [PATCH 30/30] Add SetTraceID SetSpanID SetTraceFlags to Record --- sdk/log/DESIGN.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 8ba2b4150c6..521788cc407 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -329,10 +329,16 @@ func (r *Record) SetAttributes(attrs ...log.KeyValue) func (r *Record) TraceID() trace.TraceID +func (r *Record) SetTraceID(id trace.TraceID) + func (r *Record) SpanID() trace.SpanID +func (r *Record) SetSpanID(id trace.SpanID) + func (r *Record) TraceFlags() trace.TraceFlags +func (r *Record) SetTraceFlags(flags trace.TraceFlags) + // Resource returns the entity that collected the log. func (r *Record) Resource() resource.Resource