From 61a12f32b292bc6d45aba67c23811d388c9be4d3 Mon Sep 17 00:00:00 2001 From: Kyle Nusbaum Date: Thu, 8 Dec 2022 13:12:38 -0600 Subject: [PATCH] fix and update --- contrib/labstack/echo.v4/echotrace.go | 3 ++- contrib/labstack/echo.v4/option.go | 4 ++-- contrib/labstack/echo/echotrace.go | 7 ++++++- contrib/labstack/echo/echotrace_test.go | 9 +++++++-- contrib/labstack/echo/option.go | 4 +--- 5 files changed, 18 insertions(+), 9 deletions(-) diff --git a/contrib/labstack/echo.v4/echotrace.go b/contrib/labstack/echo.v4/echotrace.go index ef24309158..15d09eb16d 100644 --- 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 index 1d1f4ca66c..d634b3136b 100644 --- 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 index 7106a2eb62..ffd7244cdd 100644 --- 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,12 +69,14 @@ 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") @@ -81,12 +84,14 @@ func Middleware(opts ...Option) echo.MiddlewareFunc { } 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 index b7f3a3ab73..4e70c23c54 100644 --- 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,7 +251,7 @@ 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", @@ -256,7 +259,7 @@ func TestStatusError(t *testing.T) { 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("", 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 index a3ae547ed6..959a0627d6 100644 --- 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