chore: remove superfluous config and MR comments

Author: asecondo

CHANGELOG.md

@@ -27,7 +27,7 @@ The next release will require at least [Go 1.21].
 - Add support for Summary metrics to `go.opentelemetry.io/contrib/bridges/prometheus`. (#5089)
 - Add support for Exponential (native) Histograms in `go.opentelemetry.io/contrib/bridges/prometheus`. (#5093)
 - Implemented setting the `cloud.resource_id` resource attribute in `go.opentelemetry.io/detectors/aws/ecs` based on the ECS Metadata v4 endpoint. (#5091)
-- Add support to record panics in `go.opentelemetry.io.contrib/instrumentation/github.com/gin-gonic/gin/otelgin` (#5088)
+- Add support to record panics in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`. (#5090)
 
 ### Removed
 

instrumentation/github.com/gin-gonic/gin/otelgin/gintrace.go

@@ -76,14 +76,12 @@ func Middleware(service string, opts ...Option) gin.HandlerFunc {
 		}
 		ctx, span := tracer.Start(ctx, spanName, opts...)
 		defer func() {
-			if cfg.RecordPanicConfig.enabled {
-				if r := recover(); r != nil {
-					err := fmt.Errorf("error handling request: %s", r)
-					span.RecordError(err, oteltrace.WithStackTrace(cfg.RecordPanicConfig.stackTrace))
-					span.SetStatus(codes.Error, "panic recovered")
-					span.End()
-					panic(r)
-				}
+			if r := recover(); r != nil {
+				err := fmt.Errorf("%+v", r)
+				span.RecordError(err, oteltrace.WithStackTrace(cfg.RecordPanicStackTrace))
+				span.SetStatus(codes.Error, err.Error())
+				span.End()
+				panic(r)
 			}
 			span.End()
 		}()

instrumentation/github.com/gin-gonic/gin/otelgin/option.go

@@ -13,51 +13,11 @@ import (
 )
 
 type config struct {
-	TracerProvider    oteltrace.TracerProvider
-	Propagators       propagation.TextMapPropagator
-	Filters           []Filter
-	SpanNameFormatter SpanNameFormatter
-	RecordPanicConfig RecordPanicConfig
-}
-
-// RecordPanicConfig is a configuration for panic recording.
-type RecordPanicConfig struct {
-	enabled    bool
-	stackTrace bool
-}
-
-// RecordPanicOption applies a configuration for panic recording.
-type RecordPanicOption interface {
-	apply(*RecordPanicConfig)
-}
-
-type recordPanicOptionFunc func(*RecordPanicConfig)
-
-func (o recordPanicOptionFunc) apply(c *RecordPanicConfig) {
-	o(c)
-}
-
-// WithRecordPanicEnabled enables panic recording based on the provided boolean.
-func WithRecordPanicEnabled(enabled bool) RecordPanicOption {
-	return recordPanicOptionFunc(func(cfg *RecordPanicConfig) {
-		cfg.enabled = enabled
-	})
-}
-
-// WithRecordPanicStackTrace enables recording of the panic stack trace based on the provided boolean.
-func WithRecordPanicStackTrace(stackTrace bool) RecordPanicOption {
-	return recordPanicOptionFunc(func(cfg *RecordPanicConfig) {
-		cfg.stackTrace = stackTrace
-	})
-}
-
-// NewRecordPanicConfig creates a new RecordPanicConfig with the provided options.
-func NewRecordPanicConfig(opts ...RecordPanicOption) RecordPanicConfig {
-	cfg := RecordPanicConfig{}
-	for _, opt := range opts {
-		opt.apply(&cfg)
-	}
-	return cfg
+	TracerProvider        oteltrace.TracerProvider
+	Propagators           propagation.TextMapPropagator
+	Filters               []Filter
+	SpanNameFormatter     SpanNameFormatter
+	RecordPanicStackTrace bool
 }
 
 // Filter is a predicate used to determine whether a given http.request should
@@ -119,9 +79,9 @@ func WithSpanNameFormatter(f func(r *http.Request) string) Option {
 	})
 }
 
-// WithRecordPanicConfig specifies a configuration for panic recording.
-func WithRecordPanicConfig(cfg RecordPanicConfig) Option {
+// WithRecordPanicStackTrace specifies whether to record the stack trace of a panic.
+func WithRecordPanicStackTrace(stackTrace bool) Option {
 	return optionFunc(func(c *config) {
-		c.RecordPanicConfig = cfg
+		c.RecordPanicStackTrace = stackTrace
 	})
 }

instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go

@@ -261,48 +261,47 @@ func TestRecordPanic(t *testing.T) {
 		c.Next()
 	}
 
-	t.Run("panic recovered and recorded", func(t *testing.T) {
-		sr := tracetest.NewSpanRecorder()
-		provider := sdktrace.NewTracerProvider()
-		provider.RegisterSpanProcessor(sr)
-		router := gin.New()
-		recordPanicConfig := otelgin.NewRecordPanicConfig(otelgin.WithRecordPanicEnabled(true), otelgin.WithRecordPanicStackTrace(true))
-		router.Use(recoveryMiddleware, otelgin.Middleware("potato", otelgin.WithTracerProvider(provider), otelgin.WithRecordPanicConfig(recordPanicConfig)))
-		router.GET("/user/:id", func(c *gin.Context) { panic("corn") })
-		router.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/user/123", nil))
-
-		require.Len(t, sr.Ended(), 1, "should emit a span")
-		span := sr.Ended()[0]
-		assert.Equal(t, span.Status().Code, codes.Error, "should set Error status for panics")
-		require.Len(t, span.Events(), 1, "should emit an event")
-		event := span.Events()[0]
-		assert.Equal(t, event.Name, "exception")
-
-		var foundStackTrace bool
-
-		for _, attr := range event.Attributes {
-			if attr.Key == "exception.stacktrace" {
-				foundStackTrace = true
-				break
-			}
-		}
+	testCases := []struct {
+		name             string
+		expectStackTrace bool
+	}{
+		{
+			name:             "should record stack trace",
+			expectStackTrace: true,
+		},
+		{
+			name:             "should not record stack trace",
+			expectStackTrace: false,
+		},
+	}
 
-		assert.True(t, foundStackTrace, "should record a stack trace")
-	})
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			sr := tracetest.NewSpanRecorder()
+			provider := sdktrace.NewTracerProvider()
+			provider.RegisterSpanProcessor(sr)
+			router := gin.New()
+			router.Use(recoveryMiddleware, otelgin.Middleware("potato", otelgin.WithTracerProvider(provider), otelgin.WithRecordPanicStackTrace(tc.expectStackTrace)))
+			router.GET("/user/:id", func(c *gin.Context) { panic("corn") })
+			router.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/user/123", nil))
 
-	t.Run("panic recovered and not recorded", func(t *testing.T) {
-		sr := tracetest.NewSpanRecorder()
-		provider := sdktrace.NewTracerProvider()
-		provider.RegisterSpanProcessor(sr)
-		router := gin.New()
-		recordPanicConfig := otelgin.NewRecordPanicConfig(otelgin.WithRecordPanicEnabled(false))
-		router.Use(recoveryMiddleware, otelgin.Middleware("potato", otelgin.WithTracerProvider(provider), otelgin.WithRecordPanicConfig(recordPanicConfig)))
-		router.GET("/user/:id", func(c *gin.Context) { panic("corn") })
-		router.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/user/123", nil))
-
-		require.Len(t, sr.Ended(), 1, "should emit a span")
-		span := sr.Ended()[0]
-		assert.Equal(t, span.Status().Code, codes.Unset, "should set Unset status for unrecovered panics")
-		require.Empty(t, span.Events(), "should not emit an event")
-	})
+			require.Len(t, sr.Ended(), 1, "should emit a span")
+			span := sr.Ended()[0]
+			assert.Equal(t, span.Status().Code, codes.Error, "should set Error status for panics")
+			require.Len(t, span.Events(), 1, "should emit an event")
+			event := span.Events()[0]
+			assert.Equal(t, event.Name, "exception")
+
+			var foundStackTrace bool
+
+			for _, attr := range event.Attributes {
+				if attr.Key == "exception.stacktrace" {
+					foundStackTrace = true
+					break
+				}
+			}
+
+			assert.Equal(t, tc.expectStackTrace, foundStackTrace, "should record a stack trace")
+		})
+	}
 }