diff --git a/CHANGELOG.md b/CHANGELOG.md index e730195f060..2709723be98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,7 +46,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm Additionally, this tag is overridden, as specified in the OTel specification, if the event contains an attribute with that key. (#1768) - Zipkin Exporter: Ensure mapping between OTel and Zipkin span data complies with the specification. (#1688) - Fixed typo for default service name in Jaeger Exporter. (#1797) -- Fix flaky OTLP for the reconnnection of the client connection. (#1527, TBD) +- Fix flaky OTLP for the reconnnection of the client connection. (#1527, #1814) ### Changed @@ -72,7 +72,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Make `ExportSpans` in Jaeger Exporter honor context deadline. (#1773) - The `go.opentelemetry.io/otel/sdk/export/trace` package is merged into the `go.opentelemetry.io/otel/sdk/trace` package. (#1778) - The prometheus.InstallNewPipeline example is moved from comment to example test (#1796) -- Convenience functions for stdout exporter have been updated to return the `TracerProvider` implementation and enable the shutdown of the exporter. (#1800) +- The convenience functions for the stdout exporter have been updated to return the `TracerProvider` implementation and enable the shutdown of the exporter. (#1800) +- Replace the flush function returned from the Jaeger exporter's convenience creation functions (`InstallNewPipeline` and `NewExportPipeline`) with the `TracerProvider` implementation they create. + This enables the caller to shutdown and flush using the related `TracerProvider` methods. (#1822) ### Removed diff --git a/example/jaeger/main.go b/example/jaeger/main.go index 832f7ac4f06..e41d1b398c4 100644 --- a/example/jaeger/main.go +++ b/example/jaeger/main.go @@ -29,10 +29,9 @@ import ( sdktrace "go.opentelemetry.io/otel/sdk/trace" ) -// initTracer creates a new trace provider instance and registers it as global trace provider. -func initTracer() func() { - // Create and install Jaeger export pipeline. - flush, err := jaeger.InstallNewPipeline( +func main() { + // Create and install Jaeger export pipeline as the global. + tp, err := jaeger.InstallNewPipeline( jaeger.WithCollectorEndpoint("http://localhost:14268/api/traces"), jaeger.WithSDKOptions( sdktrace.WithSampler(sdktrace.AlwaysSample()), @@ -46,16 +45,15 @@ func initTracer() func() { if err != nil { log.Fatal(err) } - return flush -} - -func main() { - ctx := context.Background() + defer func() { + if err := tp.Shutdown(context.Background()); err != nil { + log.Fatal(err) + } + }() - flush := initTracer() - defer flush() + tr := tp.Tracer("component-main") - tr := otel.Tracer("component-main") + ctx := context.Background() ctx, span := tr.Start(ctx, "foo") defer span.End() @@ -63,6 +61,7 @@ func main() { } func bar(ctx context.Context) { + // Use the global TracerProvider. tr := otel.Tracer("component-bar") _, span := tr.Start(ctx, "bar") span.SetAttributes(attribute.Key("testset").String("value")) diff --git a/exporters/trace/jaeger/jaeger.go b/exporters/trace/jaeger/jaeger.go index a66511bae21..d7a9ea53c55 100644 --- a/exporters/trace/jaeger/jaeger.go +++ b/exporters/trace/jaeger/jaeger.go @@ -132,7 +132,7 @@ func NewRawExporter(endpointOption EndpointOption, opts ...Option) (*Exporter, e // NewExportPipeline sets up a complete export pipeline // with the recommended setup for trace provider -func NewExportPipeline(endpointOption EndpointOption, opts ...Option) (trace.TracerProvider, func(), error) { +func NewExportPipeline(endpointOption EndpointOption, opts ...Option) (*sdktrace.TracerProvider, error) { o := options{} for _, opt := range opts { opt(&o) @@ -140,24 +140,24 @@ func NewExportPipeline(endpointOption EndpointOption, opts ...Option) (trace.Tra exporter, err := NewRawExporter(endpointOption, opts...) if err != nil { - return nil, nil, err + return nil, err } pOpts := append(o.TracerProviderOptions, sdktrace.WithSyncer(exporter)) tp := sdktrace.NewTracerProvider(pOpts...) - return tp, exporter.Flush, nil + return tp, nil } // InstallNewPipeline instantiates a NewExportPipeline with the // recommended configuration and registers it globally. -func InstallNewPipeline(endpointOption EndpointOption, opts ...Option) (func(), error) { - tp, flushFn, err := NewExportPipeline(endpointOption, opts...) +func InstallNewPipeline(endpointOption EndpointOption, opts ...Option) (*sdktrace.TracerProvider, error) { + tp, err := NewExportPipeline(endpointOption, opts...) if err != nil { - return nil, err + return tp, err } otel.SetTracerProvider(tp) - return flushFn, nil + return tp, nil } // Exporter is an implementation of an OTel SpanSyncer that uploads spans to diff --git a/exporters/trace/jaeger/jaeger_test.go b/exporters/trace/jaeger/jaeger_test.go index 8b1814baa14..c9651ec82e2 100644 --- a/exporters/trace/jaeger/jaeger_test.go +++ b/exporters/trace/jaeger/jaeger_test.go @@ -46,93 +46,33 @@ const ( ) func TestInstallNewPipeline(t *testing.T) { - testCases := []struct { - name string - endpoint EndpointOption - options []Option - expectedProvider trace.TracerProvider - }{ - { - name: "simple pipeline", - endpoint: WithCollectorEndpoint(collectorEndpoint), - expectedProvider: &sdktrace.TracerProvider{}, - }, - { - name: "with agent endpoint", - endpoint: WithAgentEndpoint(), - expectedProvider: &sdktrace.TracerProvider{}, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - fn, err := InstallNewPipeline( - tc.endpoint, - tc.options..., - ) - defer fn() - - assert.NoError(t, err) - assert.IsType(t, tc.expectedProvider, otel.GetTracerProvider()) - - otel.SetTracerProvider(nil) - }) - } + tp, err := InstallNewPipeline(WithCollectorEndpoint(collectorEndpoint)) + require.NoError(t, err) + // Ensure InstallNewPipeline sets the global TracerProvider. By default + // the global tracer provider will be a NoOp implementation, this checks + // if that has been overwritten. + assert.IsType(t, tp, otel.GetTracerProvider()) } -func TestNewExportPipeline(t *testing.T) { +func TestNewRawExporterOptions(t *testing.T) { testCases := []struct { - name string - endpoint EndpointOption - options []Option - expectedProviderType trace.TracerProvider - testSpanSampling, spanShouldBeSampled bool + name string + endpoint EndpointOption }{ { - name: "simple pipeline", - endpoint: WithCollectorEndpoint(collectorEndpoint), - expectedProviderType: &sdktrace.TracerProvider{}, - }, - { - name: "always on", + name: "default exporter with collector endpoint", endpoint: WithCollectorEndpoint(collectorEndpoint), - options: []Option{ - WithSDKOptions(sdktrace.WithSampler(sdktrace.AlwaysSample())), - }, - expectedProviderType: &sdktrace.TracerProvider{}, - testSpanSampling: true, - spanShouldBeSampled: true, }, { - name: "never", - endpoint: WithCollectorEndpoint(collectorEndpoint), - options: []Option{ - WithSDKOptions(sdktrace.WithSampler(sdktrace.NeverSample())), - }, - expectedProviderType: &sdktrace.TracerProvider{}, - testSpanSampling: true, - spanShouldBeSampled: false, + name: "default exporter with agent endpoint", + endpoint: WithAgentEndpoint(), }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - tp, fn, err := NewExportPipeline( - tc.endpoint, - tc.options..., - ) - defer fn() - + _, err := NewRawExporter(tc.endpoint) assert.NoError(t, err) - assert.NotEqual(t, tp, otel.GetTracerProvider()) - assert.IsType(t, tc.expectedProviderType, tp) - - if tc.testSpanSampling { - _, span := tp.Tracer("jaeger test").Start(context.Background(), tc.name) - spanCtx := span.SpanContext() - assert.Equal(t, tc.spanShouldBeSampled, spanCtx.IsSampled()) - span.End() - } }) } } @@ -254,7 +194,7 @@ func withTestCollectorEndpointInjected(ce *testCollectorEndpoint) func() (batchU } } -func TestExporter_ExportSpan(t *testing.T) { +func TestExporterExportSpan(t *testing.T) { const ( serviceName = "test-service" tagKey = "key" @@ -262,7 +202,7 @@ func TestExporter_ExportSpan(t *testing.T) { ) testCollector := &testCollectorEndpoint{} - tp, spanFlush, err := NewExportPipeline( + tp, err := NewExportPipeline( withTestCollectorEndpointInjected(testCollector), WithSDKOptions( sdktrace.WithResource(resource.NewWithAttributes( @@ -271,16 +211,16 @@ func TestExporter_ExportSpan(t *testing.T) { )), ), ) - assert.NoError(t, err) - otel.SetTracerProvider(tp) - tracer := otel.Tracer("test-tracer") + require.NoError(t, err) + tracer := tp.Tracer("test-tracer") + ctx := context.Background() for i := 0; i < 3; i++ { - _, span := tracer.Start(context.Background(), fmt.Sprintf("test-span-%d", i)) + _, span := tracer.Start(ctx, fmt.Sprintf("test-span-%d", i)) span.End() assert.True(t, span.SpanContext().IsValid()) } - spanFlush() + require.NoError(t, tp.Shutdown(ctx)) batchesUploaded := testCollector.batchesUploaded require.Len(t, batchesUploaded, 1) uploadedBatch := batchesUploaded[0] @@ -893,7 +833,7 @@ func TestNewExporterPipelineWithOptions(t *testing.T) { ) testCollector := &testCollectorEndpoint{} - tp, spanFlush, err := NewExportPipeline( + tp, err := NewExportPipeline( withTestCollectorEndpointInjected(testCollector), WithSDKOptions( sdktrace.WithResource(resource.NewWithAttributes( @@ -905,15 +845,15 @@ func TestNewExporterPipelineWithOptions(t *testing.T) { }), ), ) - assert.NoError(t, err) + require.NoError(t, err) - otel.SetTracerProvider(tp) - _, span := otel.Tracer("test-tracer").Start(context.Background(), "test-span") + ctx := context.Background() + _, span := tp.Tracer("test-tracer").Start(ctx, "test-span") for i := 0; i < eventCountLimit*2; i++ { span.AddEvent(fmt.Sprintf("event-%d", i)) } span.End() - spanFlush() + require.NoError(t, tp.Shutdown(ctx)) assert.True(t, span.SpanContext().IsValid())