fix and update

DataDog · Dec 8, 2022 · 61a12f3 · 61a12f3
1 parent 83001e7
commit 61a12f3
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 9 deletions.
diff --git a/contrib/labstack/echo.v4/echotrace.go b/contrib/labstack/echo.v4/echotrace.go
@@ -63,7 +63,8 @@ func Middleware(opts ...Option) echo.MiddlewareFunc {
 
 			span, ctx := httptrace.StartRequestSpan(request, opts...)
 			defer func() {
-				httptrace.FinishRequestSpan(span, c.Response().Status, finishOpts...)
+				//httptrace.FinishRequestSpan(span, c.Response().Status, finishOpts...)
+				span.Finish(finishOpts...)
 			}()
 
 			// pass the span through the request context

diff --git a/contrib/labstack/echo.v4/option.go b/contrib/labstack/echo.v4/option.go
@@ -18,7 +18,7 @@ type config struct {
 	analyticsRate     float64
 	noDebugStack      bool
 	ignoreRequestFunc IgnoreRequestFunc
-	isStatusError func(statusCode int) bool
+	isStatusError     func(statusCode int) bool
 }
 
 // Option represents an option that can be passed to Middleware.
@@ -66,7 +66,6 @@ func WithAnalyticsRate(rate float64) Option {
 	}
 }
 
-
 // NoDebugStack prevents stack traces from being attached to spans finishing
 // with an error. This is useful in situations where errors are frequent and
 // performance is critical.
@@ -82,6 +81,7 @@ func WithIgnoreRequest(ignoreRequestFunc IgnoreRequestFunc) Option {
 	return func(cfg *config) {
 		cfg.ignoreRequestFunc = ignoreRequestFunc
 	}
+}
 
 // WithStatusCheck specifies a function fn which reports whether the passed
 // statusCode should be considered an error.

