diff --git a/CHANGELOG.md b/CHANGELOG.md index 831ed541f81..ae562cbd80f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,6 +96,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm The information that could be configured in the `Process` struct should be configured in a `Resource` instead. (#1776, #1804) - Remove the `WithDisabled` option from the Jaeger exporter. To disable the exporter unregister it from the `TracerProvider` or use a no-operation `TracerProvider`. (#1806) +- Removed the Jaeger exporter `WithSDKOptions` `Option`. + This option was used to set SDK options for the exporter creation convenience functions. + These functions are provided as a way to easily setup or install the exporter with what are deemed reasonable SDK settings for common use cases. + If the SDK needs to be configured differently, the `NewRawExporter` function and direct setup of the SDK with the desired settings should be used. (#1825) ## [0.19.0] - 2021-03-18 diff --git a/example/jaeger/main.go b/example/jaeger/main.go index e41d1b398c4..f018852db11 100644 --- a/example/jaeger/main.go +++ b/example/jaeger/main.go @@ -19,41 +19,71 @@ package main import ( "context" "log" + "time" "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/sdk/resource" - "go.opentelemetry.io/otel/semconv" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/exporters/trace/jaeger" - sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/resource" + tracesdk "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/semconv" ) -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()), - sdktrace.WithResource(resource.NewWithAttributes( - semconv.ServiceNameKey.String("trace-demo"), - attribute.String("exporter", "jaeger"), - attribute.Float64("float", 312.23), - )), - ), +const ( + service = "trace-demo" + environment = "production" + id = 1 +) + +// tracerProvider returns an OpenTelemetry TracerProvider configured to use +// the Jaeger exporter that will send spans to the provided url. The returned +// TracerProvider will also use a Resource configured with all the information +// about the application. +func tracerProvider(url string) (*tracesdk.TracerProvider, error) { + // Create the Jaeger exporter + exp, err := jaeger.NewRawExporter(jaeger.WithCollectorEndpoint(url)) + if err != nil { + return nil, err + } + tp := tracesdk.NewTracerProvider( + // Always be sure to batch in production. + tracesdk.WithBatcher(exp), + // Record information about this application in an Resource. + tracesdk.WithResource(resource.NewWithAttributes( + semconv.ServiceNameKey.String(service), + attribute.String("environment", environment), + attribute.Int64("ID", id), + )), ) + return tp, nil +} + +func main() { + tp, err := tracerProvider("http://localhost:14268/api/traces") if err != nil { log.Fatal(err) } - defer func() { - if err := tp.Shutdown(context.Background()); err != nil { + + // Register our TracerProvider as the global so any imported + // instrumentation in the future will default to using it. + otel.SetTracerProvider(tp) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Cleanly shutdown and flush telemetry when the application exits. + defer func(ctx context.Context) { + // Do not make the application hang when it is shutdown. + ctx, cancel = context.WithTimeout(ctx, time.Second*5) + defer cancel() + if err := tp.Shutdown(ctx); err != nil { log.Fatal(err) } - }() + }(ctx) tr := tp.Tracer("component-main") - ctx := context.Background() ctx, span := tr.Start(ctx, "foo") defer span.End() diff --git a/exporters/trace/jaeger/jaeger.go b/exporters/trace/jaeger/jaeger.go index d7a9ea53c55..6f918b3250c 100644 --- a/exporters/trace/jaeger/jaeger.go +++ b/exporters/trace/jaeger/jaeger.go @@ -53,9 +53,6 @@ type options struct { // BatchMaxCount defines the maximum number of spans sent in one batch BatchMaxCount int - - // TracerProviderOptions defines the options for tracer provider of sdk. - TracerProviderOptions []sdktrace.TracerProviderOption } // WithBufferMaxCount defines the total number of traces that can be buffered in memory @@ -72,13 +69,6 @@ func WithBatchMaxCount(batchMaxCount int) Option { } } -// WithSDKOptions configures options for tracer provider of sdk. -func WithSDKOptions(opts ...sdktrace.TracerProviderOption) Option { - return func(o *options) { - o.TracerProviderOptions = opts - } -} - // NewRawExporter returns an OTel Exporter implementation that exports the // collected spans to Jaeger. func NewRawExporter(endpointOption EndpointOption, opts ...Option) (*Exporter, error) { @@ -133,18 +123,17 @@ 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) (*sdktrace.TracerProvider, error) { - o := options{} - for _, opt := range opts { - opt(&o) - } - exporter, err := NewRawExporter(endpointOption, opts...) if err != nil { return nil, err } - pOpts := append(o.TracerProviderOptions, sdktrace.WithSyncer(exporter)) - tp := sdktrace.NewTracerProvider(pOpts...) + // TODO (MrAlias): The recommended default setup needs to register the + // exporter as a batcher, not a syncer. This will conflict with batching + // of the bundler currently, but when the bundler is removed it needs to + // be updated. + // https://github.com/open-telemetry/opentelemetry-go/issues/1799 + tp := sdktrace.NewTracerProvider(sdktrace.WithSyncer(exporter)) return tp, nil } diff --git a/exporters/trace/jaeger/jaeger_test.go b/exporters/trace/jaeger/jaeger_test.go index c9651ec82e2..612eecc9066 100644 --- a/exporters/trace/jaeger/jaeger_test.go +++ b/exporters/trace/jaeger/jaeger_test.go @@ -17,6 +17,7 @@ package jaeger import ( "context" "encoding/binary" + "errors" "fmt" "os" "sort" @@ -54,25 +55,35 @@ func TestInstallNewPipeline(t *testing.T) { assert.IsType(t, tp, otel.GetTracerProvider()) } -func TestNewRawExporterOptions(t *testing.T) { - testCases := []struct { - name string - endpoint EndpointOption +func TestNewExportPipelinePassthroughError(t *testing.T) { + for _, testcase := range []struct { + name string + failing bool + epo EndpointOption }{ { - name: "default exporter with collector endpoint", - endpoint: WithCollectorEndpoint(collectorEndpoint), + name: "failing underlying NewRawExporter", + failing: true, + epo: func() (batchUploader, error) { + return nil, errors.New("error") + }, }, { - name: "default exporter with agent endpoint", - endpoint: WithAgentEndpoint(), + name: "with default agent endpoint", + epo: WithAgentEndpoint(), }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - _, err := NewRawExporter(tc.endpoint) - assert.NoError(t, err) + { + name: "with collector endpoint", + epo: WithCollectorEndpoint(collectorEndpoint), + }, + } { + t.Run(testcase.name, func(t *testing.T) { + _, err := NewExportPipeline(testcase.epo) + if testcase.failing { + require.Error(t, err) + return + } + require.NoError(t, err) }) } } @@ -202,17 +213,18 @@ func TestExporterExportSpan(t *testing.T) { ) testCollector := &testCollectorEndpoint{} - tp, err := NewExportPipeline( - withTestCollectorEndpointInjected(testCollector), - WithSDKOptions( - sdktrace.WithResource(resource.NewWithAttributes( - semconv.ServiceNameKey.String(serviceName), - attribute.String(tagKey, tagVal), - )), - ), - ) + exp, err := NewRawExporter(withTestCollectorEndpointInjected(testCollector)) require.NoError(t, err) + tp := sdktrace.NewTracerProvider( + sdktrace.WithSyncer(exp), + sdktrace.WithResource(resource.NewWithAttributes( + semconv.ServiceNameKey.String(serviceName), + attribute.String(tagKey, tagVal), + )), + ) + tracer := tp.Tracer("test-tracer") + ctx := context.Background() for i := 0; i < 3; i++ { _, span := tracer.Start(ctx, fmt.Sprintf("test-span-%d", i)) @@ -221,6 +233,7 @@ func TestExporterExportSpan(t *testing.T) { } require.NoError(t, tp.Shutdown(ctx)) + batchesUploaded := testCollector.batchesUploaded require.Len(t, batchesUploaded, 1) uploadedBatch := batchesUploaded[0] @@ -823,49 +836,3 @@ func TestProcess(t *testing.T) { }) } } - -func TestNewExporterPipelineWithOptions(t *testing.T) { - const ( - serviceName = "test-service" - eventCountLimit = 10 - tagKey = "key" - tagVal = "val" - ) - - testCollector := &testCollectorEndpoint{} - tp, err := NewExportPipeline( - withTestCollectorEndpointInjected(testCollector), - WithSDKOptions( - sdktrace.WithResource(resource.NewWithAttributes( - semconv.ServiceNameKey.String(serviceName), - attribute.String(tagKey, tagVal), - )), - sdktrace.WithSpanLimits(sdktrace.SpanLimits{ - EventCountLimit: eventCountLimit, - }), - ), - ) - require.NoError(t, err) - - 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() - require.NoError(t, tp.Shutdown(ctx)) - - assert.True(t, span.SpanContext().IsValid()) - - batchesUploaded := testCollector.batchesUploaded - require.Len(t, batchesUploaded, 1) - uploadedBatch := batchesUploaded[0] - assert.Equal(t, serviceName, uploadedBatch.GetProcess().GetServiceName()) - require.Len(t, uploadedBatch.GetSpans(), 1) - uploadedSpan := uploadedBatch.GetSpans()[0] - assert.Len(t, uploadedSpan.GetLogs(), eventCountLimit) - - require.Len(t, uploadedBatch.GetProcess().GetTags(), 1) - assert.Equal(t, tagKey, uploadedBatch.GetProcess().GetTags()[0].GetKey()) - assert.Equal(t, tagVal, uploadedBatch.GetProcess().GetTags()[0].GetVStr()) -}