Skip to content

Commit

Permalink
Shutdown underlying span exporter while shutting down BatchSpanProces…
Browse files Browse the repository at this point in the history
…sor (open-telemetry#1443)

* Fix BatchSpanProcessor does not shutdown underlying span exporter

* Update CHANGELOG

* Fix tests

* Update CHANGELOG.md
  • Loading branch information
XSAM authored Jan 14, 2021
1 parent dfece3d commit c29c6fd
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 6 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Remove Metric export functionality related to quantiles and summary data points: this is not specified (#1412)
- Remove DDSketch metric aggregator; our intention is to re-introduce this as an option of the histogram aggregator after [new OTLP histogram data types](https://github.com/open-telemetry/opentelemetry-proto/pull/226) are released (#1412)

### Fixed

- `BatchSpanProcessor.Shutdown()` will now shutdown underlying `export.SpanExporter`. (#1443)

## [0.15.0] - 2020-12-10

### Added
Expand Down
5 changes: 5 additions & 0 deletions sdk/trace/batch_span_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,11 @@ func (bsp *BatchSpanProcessor) Shutdown(ctx context.Context) error {
go func() {
close(bsp.stopCh)
bsp.stopWait.Wait()
if bsp.e != nil {
if err := bsp.e.Shutdown(ctx); err != nil {
otel.Handle(err)
}
}
close(wait)
}()
// Wait until the wait group is done or the context is cancelled
Expand Down
21 changes: 15 additions & 6 deletions sdk/trace/batch_span_processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,20 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"

"go.opentelemetry.io/otel/trace"

export "go.opentelemetry.io/otel/sdk/export/trace"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
)

type testBatchExporter struct {
mu sync.Mutex
spans []*export.SpanSnapshot
sizes []int
batchCount int
mu sync.Mutex
spans []*export.SpanSnapshot
sizes []int
batchCount int
shutdownCount int
}

func (t *testBatchExporter) ExportSpans(ctx context.Context, ss []*export.SpanSnapshot) error {
Expand All @@ -44,7 +47,10 @@ func (t *testBatchExporter) ExportSpans(ctx context.Context, ss []*export.SpanSn
return nil
}

func (t *testBatchExporter) Shutdown(context.Context) error { return nil }
func (t *testBatchExporter) Shutdown(context.Context) error {
t.shutdownCount++
return nil
}

func (t *testBatchExporter) len() int {
t.mu.Lock()
Expand Down Expand Up @@ -231,16 +237,19 @@ func getSpanContext() trace.SpanContext {
}

func TestBatchSpanProcessorShutdown(t *testing.T) {
bsp := sdktrace.NewBatchSpanProcessor(&testBatchExporter{})
var bp testBatchExporter
bsp := sdktrace.NewBatchSpanProcessor(&bp)

err := bsp.Shutdown(context.Background())
if err != nil {
t.Error("Error shutting the BatchSpanProcessor down\n")
}
assert.Equal(t, 1, bp.shutdownCount, "shutdown from span exporter not called")

// Multiple call to Shutdown() should not panic.
err = bsp.Shutdown(context.Background())
if err != nil {
t.Error("Error shutting the BatchSpanProcessor down\n")
}
assert.Equal(t, 1, bp.shutdownCount)
}

0 comments on commit c29c6fd

Please sign in to comment.