diff --git a/contrib/labstack/echo/echotrace.go b/contrib/labstack/echo/echotrace.go
@@ -51,7 +51,8 @@ func Middleware(opts ...Option) echo.MiddlewareFunc {
 
 			span, ctx := httptrace.StartRequestSpan(request, opts...)
 			defer func() {
-				httptrace.FinishRequestSpan(span, c.Response().Status, finishOpts...)
+				//httptrace.FinishRequestSpan(span, c.Response().Status, finishOpts...)
+				span.Finish(finishOpts...)
 			}()
 
 			// pass the span through the request context
@@ -68,25 +69,29 @@ func Middleware(opts ...Option) echo.MiddlewareFunc {
 				switch err := err.(type) {
 				case *echo.HTTPError:
 					if cfg.isStatusError(err.Code) {
+						fmt.Printf("FINISH OPTS ADDING ERROR!\n")
 						finishOpts = append(finishOpts, tracer.WithError(err))
 					}
 					span.SetTag(ext.HTTPCode, strconv.Itoa(err.Code))
 				default:
 					// Any non-HTTPError errors appear as 5xx errors.
 					if cfg.isStatusError(500) {
+						fmt.Printf("FINISH OPTS ADDING ERROR!\n")
 						finishOpts = append(finishOpts, tracer.WithError(err))
 					}
 					span.SetTag(ext.HTTPCode, "500")
 				}
 			} else if status := c.Response().Status; status > 0 {
 				if cfg.isStatusError(status) {
 					// mark 5xx server error
+					fmt.Printf("FINISH OPTS ADDING ERROR!\n")
 					finishOpts = append(finishOpts, tracer.WithError(fmt.Errorf("%d: %s", status, http.StatusText(status))))
 				}
 				span.SetTag(ext.HTTPCode, strconv.Itoa(status))
 			} else {
 				if cfg.isStatusError(200) {
 					// mark 5xx server error
+					fmt.Printf("FINISH OPTS ADDING ERROR!\n")
 					finishOpts = append(finishOpts, tracer.WithError(fmt.Errorf("%d: %s", 200, http.StatusText(200))))
 				}
 				span.SetTag(ext.HTTPCode, "200")

diff --git a/contrib/labstack/echo/echotrace_test.go b/contrib/labstack/echo/echotrace_test.go
@@ -18,6 +18,7 @@ import (
 
 	"github.com/labstack/echo"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestChildSpan(t *testing.T) {
@@ -175,6 +176,7 @@ func TestError(t *testing.T) {
 	assert.Equal("http.request", span.OperationName())
 	assert.Equal("foobar", span.Tag(ext.ServiceName))
 	assert.Equal("500", span.Tag(ext.HTTPCode))
+	require.NotNil(t, span.Tag(ext.Error))
 	assert.Equal(wantErr.Error(), span.Tag(ext.Error).(error).Error())
 	assert.Equal("labstack/echo", span.Tag(ext.Component))
 	assert.Equal(ext.SpanKindServer, span.Tag(ext.SpanKind))
@@ -215,6 +217,7 @@ func TestErrorHandling(t *testing.T) {
 	assert.Equal("http.request", span.OperationName())
 	assert.Equal("foobar", span.Tag(ext.ServiceName))
 	assert.Equal("500", span.Tag(ext.HTTPCode))
+	require.NotNil(t, span.Tag(ext.Error))
 	assert.Equal(wantErr.Error(), span.Tag(ext.Error).(error).Error())
 	assert.Equal("labstack/echo", span.Tag(ext.Component))
 	assert.Equal(ext.SpanKindServer, span.Tag(ext.SpanKind))
@@ -248,15 +251,15 @@ func TestStatusError(t *testing.T) {
 				return echo.NewHTTPError(http.StatusBadRequest, "my error message")
 			},
 		},
-		{
+		{ //03
 			isStatusError: func(statusCode int) bool { return statusCode >= 400 && statusCode < 500 },
 			err:           nil,
 			code:          "500",
 			handler: func(c echo.Context) error {
 				return errors.New("oh no")
 			},
 		},
-		{
+		{ //04
 			isStatusError: func(statusCode int) bool { return statusCode >= 400 && statusCode < 500 },
 			err:           nil,
 			code:          "500",
@@ -320,6 +323,7 @@ func TestStatusError(t *testing.T) {
 			err := span.Tag(ext.Error)
 			if tt.err != nil {
 				assert.NotNil(err)
+				require.NotNil(t, span.Tag(ext.Error))
 				assert.Equal(tt.err.Error(), err.(error).Error())
 			} else {
 				assert.Nil(err)
@@ -380,6 +384,7 @@ func TestNoDebugStack(t *testing.T) {
 	assert.Len(spans, 1)
 
 	span := spans[0]
+	require.NotNil(t, span.Tag(ext.Error))
 	assert.Equal(wantErr.Error(), span.Tag(ext.Error).(error).Error())
 	assert.Equal("<debug stack disabled>", span.Tag(ext.ErrorStack))
 	assert.Equal("labstack/echo", span.Tag(ext.Component))

diff --git a/contrib/labstack/echo/option.go b/contrib/labstack/echo/option.go
@@ -65,7 +65,6 @@ func WithAnalyticsRate(rate float64) Option {
 	}
 }
 
-<<<<<<< HEAD
 // NoDebugStack prevents stack traces from being attached to spans finishing
 // with an error. This is useful in situations where errors are frequent and
 // performance is critical.
@@ -74,7 +73,7 @@ func NoDebugStack() Option {
 		cfg.noDebugStack = true
 	}
 }
-=======
+
 // WithStatusCheck specifies a function fn which reports whether the passed
 // statusCode should be considered an error.
 func WithStatusCheck(fn func(statusCode int) bool) Option {
@@ -86,4 +85,3 @@ func WithStatusCheck(fn func(statusCode int) bool) Option {
 func isServerError(statusCode int) bool {
 	return statusCode >= 500 && statusCode < 600
 }
->>>>>>> add WithStatusCheck option