otelrestful: support WithPublicEndpoint and WithPublicEndpointFn (#3563)

* otelrestful: support WithPublicEndpoint Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de> * otelrestful: support WithPublicEndpointFn Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de> * Update instrumentation/github.com/emicklei/go-restful/otelrestful/config.go Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> * Update CHANGELOG.md Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> --------- Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de> Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
open-telemetry · Mar 17, 2023 · 6b0728c · 6b0728c
1 parent 2f803f5
commit 6b0728c
Show file tree

Hide file tree

Showing 6 changed files with 186 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
+		})
+	}
+}