Skip to content

Commit

Permalink
sdk/log: SimpleProcessor to not panic for zero value (open-telemetry#…
Browse files Browse the repository at this point in the history
  • Loading branch information
pellared authored Aug 4, 2024
1 parent f7977e0 commit f079b03
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 7 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
22 changes: 16 additions & 6 deletions sdk/log/simple.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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}
}

Expand All @@ -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() {
Expand All @@ -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)
}

Expand Down
15 changes: 14 additions & 1 deletion sdk/log/simple_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}))
}

Expand All @@ -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

Expand Down

0 comments on commit f079b03

Please sign in to comment.