From d595dab76fd30a34ef8c739effc7c845c24dd21f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Thu, 11 Jul 2024 20:06:19 +0200 Subject: [PATCH] sdk/log: Simple processor may be useful for production (#5578) Update the design doc: - describe that minimizing the amount of the heap allocation is one of the design goals - provide more usage examples Moreover update the docs to not discourage using a simple processor from production. It would be good if the user would use it e.g. with an [user_events](https://docs.kernel.org/trace/user_events.html), [LLTng](https://lttng.org/docs/v2.13/#doc-tracing-your-own-user-application) or [ETW](https://learn.microsoft.com/en-us/windows/win32/etw/about-event-tracing) exporter. However, we recommend using a batch processor when using OTLP over HTTP or gRPC exporters. --- .../otlp/otlplog/otlploggrpc/exporter.go | 3 ++ .../otlp/otlplog/otlploghttp/exporter.go | 3 ++ sdk/log/DESIGN.md | 34 ++++++++++++++----- sdk/log/simple.go | 11 +++--- 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/exporters/otlp/otlplog/otlploggrpc/exporter.go b/exporters/otlp/otlplog/otlploggrpc/exporter.go index 9eaac5deea9..dcd6b9ecf9f 100644 --- a/exporters/otlp/otlplog/otlploggrpc/exporter.go +++ b/exporters/otlp/otlplog/otlploggrpc/exporter.go @@ -32,6 +32,9 @@ type Exporter struct { var _ log.Exporter = (*Exporter)(nil) // New returns a new [Exporter]. +// +// It is recommended to use it with a [BatchProcessor] +// or other processor exporting records asynchronously. func New(_ context.Context, options ...Option) (*Exporter, error) { cfg := newConfig(options) c, err := newClient(cfg) diff --git a/exporters/otlp/otlplog/otlploghttp/exporter.go b/exporters/otlp/otlplog/otlploghttp/exporter.go index deee36c001c..f1c8d3ae0a7 100644 --- a/exporters/otlp/otlplog/otlploghttp/exporter.go +++ b/exporters/otlp/otlplog/otlploghttp/exporter.go @@ -23,6 +23,9 @@ type Exporter struct { var _ log.Exporter = (*Exporter)(nil) // New returns a new [Exporter]. +// +// It is recommended to use it with a [BatchProcessor] +// or other processor exporting records asynchronously. func New(_ context.Context, options ...Option) (*Exporter, error) { cfg := newConfig(options) c, err := newHTTPClient(cfg) diff --git a/sdk/log/DESIGN.md b/sdk/log/DESIGN.md index 6d73150db5d..cda06511203 100644 --- a/sdk/log/DESIGN.md +++ b/sdk/log/DESIGN.md @@ -5,13 +5,28 @@ `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 [#4955](https://github.com/open-telemetry/opentelemetry-go/pull/4955). +## Background + +The goal is to design the exported API of the SDK would have low performance +overhead. Most importantly, have a design that reduces the amount of heap +allocations and even make it possible to have a zero-allocation implementation. +Eliminating the amount of heap allocations reduces the GC pressure which can +produce some of the largest improvements in performance.[^1] + +The main and recommended use case is to configure the SDK to use an OTLP +exporter with a batch processor.[^2] Therefore, the implementation aims to be +high-performant in this scenario. Some users that require high throughput may +also want to use e.g. an [user_events](https://docs.kernel.org/trace/user_events.html), +[LLTng](https://lttng.org/docs/v2.13/#doc-tracing-your-own-user-application) +or [ETW](https://learn.microsoft.com/en-us/windows/win32/etw/about-event-tracing) +exporter with a simple processor. Users may also want to use +[OTLP File](https://opentelemetry.io/docs/specs/otel/protocol/file-exporter/) +or [Standard Output](https://opentelemetry.io/docs/specs/otel/logs/sdk_exporters/stdout/) +exporter in order to emit logs to standard output/error or files. + ## Modules structure The SDK is published as a single `go.opentelemetry.io/otel/sdk/log` Go module. @@ -112,10 +127,10 @@ 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 -single `Expoter` interface.[^2] +single `Expoter` interface.[^3] However, introducing a `Processor` interface makes it easier -to create custom processor decorators[^3] +to create custom processor decorators[^4] and makes the design more aligned with the specification. ### Embed log.Record @@ -131,6 +146,7 @@ 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. -[^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]: [Introduce Processor](https://github.com/pellared/opentelemetry-go/pull/9) +[^1]: [A Guide to the Go Garbage Collector](https://tip.golang.org/doc/gc-guide) +[^2]: [OpenTelemetry Logging](https://opentelemetry.io/docs/specs/otel/logs) +[^3]: [Conversation on representing LogRecordProcessor and LogRecordExporter via a single Expoter interface](https://github.com/open-telemetry/opentelemetry-go/pull/4954#discussion_r1515050480) +[^4]: [Introduce Processor](https://github.com/pellared/opentelemetry-go/pull/9) diff --git a/sdk/log/simple.go b/sdk/log/simple.go index fc5690b22d5..4d2bb412045 100644 --- a/sdk/log/simple.go +++ b/sdk/log/simple.go @@ -18,11 +18,12 @@ type SimpleProcessor struct { // NewSimpleProcessor is a simple Processor adapter. // -// This Processor is not recommended for production use. The synchronous -// nature of this Processor make it good for testing, debugging, or -// showing examples of other features, but it can be slow and have a high -// computation resource usage overhead. [NewBatchProcessor] is recommended -// for production use instead. +// This Processor is not recommended for production use due to its synchronous +// nature, which makes it suitable for testing, debugging, or demonstrating +// other features, but can lead to slow performance and high computational +// overhead. For production environments, it is recommended to use +// [NewBatchProcessor] instead. However, there may be exceptions where certain +// [Exporter] implementations perform better with this Processor. func NewSimpleProcessor(exporter Exporter, _ ...SimpleProcessorOption) *SimpleProcessor { if exporter == nil { // Do not panic on nil exporter.