Extend jaeger results

go.mod

@@ -18,3 +18,4 @@ require (
 	github.com/uber/jaeger-client-go v2.25.0+incompatible
 	github.com/uber/jaeger-lib v2.4.0+incompatible // indirect
 )
+

jaegertracing/jaegertracing.go

@@ -22,6 +22,7 @@ package jaegertracing
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -48,8 +49,8 @@ type (
 		// OpenTracing Tracer instance which should be got before
 		Tracer opentracing.Tracer
 
-		// componentName used for describing the tracing component name
-		componentName string
+		// ComponentName used for describing the tracing component name
+		ComponentName string
 
 		// add req body & resp body to tracing tags
 		IsBodyDump bool
@@ -60,7 +61,7 @@ var (
 	// DefaultTraceConfig is the default Trace middleware config.
 	DefaultTraceConfig = TraceConfig{
 		Skipper:       middleware.DefaultSkipper,
-		componentName: defaultComponentName,
+		ComponentName: defaultComponentName,
 		IsBodyDump:    false,
 	}
 )
@@ -102,7 +103,7 @@ func New(e *echo.Echo, skipper middleware.Skipper) io.Closer {
 func Trace(tracer opentracing.Tracer) echo.MiddlewareFunc {
 	c := DefaultTraceConfig
 	c.Tracer = tracer
-	c.componentName = defaultComponentName
+	c.ComponentName = defaultComponentName
 	return TraceWithConfig(c)
 }
 
@@ -115,8 +116,8 @@ func TraceWithConfig(config TraceConfig) echo.MiddlewareFunc {
 	if config.Skipper == nil {
 		config.Skipper = middleware.DefaultSkipper
 	}
-	if config.componentName == "" {
-		config.componentName = defaultComponentName
+	if config.ComponentName == "" {
+		config.ComponentName = defaultComponentName
 	}
 
 	return func(next echo.HandlerFunc) echo.HandlerFunc {
@@ -138,7 +139,7 @@ func TraceWithConfig(config TraceConfig) echo.MiddlewareFunc {
 
 			ext.HTTPMethod.Set(sp, req.Method)
 			ext.HTTPUrl.Set(sp, req.URL.String())
-			ext.Component.Set(sp, config.componentName)
+			ext.Component.Set(sp, config.ComponentName)
 
 			// Dump request & response body
 			resBody := new(bytes.Buffer)
@@ -161,9 +162,30 @@ func TraceWithConfig(config TraceConfig) echo.MiddlewareFunc {
 			req = req.WithContext(opentracing.ContextWithSpan(req.Context(), sp))
 			c.SetRequest(req)
 
+			var err error
 			defer func() {
-				status := c.Response().Status
 				committed := c.Response().Committed
+				status := c.Response().Status
+
+				if err != nil {
+					var httpError *echo.HTTPError
+					if errors.As(err, &httpError) {
+						if httpError.Code != 0 {
+							status = httpError.Code
+						}
+						sp.SetTag("error.message", httpError.Message)
+					} else {
+						sp.SetTag("error.message", err.Error())
+					}
+					if status == http.StatusOK {
+						// this is ugly workaround for cases when httpError.code == 0 or error was not httpError and status
+						// in request was 200 (OK). In these cases replace status with something that represents an error
+						// it could be that error handlers or middlewares up in chain will output different status code to
+						// client. but at least we send something better than 200 to jaeger
+						status = http.StatusInternalServerError
+					}
+				}
+
 				ext.HTTPStatusCode.Set(sp, uint16(status))
 				if status >= http.StatusInternalServerError || !committed {
 					ext.Error.Set(sp, true)
@@ -176,7 +198,8 @@ func TraceWithConfig(config TraceConfig) echo.MiddlewareFunc {
 
 				sp.Finish()
 			}()
-			return next(c)
+			err = next(c)
+			return err
 		}
 	}
 }

jaegertracing/jaegertracing_test.go

@@ -3,6 +3,7 @@ package jaegertracing
 import (
 	"bytes"
 	"errors"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -117,15 +118,62 @@ func TestTraceWithDefaultConfig(t *testing.T) {
 
 	e := echo.New()
 	e.Use(Trace(tracer))
-	req := httptest.NewRequest(http.MethodGet, "/", nil)
-	rec := httptest.NewRecorder()
-	e.ServeHTTP(rec, req)
 
-	assert.Equal(t, "GET", tracer.currentSpan().getTag("http.method"))
-	assert.Equal(t, "/", tracer.currentSpan().getTag("http.url"))
-	assert.Equal(t, defaultComponentName, tracer.currentSpan().getTag("component"))
-	assert.Equal(t, uint16(200), tracer.currentSpan().getTag("http.status_code"))
-	assert.Equal(t, true, tracer.currentSpan().getTag("error"))
+	e.GET("/hello", func(c echo.Context) error {
+		return c.String(http.StatusOK, "world")
+	})
+
+	e.GET("/giveme400", func(c echo.Context) error {
+		return echo.NewHTTPError(http.StatusBadRequest, "baaaad request")
+	})
+
+	e.GET("/givemeerror", func(c echo.Context) error {
+		return fmt.Errorf("internal stuff went wrong")
+	})
+
+	t.Run("successful call", func(t *testing.T) {
+		req := httptest.NewRequest(http.MethodGet, "/hello", nil)
+		rec := httptest.NewRecorder()
+		e.ServeHTTP(rec, req)
+
+		assert.Equal(t, "GET", tracer.currentSpan().getTag("http.method"))
+		assert.Equal(t, "/hello", tracer.currentSpan().getTag("http.url"))
+		assert.Equal(t, defaultComponentName, tracer.currentSpan().getTag("component"))
+		assert.Equal(t, uint16(200), tracer.currentSpan().getTag("http.status_code"))
+		assert.NotEqual(t, true, tracer.currentSpan().getTag("error"))
+	})
+
+	t.Run("error from echo", func(t *testing.T) {
+		req := httptest.NewRequest(http.MethodGet, "/idontexist", nil)
+		rec := httptest.NewRecorder()
+		e.ServeHTTP(rec, req)
+
+		assert.Equal(t, "GET", tracer.currentSpan().getTag("http.method"))
+		assert.Equal(t, "/idontexist", tracer.currentSpan().getTag("http.url"))
+		assert.Equal(t, defaultComponentName, tracer.currentSpan().getTag("component"))
+		assert.Equal(t, uint16(404), tracer.currentSpan().getTag("http.status_code"))
+		assert.Equal(t, true, tracer.currentSpan().getTag("error"))
+	})
+
+	t.Run("custom http error", func(t *testing.T) {
+		req := httptest.NewRequest(http.MethodGet, "/giveme400", nil)
+		rec := httptest.NewRecorder()
+		e.ServeHTTP(rec, req)
+
+		assert.Equal(t, uint16(400), tracer.currentSpan().getTag("http.status_code"))
+		assert.Equal(t, true, tracer.currentSpan().getTag("error"))
+		assert.Equal(t, "baaaad request", tracer.currentSpan().getTag("error.message"))
+	})
+
+	t.Run("unknown error", func(t *testing.T) {
+		req := httptest.NewRequest(http.MethodGet, "/givemeerror", nil)
+		rec := httptest.NewRecorder()
+		e.ServeHTTP(rec, req)
+
+		assert.Equal(t, uint16(500), tracer.currentSpan().getTag("http.status_code"))
+		assert.Equal(t, true, tracer.currentSpan().getTag("error"))
+		assert.Equal(t, "internal stuff went wrong", tracer.currentSpan().getTag("error.message"))
+	})
 }
 
 func TestTraceWithConfig(t *testing.T) {
@@ -134,7 +182,7 @@ func TestTraceWithConfig(t *testing.T) {
 	e := echo.New()
 	e.Use(TraceWithConfig(TraceConfig{
 		Tracer:        tracer,
-		componentName: "EchoTracer",
+		ComponentName: "EchoTracer",
 	}))
 	req := httptest.NewRequest(http.MethodGet, "/trace", nil)
 	rec := httptest.NewRecorder()
@@ -153,7 +201,7 @@ func TestTraceWithConfigOfBodyDump(t *testing.T) {
 	e := echo.New()
 	e.Use(TraceWithConfig(TraceConfig{
 		Tracer:        tracer,
-		componentName: "EchoTracer",
+		ComponentName: "EchoTracer",
 		IsBodyDump:    true,
 	}))
 	e.POST("/trace", func(c echo.Context) error {
@@ -169,6 +217,8 @@ func TestTraceWithConfigOfBodyDump(t *testing.T) {
 	assert.Equal(t, "/trace", tracer.currentSpan().getTag("http.url"))
 	assert.Equal(t, `{"name": "Lorem"}`, tracer.currentSpan().getTag("http.req.body"))
 	assert.Equal(t, `Hi`, tracer.currentSpan().getTag("http.resp.body"))
+	assert.Equal(t, uint16(200), tracer.currentSpan().getTag("http.status_code"))
+	assert.Equal(t, nil, tracer.currentSpan().getTag("error"))
 	assert.Equal(t, true, tracer.hasStartSpanWithOption)
 
 }