From f079b0336474db4e793e4cc351ebba65c44eecc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Sun, 4 Aug 2024 12:35:58 +0200 Subject: [PATCH] sdk/log: SimpleProcessor to not panic for zero value (#5665) --- CHANGELOG.md | 2 ++ sdk/log/simple.go | 22 ++++++++++++++++------ sdk/log/simple_test.go | 15 ++++++++++++++- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8ebb918ae2..2ca16223768 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,10 +16,12 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm This module is unstable and breaking changes may be introduced. See our [versioning policy](VERSIONING.md) for more information about these stability guarantees. (#5629) - Add `InstrumentationScope` field to `SpanStub` in `go.opentelemetry.io/otel/sdk/trace/tracetest`, as a replacement for the deprecated `InstrumentationLibrary`. (#5627) +- Zero value of `SimpleProcessor` in `go.opentelemetry.io/otel/sdk/log` no longer panics. (#5665) ### Changed - `Processor.OnEmit` in `go.opentelemetry.io/otel/sdk/log` now accepts a pointer to `Record` instead of a value so that the record modifications done in a processor are propagated to subsequent registered processors. (#5636) +- `SimpleProcessor.Enabled` in `go.opentelemetry.io/otel/sdk/log` now returns `false` if the exporter is `nil`. (#5665) ### Fixed diff --git a/sdk/log/simple.go b/sdk/log/simple.go index 8b6c34dc080..cf7c315a62d 100644 --- a/sdk/log/simple.go +++ b/sdk/log/simple.go @@ -12,6 +12,8 @@ import ( var _ Processor = (*SimpleProcessor)(nil) // SimpleProcessor is an processor that synchronously exports log records. +// +// Use [NewSimpleProcessor] to create a SimpleProcessor. type SimpleProcessor struct { exporter Exporter } @@ -25,10 +27,6 @@ type SimpleProcessor struct { // [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. - exporter = defaultNoopExporter - } return &SimpleProcessor{exporter: exporter} } @@ -41,6 +39,10 @@ var simpleProcRecordsPool = sync.Pool{ // OnEmit batches provided log record. func (s *SimpleProcessor) OnEmit(ctx context.Context, r *Record) error { + if s.exporter == nil { + return nil + } + records := simpleProcRecordsPool.Get().(*[]Record) (*records)[0] = *r defer func() { @@ -50,18 +52,26 @@ func (s *SimpleProcessor) OnEmit(ctx context.Context, r *Record) error { return s.exporter.Export(ctx, *records) } -// Enabled returns true. +// Enabled returns true if the exporter is not nil. func (s *SimpleProcessor) Enabled(context.Context, Record) bool { - return true + return s.exporter != nil } // Shutdown shuts down the expoter. func (s *SimpleProcessor) Shutdown(ctx context.Context) error { + if s.exporter == nil { + return nil + } + return s.exporter.Shutdown(ctx) } // ForceFlush flushes the exporter. func (s *SimpleProcessor) ForceFlush(ctx context.Context) error { + if s.exporter == nil { + return nil + } + return s.exporter.ForceFlush(ctx) } diff --git a/sdk/log/simple_test.go b/sdk/log/simple_test.go index dbc91a90156..b20f2071e1b 100644 --- a/sdk/log/simple_test.go +++ b/sdk/log/simple_test.go @@ -51,7 +51,8 @@ func TestSimpleProcessorOnEmit(t *testing.T) { } func TestSimpleProcessorEnabled(t *testing.T) { - s := log.NewSimpleProcessor(nil) + e := new(exporter) + s := log.NewSimpleProcessor(e) assert.True(t, s.Enabled(context.Background(), log.Record{})) } @@ -69,6 +70,18 @@ func TestSimpleProcessorForceFlush(t *testing.T) { require.True(t, e.forceFlushCalled, "exporter ForceFlush not called") } +func TestSimpleProcessorEmpty(t *testing.T) { + assert.NotPanics(t, func() { + var s log.SimpleProcessor + ctx := context.Background() + record := new(log.Record) + assert.NoError(t, s.OnEmit(ctx, record), "OnEmit") + assert.False(t, s.Enabled(ctx, *record), "Enabled") + assert.NoError(t, s.ForceFlush(ctx), "ForceFlush") + assert.NoError(t, s.Shutdown(ctx), "Shutdown") + }) +} + func TestSimpleProcessorConcurrentSafe(t *testing.T) { const goRoutineN = 10