Skip to content

Commit

Permalink
Extend jaeger results (#47)
Browse files Browse the repository at this point in the history
* propagate status code to span
* allow the ComponentName to be adjusted
* support error messages and HttpError
  • Loading branch information
aksdb authored Jun 1, 2021
1 parent 4026d23 commit 063b8fd
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 19 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

41 changes: 32 additions & 9 deletions jaegertracing/jaegertracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ package jaegertracing

import (
"bytes"
"errors"
"fmt"
"io"
"io/ioutil"
Expand All @@ -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
Expand All @@ -60,7 +61,7 @@ var (
// DefaultTraceConfig is the default Trace middleware config.
DefaultTraceConfig = TraceConfig{
Skipper: middleware.DefaultSkipper,
componentName: defaultComponentName,
ComponentName: defaultComponentName,
IsBodyDump: false,
}
)
Expand Down Expand Up @@ -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)
}

Expand All @@ -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 {
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -176,7 +198,8 @@ func TraceWithConfig(config TraceConfig) echo.MiddlewareFunc {

sp.Finish()
}()
return next(c)
err = next(c)
return err
}
}
}
Expand Down
70 changes: 60 additions & 10 deletions jaegertracing/jaegertracing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package jaegertracing
import (
"bytes"
"errors"
"fmt"
"net/http"
"net/http/httptest"
"testing"
Expand Down Expand Up @@ -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) {
Expand All @@ -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()
Expand All @@ -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 {
Expand All @@ -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)

}
Expand Down

0 comments on commit 063b8fd

Please sign in to comment.