diff --git a/CHANGELOG.md b/CHANGELOG.md index 3db209a8f49..70b5097705a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added - Add the new `go.opentelemetry.io/contrib/instrgen` package to provide auto-generated source code instrumentation. (#3068) +- The `WithPublicEndpoint` and `WithPublicEndpointFn` options in `go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful`. (#3563) ## [1.16.0-rc.1/0.41.0-rc.1/0.9.0-rc.1] - 2023-03-02 diff --git a/instrumentation/github.com/emicklei/go-restful/otelrestful/config.go b/instrumentation/github.com/emicklei/go-restful/otelrestful/config.go index 11ef952c451..2451d2d3872 100644 --- a/instrumentation/github.com/emicklei/go-restful/otelrestful/config.go +++ b/instrumentation/github.com/emicklei/go-restful/otelrestful/config.go @@ -15,14 +15,18 @@ package otelrestful // import "go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful" import ( + "net/http" + "go.opentelemetry.io/otel/propagation" oteltrace "go.opentelemetry.io/otel/trace" ) // config is used to configure the go-restful middleware. type config struct { - TracerProvider oteltrace.TracerProvider - Propagators propagation.TextMapPropagator + TracerProvider oteltrace.TracerProvider + Propagators propagation.TextMapPropagator + PublicEndpoint bool + PublicEndpointFn func(*http.Request) bool } // Option applies a configuration value. @@ -36,6 +40,15 @@ func (o optionFunc) apply(c *config) { o(c) } +// WithPublicEndpoint configures the Handler to link the span with an incoming +// span context. If this option is not provided, then the association is a child +// association instead of a link. +func WithPublicEndpoint() Option { + return optionFunc(func(c *config) { + c.PublicEndpoint = true + }) +} + // WithPropagators specifies propagators to use for extracting // information from the HTTP requests. If none are specified, global // ones will be used. @@ -56,3 +69,14 @@ func WithTracerProvider(provider oteltrace.TracerProvider) Option { } }) } + +// WithPublicEndpointFn runs with every request, and allows conditionnally +// configuring the Handler to link the span with an incoming span context. If +// this option is not provided or returns false, then the association is a +// child association instead of a link. +// Note: [WithPublicEndpoint] takes precedence over WithPublicEndpointFn. +func WithPublicEndpointFn(fn func(*http.Request) bool) Option { + return optionFunc(func(c *config) { + c.PublicEndpointFn = fn + }) +} diff --git a/instrumentation/github.com/emicklei/go-restful/otelrestful/go.mod b/instrumentation/github.com/emicklei/go-restful/otelrestful/go.mod index 472a4e725f8..1590597d028 100644 --- a/instrumentation/github.com/emicklei/go-restful/otelrestful/go.mod +++ b/instrumentation/github.com/emicklei/go-restful/otelrestful/go.mod @@ -9,6 +9,7 @@ require ( github.com/stretchr/testify v1.8.2 go.opentelemetry.io/contrib/propagators/b3 v1.16.0-rc.1 go.opentelemetry.io/otel v1.15.0-rc.1 + go.opentelemetry.io/otel/sdk v1.14.0 go.opentelemetry.io/otel/trace v1.15.0-rc.1 ) @@ -21,5 +22,6 @@ require ( github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect go.opentelemetry.io/otel/metric v1.15.0-rc.1 // indirect + golang.org/x/sys v0.5.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/instrumentation/github.com/emicklei/go-restful/otelrestful/go.sum b/instrumentation/github.com/emicklei/go-restful/otelrestful/go.sum index 53ce3732904..39d2d164d1d 100644 --- a/instrumentation/github.com/emicklei/go-restful/otelrestful/go.sum +++ b/instrumentation/github.com/emicklei/go-restful/otelrestful/go.sum @@ -30,8 +30,12 @@ go.opentelemetry.io/otel v1.15.0-rc.1 h1:KgZyVIfe3rPjWZHAZE0A9sH5U4tjyh1VeP+BFIg go.opentelemetry.io/otel v1.15.0-rc.1/go.mod h1:IZXh/uN07z/0si8lWvFW2FkwzAmSGE4DhF4quJIsLnY= go.opentelemetry.io/otel/metric v1.15.0-rc.1 h1:ueivGgoyP2c58JZvmJriF35k238mVyRtlODD6BRgowU= go.opentelemetry.io/otel/metric v1.15.0-rc.1/go.mod h1:bpPBxLwoWWmiK+Hmb6ZaG0zDLIi59lK7M+GjgZ5PN+4= +go.opentelemetry.io/otel/sdk v1.14.0 h1:PDCppFRDq8A1jL9v6KMI6dYesaq+DFcDZvjsoGvxGzY= +go.opentelemetry.io/otel/sdk v1.14.0/go.mod h1:bwIC5TjrNG6QDCHNWvW4HLHtUQ4I+VQDsnjhvyZCALM= go.opentelemetry.io/otel/trace v1.15.0-rc.1 h1:xK6jLm8h2KFhdItNvzAuNvnoWjRPU9u7whXNNBMxjtc= go.opentelemetry.io/otel/trace v1.15.0-rc.1/go.mod h1:2cLx8hBNS4rUWB+JA9PuCGggQl+KJioCaoV2CKewY4s= +golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU= +golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/instrumentation/github.com/emicklei/go-restful/otelrestful/restful.go b/instrumentation/github.com/emicklei/go-restful/otelrestful/restful.go index cd7faef695f..14441123716 100644 --- a/instrumentation/github.com/emicklei/go-restful/otelrestful/restful.go +++ b/instrumentation/github.com/emicklei/go-restful/otelrestful/restful.go @@ -61,6 +61,14 @@ func OTelFilter(service string, opts ...Option) restful.FilterFunction { opts = append(opts, oteltrace.WithAttributes(rAttr)) } + if cfg.PublicEndpoint || (cfg.PublicEndpointFn != nil && cfg.PublicEndpointFn(r.WithContext(ctx))) { + opts = append(opts, oteltrace.WithNewRoot()) + // Linking incoming span context if any for public endpoint. + if s := oteltrace.SpanContextFromContext(ctx); s.IsValid() && s.IsRemote() { + opts = append(opts, oteltrace.WithLinks(oteltrace.Link{SpanContext: s})) + } + } + ctx, span := tracer.Start(ctx, spanName, opts...) defer span.End() diff --git a/instrumentation/github.com/emicklei/go-restful/otelrestful/restful_test.go b/instrumentation/github.com/emicklei/go-restful/otelrestful/restful_test.go index ffb9ed0f91e..30c718b72e1 100644 --- a/instrumentation/github.com/emicklei/go-restful/otelrestful/restful_test.go +++ b/instrumentation/github.com/emicklei/go-restful/otelrestful/restful_test.go @@ -22,11 +22,14 @@ import ( "github.com/emicklei/go-restful/v3" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful" b3prop "go.opentelemetry.io/contrib/propagators/b3" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/propagation" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" oteltrace "go.opentelemetry.io/otel/trace" ) @@ -118,3 +121,145 @@ func TestPropagationWithCustomPropagators(t *testing.T) { container.ServeHTTP(w, r) } + +func TestWithPublicEndpoint(t *testing.T) { + spanRecorder := tracetest.NewSpanRecorder() + provider := sdktrace.NewTracerProvider( + sdktrace.WithSpanProcessor(spanRecorder), + ) + remoteSpan := oteltrace.SpanContextConfig{ + TraceID: oteltrace.TraceID{0x01}, + SpanID: oteltrace.SpanID{0x01}, + Remote: true, + } + prop := propagation.TraceContext{} + + handlerFunc := func(req *restful.Request, resp *restful.Response) { + s := oteltrace.SpanFromContext(req.Request.Context()) + sc := s.SpanContext() + + // Should be with new root trace. + assert.True(t, sc.IsValid()) + assert.False(t, sc.IsRemote()) + assert.NotEqual(t, remoteSpan.TraceID, sc.TraceID()) + } + + ws := &restful.WebService{} + ws.Route(ws.GET("/user/{id}").To(handlerFunc)) + + container := restful.NewContainer() + container.Filter(otelrestful.OTelFilter("test_handler", + otelrestful.WithPublicEndpoint(), + otelrestful.WithPropagators(prop), + otelrestful.WithTracerProvider(provider)), + ) + container.Add(ws) + + r, err := http.NewRequest(http.MethodGet, "http://localhost/user/123", nil) + require.NoError(t, err) + + sc := oteltrace.NewSpanContext(remoteSpan) + ctx := oteltrace.ContextWithSpanContext(context.Background(), sc) + prop.Inject(ctx, propagation.HeaderCarrier(r.Header)) + + rr := httptest.NewRecorder() + container.ServeHTTP(rr, r) + assert.Equal(t, 200, rr.Result().StatusCode) + + // Recorded span should be linked with an incoming span context. + assert.NoError(t, spanRecorder.ForceFlush(ctx)) + done := spanRecorder.Ended() + require.Len(t, done, 1) + require.Len(t, done[0].Links(), 1, "should contain link") + require.True(t, sc.Equal(done[0].Links()[0].SpanContext), "should link incoming span context") +} + +func TestWithPublicEndpointFn(t *testing.T) { + remoteSpan := oteltrace.SpanContextConfig{ + TraceID: oteltrace.TraceID{0x01}, + SpanID: oteltrace.SpanID{0x01}, + TraceFlags: oteltrace.FlagsSampled, + Remote: true, + } + prop := propagation.TraceContext{} + + for _, tt := range []struct { + name string + fn func(*http.Request) bool + handlerAssert func(*testing.T, oteltrace.SpanContext) + spansAssert func(*testing.T, oteltrace.SpanContext, []sdktrace.ReadOnlySpan) + }{ + { + name: "with the method returning true", + fn: func(r *http.Request) bool { + return true + }, + handlerAssert: func(t *testing.T, sc oteltrace.SpanContext) { + // Should be with new root trace. + assert.True(t, sc.IsValid()) + assert.False(t, sc.IsRemote()) + assert.NotEqual(t, remoteSpan.TraceID, sc.TraceID()) + }, + spansAssert: func(t *testing.T, sc oteltrace.SpanContext, spans []sdktrace.ReadOnlySpan) { + require.Len(t, spans, 1) + require.Len(t, spans[0].Links(), 1, "should contain link") + require.True(t, sc.Equal(spans[0].Links()[0].SpanContext), "should link incoming span context") + }, + }, + { + name: "with the method returning false", + fn: func(r *http.Request) bool { + return false + }, + handlerAssert: func(t *testing.T, sc oteltrace.SpanContext) { + // Should have remote span as parent + assert.True(t, sc.IsValid()) + assert.False(t, sc.IsRemote()) + assert.Equal(t, remoteSpan.TraceID, sc.TraceID()) + }, + spansAssert: func(t *testing.T, _ oteltrace.SpanContext, spans []sdktrace.ReadOnlySpan) { + require.Len(t, spans, 1) + require.Len(t, spans[0].Links(), 0, "should not contain link") + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + spanRecorder := tracetest.NewSpanRecorder() + provider := sdktrace.NewTracerProvider( + sdktrace.WithSpanProcessor(spanRecorder), + ) + + handlerFunc := func(req *restful.Request, resp *restful.Response) { + s := oteltrace.SpanFromContext(req.Request.Context()) + tt.handlerAssert(t, s.SpanContext()) + } + + ws := &restful.WebService{} + ws.Route(ws.GET("/user/{id}").To(handlerFunc)) + + container := restful.NewContainer() + container.Filter(otelrestful.OTelFilter("test_handler", + otelrestful.WithPublicEndpointFn(tt.fn), + otelrestful.WithPropagators(prop), + otelrestful.WithTracerProvider(provider)), + ) + container.Add(ws) + + r, err := http.NewRequest(http.MethodGet, "http://localhost/user/123", nil) + require.NoError(t, err) + + sc := oteltrace.NewSpanContext(remoteSpan) + ctx := oteltrace.ContextWithSpanContext(context.Background(), sc) + prop.Inject(ctx, propagation.HeaderCarrier(r.Header)) + + rr := httptest.NewRecorder() + container.ServeHTTP(rr, r) + assert.Equal(t, http.StatusOK, rr.Result().StatusCode) + + // Recorded span should be linked with an incoming span context. + assert.NoError(t, spanRecorder.ForceFlush(ctx)) + spans := spanRecorder.Ended() + tt.spansAssert(t, sc, spans) + }) + } +}