From 4d3fad6e3f0b4fbf58bf79deb389fc210eda7eb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Mon, 2 Sep 2024 17:26:27 +0200 Subject: [PATCH] fix(contrib/labstack/echo.v4): sync with main --- contrib/labstack/echo.v4/echotrace.go | 46 +++++++++++++++------- contrib/labstack/echo.v4/echotrace_test.go | 4 +- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/contrib/labstack/echo.v4/echotrace.go b/contrib/labstack/echo.v4/echotrace.go index 7e91b5a047..7b0fd3bfa3 100644 --- a/contrib/labstack/echo.v4/echotrace.go +++ b/contrib/labstack/echo.v4/echotrace.go @@ -10,7 +10,6 @@ package echo import ( - "errors" "fmt" "math" "net/http" @@ -38,24 +37,34 @@ func Middleware(opts ...Option) echo.MiddlewareFunc { for _, fn := range opts { fn.apply(cfg) } - instr.Logger().Debug("contrib/labstack/echo: Configuring Middleware: %#v", cfg) - spanOpts := []tracer.StartSpanOption{ - tracer.ServiceName(cfg.serviceName), + instr.Logger().Debug("contrib/labstack/echo.v4: Configuring Middleware: %#v", cfg) + spanOpts := make([]tracer.StartSpanOption, 0, 3+len(cfg.tags)) + spanOpts = append(spanOpts, tracer.ServiceName(cfg.serviceName)) + for k, v := range cfg.tags { + spanOpts = append(spanOpts, tracer.Tag(k, v)) + } + spanOpts = append(spanOpts, tracer.Tag(ext.Component, instrumentation.PackageLabstackEchoV4), tracer.Tag(ext.SpanKind, ext.SpanKindServer), - } + ) return func(next echo.HandlerFunc) echo.HandlerFunc { return func(c echo.Context) error { + // If we have an ignoreRequestFunc, use it to see if we proceed with tracing + if cfg.ignoreRequestFunc != nil && cfg.ignoreRequestFunc(c) { + return next(c) + } + request := c.Request() - resource := request.Method + " " + c.Path() + route := c.Path() + resource := request.Method + " " + route opts := options.Copy(spanOpts) // opts must be a copy of spanOpts, locally scoped, to avoid races. if !math.IsNaN(cfg.analyticsRate) { opts = append(opts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate)) } opts = append(opts, tracer.ResourceName(resource), + tracer.Tag(ext.HTTPRoute, route), httptrace.HeaderTagsFromRequest(request, cfg.headerTags)) - // TODO: Should this also have an `http.route` tag like the v4 library does? var finishOpts []tracer.FinishOption if cfg.noDebugStack { @@ -70,18 +79,15 @@ func Middleware(opts ...Option) echo.MiddlewareFunc { // pass the span through the request context c.SetRequest(request.WithContext(ctx)) - // Use AppSec if enabled by user if instr.AppSecEnabled() { next = withAppSec(next, span) } - // serve the request to the next middleware err := next(c) - if err != nil { + if err != nil && !shouldIgnoreError(cfg, err) { // It is impossible to determine what the final status code of a request is in echo. // This is the best we can do. - var echoErr *echo.HTTPError - if errors.As(err, &echoErr) { + if echoErr, ok := cfg.translateError(err); ok { if cfg.isStatusError(echoErr.Code) { finishOpts = append(finishOpts, tracer.WithError(err)) } @@ -95,12 +101,16 @@ func Middleware(opts ...Option) echo.MiddlewareFunc { } } else if status := c.Response().Status; status > 0 { if cfg.isStatusError(status) { - finishOpts = append(finishOpts, tracer.WithError(fmt.Errorf("%d: %s", status, http.StatusText(status)))) + if statusErr := errorFromStatusCode(status); !shouldIgnoreError(cfg, statusErr) { + finishOpts = append(finishOpts, tracer.WithError(statusErr)) + } } span.SetTag(ext.HTTPCode, strconv.Itoa(status)) } else { if cfg.isStatusError(200) { - finishOpts = append(finishOpts, tracer.WithError(fmt.Errorf("%d: %s", 200, http.StatusText(200)))) + if statusErr := errorFromStatusCode(200); !shouldIgnoreError(cfg, statusErr) { + finishOpts = append(finishOpts, tracer.WithError(statusErr)) + } } span.SetTag(ext.HTTPCode, "200") } @@ -108,3 +118,11 @@ func Middleware(opts ...Option) echo.MiddlewareFunc { } } } + +func errorFromStatusCode(statusCode int) error { + return fmt.Errorf("%d: %s", statusCode, http.StatusText(statusCode)) +} + +func shouldIgnoreError(cfg *config, err error) bool { + return cfg.errCheck != nil && !cfg.errCheck(err) +} diff --git a/contrib/labstack/echo.v4/echotrace_test.go b/contrib/labstack/echo.v4/echotrace_test.go index 525f7b5e57..93eecba00c 100644 --- a/contrib/labstack/echo.v4/echotrace_test.go +++ b/contrib/labstack/echo.v4/echotrace_test.go @@ -592,10 +592,10 @@ func TestWithErrorCheck(t *testing.T) { span := spans[0] if tt.wantErr == nil { - assert.NotContains(t, span.Tags(), ext.Error) + assert.NotContains(t, span.Tags(), ext.ErrorMsg) return } - assert.Equal(t, tt.wantErr, span.Tag(ext.Error)) + assert.Equal(t, tt.wantErr.Error(), span.Tag(ext.ErrorMsg)) }) } }