From 6082e830c280f443627ca7f738072602e732a554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 11 Jun 2024 07:28:56 -0700 Subject: [PATCH] sdk/log: Remove slice allocation from SimpleProcessor.OnEmit (#5493) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reason for this improvement (apart that, in general, it is good to have better performance) is there may be good use case to use a `SimpleProcessor` to emit logs efficiently to standard output. It could be one of the most efficient solutions (from application performance perspective) and thanks to such configuration the user would not lose any logs if the application suddenly crashes. For instance, a useful configuration could be a simple processor with an OTLP file exporter (https://github.com/open-telemetry/opentelemetry-go/issues/5408). I think we might consider changing the following portion of `SimpleProcessor` documentation (but I would prefer to do it as separate PR): > // 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. ``` goos: linux goarch: amd64 pkg: go.opentelemetry.io/otel/sdk/log cpu: Intel(R) Core(TM) i9-10885H CPU @ 2.40GHz │ old.txt │ new.txt │ │ sec/op │ sec/op vs base │ Processor/Simple-16 449.4n ± 7% 156.2n ± 5% -65.25% (p=0.000 n=10) Processor/ModifyTimestampSimple-16 468.0n ± 6% 171.3n ± 15% -63.40% (p=0.000 n=10) Processor/ModifyAttributesSimple-16 515.8n ± 3% 233.2n ± 8% -54.77% (p=0.000 n=10) geomean 476.9n 184.1n -61.40% │ old.txt │ new.txt │ │ B/op │ B/op vs base │ Processor/Simple-16 417.0 ± 0% 0.0 ± 0% -100.00% (p=0.000 n=10) Processor/ModifyTimestampSimple-16 417.0 ± 0% 0.0 ± 0% -100.00% (p=0.000 n=10) Processor/ModifyAttributesSimple-16 465.00 ± 0% 48.00 ± 0% -89.68% (p=0.000 n=10) geomean 432.4 ? ¹ ² ¹ summaries must be >0 to compute geomean ² ratios must be >0 to compute geomean │ old.txt │ new.txt │ │ allocs/op │ allocs/op vs base │ Processor/Simple-16 1.000 ± 0% 0.000 ± 0% -100.00% (p=0.000 n=10) Processor/ModifyTimestampSimple-16 1.000 ± 0% 0.000 ± 0% -100.00% (p=0.000 n=10) Processor/ModifyAttributesSimple-16 2.000 ± 0% 1.000 ± 0% -50.00% (p=0.000 n=10) geomean 1.260 ? ¹ ² ¹ summaries must be >0 to compute geomean ² ratios must be >0 to compute geomean ``` --------- Co-authored-by: Tyler Yahn --- CHANGELOG.md | 1 + sdk/log/simple.go | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f28a537f062..6e8f2a1c5d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `IsEmpty` method is added to the `Instrument` type in `go.opentelemetry.io/otel/sdk/metric`. This method is used to check if an `Instrument` instance is a zero-value. (#5431) - Store and provide the emitted `context.Context` in `ScopeRecords` of `go.opentelemetry.io/otel/sdk/log/logtest`. (#5468) +- `SimpleProcessor.OnEmit` in `go.opentelemetry.io/otel/sdk/log` no longer allocates a slice which makes it possible to have a zero-allocation log processing using `SimpleProcessor`. (#5493) ### Changed diff --git a/sdk/log/simple.go b/sdk/log/simple.go index c7aa14b8706..fc5690b22d5 100644 --- a/sdk/log/simple.go +++ b/sdk/log/simple.go @@ -5,6 +5,7 @@ package log // import "go.opentelemetry.io/otel/sdk/log" import ( "context" + "sync" ) // Compile-time check SimpleProcessor implements Processor. @@ -30,9 +31,22 @@ func NewSimpleProcessor(exporter Exporter, _ ...SimpleProcessorOption) *SimplePr return &SimpleProcessor{exporter: exporter} } +var simpleProcRecordsPool = sync.Pool{ + New: func() any { + records := make([]Record, 1) + return &records + }, +} + // OnEmit batches provided log record. func (s *SimpleProcessor) OnEmit(ctx context.Context, r Record) error { - return s.exporter.Export(ctx, []Record{r}) + records := simpleProcRecordsPool.Get().(*[]Record) + (*records)[0] = r + defer func() { + simpleProcRecordsPool.Put(records) + }() + + return s.exporter.Export(ctx, *records) } // Enabled returns true.