Skip to content

Commit 063b8fd

Browse files
aksdbaksdb
authored
Extend jaeger results (#47)
* propagate status code to span * allow the ComponentName to be adjusted * support error messages and HttpError
1 parent 4026d23 commit 063b8fd

File tree

3 files changed

+93
-19
lines changed

3 files changed

+93
-19
lines changed

‎go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,4 @@ require (
1818
github.com/uber/jaeger-client-go v2.25.0+incompatible
1919
github.com/uber/jaeger-lib v2.4.0+incompatible // indirect
2020
)
21+

‎jaegertracing/jaegertracing.go

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ package jaegertracing
2222

2323
import (
2424
"bytes"
25+
"errors"
2526
"fmt"
2627
"io"
2728
"io/ioutil"
@@ -48,8 +49,8 @@ type (
4849
// OpenTracing Tracer instance which should be got before
4950
Tracer opentracing.Tracer
5051

51-
// componentName used for describing the tracing component name
52-
componentName string
52+
// ComponentName used for describing the tracing component name
53+
ComponentName string
5354

5455
// add req body & resp body to tracing tags
5556
IsBodyDump bool
@@ -60,7 +61,7 @@ var (
6061
// DefaultTraceConfig is the default Trace middleware config.
6162
DefaultTraceConfig = TraceConfig{
6263
Skipper: middleware.DefaultSkipper,
63-
componentName: defaultComponentName,
64+
ComponentName: defaultComponentName,
6465
IsBodyDump: false,
6566
}
6667
)
@@ -102,7 +103,7 @@ func New(e *echo.Echo, skipper middleware.Skipper) io.Closer {
102103
func Trace(tracer opentracing.Tracer) echo.MiddlewareFunc {
103104
c := DefaultTraceConfig
104105
c.Tracer = tracer
105-
c.componentName = defaultComponentName
106+
c.ComponentName = defaultComponentName
106107
return TraceWithConfig(c)
107108
}
108109

@@ -115,8 +116,8 @@ func TraceWithConfig(config TraceConfig) echo.MiddlewareFunc {
115116
if config.Skipper == nil {
116117
config.Skipper = middleware.DefaultSkipper
117118
}
118-
if config.componentName == "" {
119-
config.componentName = defaultComponentName
119+
if config.ComponentName == "" {
120+
config.ComponentName = defaultComponentName
120121
}
121122

122123
return func(next echo.HandlerFunc) echo.HandlerFunc {
@@ -138,7 +139,7 @@ func TraceWithConfig(config TraceConfig) echo.MiddlewareFunc {
138139

139140
ext.HTTPMethod.Set(sp, req.Method)
140141
ext.HTTPUrl.Set(sp, req.URL.String())
141-
ext.Component.Set(sp, config.componentName)
142+
ext.Component.Set(sp, config.ComponentName)
142143

143144
// Dump request & response body
144145
resBody := new(bytes.Buffer)
@@ -161,9 +162,30 @@ func TraceWithConfig(config TraceConfig) echo.MiddlewareFunc {
161162
req = req.WithContext(opentracing.ContextWithSpan(req.Context(), sp))
162163
c.SetRequest(req)
163164

165+
var err error
164166
defer func() {
165-
status := c.Response().Status
166167
committed := c.Response().Committed
168+
status := c.Response().Status
169+
170+
if err != nil {
171+
var httpError *echo.HTTPError
172+
if errors.As(err, &httpError) {
173+
if httpError.Code != 0 {
174+
status = httpError.Code
175+
}
176+
sp.SetTag("error.message", httpError.Message)
177+
} else {
178+
sp.SetTag("error.message", err.Error())
179+
}
180+
if status == http.StatusOK {
181+
// this is ugly workaround for cases when httpError.code == 0 or error was not httpError and status
182+
// in request was 200 (OK). In these cases replace status with something that represents an error
183+
// it could be that error handlers or middlewares up in chain will output different status code to
184+
// client. but at least we send something better than 200 to jaeger
185+
status = http.StatusInternalServerError
186+
}
187+
}
188+
167189
ext.HTTPStatusCode.Set(sp, uint16(status))
168190
if status >= http.StatusInternalServerError || !committed {
169191
ext.Error.Set(sp, true)
@@ -176,7 +198,8 @@ func TraceWithConfig(config TraceConfig) echo.MiddlewareFunc {
176198

177199
sp.Finish()
178200
}()
179-
return next(c)
201+
err = next(c)
202+
return err
180203
}
181204
}
182205
}

‎jaegertracing/jaegertracing_test.go

Lines changed: 60 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package jaegertracing
33
import (
44
"bytes"
55
"errors"
6+
"fmt"
67
"net/http"
78
"net/http/httptest"
89
"testing"
@@ -117,15 +118,62 @@ func TestTraceWithDefaultConfig(t *testing.T) {
117118

118119
e := echo.New()
119120
e.Use(Trace(tracer))
120-
req := httptest.NewRequest(http.MethodGet, "/", nil)
121-
rec := httptest.NewRecorder()
122-
e.ServeHTTP(rec, req)
123121

124-
assert.Equal(t, "GET", tracer.currentSpan().getTag("http.method"))
125-
assert.Equal(t, "/", tracer.currentSpan().getTag("http.url"))
126-
assert.Equal(t, defaultComponentName, tracer.currentSpan().getTag("component"))
127-
assert.Equal(t, uint16(200), tracer.currentSpan().getTag("http.status_code"))
128-
assert.Equal(t, true, tracer.currentSpan().getTag("error"))
122+
e.GET("/hello", func(c echo.Context) error {
123+
return c.String(http.StatusOK, "world")
124+
})
125+
126+
e.GET("/giveme400", func(c echo.Context) error {
127+
return echo.NewHTTPError(http.StatusBadRequest, "baaaad request")
128+
})
129+
130+
e.GET("/givemeerror", func(c echo.Context) error {
131+
return fmt.Errorf("internal stuff went wrong")
132+
})
133+
134+
t.Run("successful call", func(t *testing.T) {
135+
req := httptest.NewRequest(http.MethodGet, "/hello", nil)
136+
rec := httptest.NewRecorder()
137+
e.ServeHTTP(rec, req)
138+
139+
assert.Equal(t, "GET", tracer.currentSpan().getTag("http.method"))
140+
assert.Equal(t, "/hello", tracer.currentSpan().getTag("http.url"))
141+
assert.Equal(t, defaultComponentName, tracer.currentSpan().getTag("component"))
142+
assert.Equal(t, uint16(200), tracer.currentSpan().getTag("http.status_code"))
143+
assert.NotEqual(t, true, tracer.currentSpan().getTag("error"))
144+
})
145+
146+
t.Run("error from echo", func(t *testing.T) {
147+
req := httptest.NewRequest(http.MethodGet, "/idontexist", nil)
148+
rec := httptest.NewRecorder()
149+
e.ServeHTTP(rec, req)
150+
151+
assert.Equal(t, "GET", tracer.currentSpan().getTag("http.method"))
152+
assert.Equal(t, "/idontexist", tracer.currentSpan().getTag("http.url"))
153+
assert.Equal(t, defaultComponentName, tracer.currentSpan().getTag("component"))
154+
assert.Equal(t, uint16(404), tracer.currentSpan().getTag("http.status_code"))
155+
assert.Equal(t, true, tracer.currentSpan().getTag("error"))
156+
})
157+
158+
t.Run("custom http error", func(t *testing.T) {
159+
req := httptest.NewRequest(http.MethodGet, "/giveme400", nil)
160+
rec := httptest.NewRecorder()
161+
e.ServeHTTP(rec, req)
162+
163+
assert.Equal(t, uint16(400), tracer.currentSpan().getTag("http.status_code"))
164+
assert.Equal(t, true, tracer.currentSpan().getTag("error"))
165+
assert.Equal(t, "baaaad request", tracer.currentSpan().getTag("error.message"))
166+
})
167+
168+
t.Run("unknown error", func(t *testing.T) {
169+
req := httptest.NewRequest(http.MethodGet, "/givemeerror", nil)
170+
rec := httptest.NewRecorder()
171+
e.ServeHTTP(rec, req)
172+
173+
assert.Equal(t, uint16(500), tracer.currentSpan().getTag("http.status_code"))
174+
assert.Equal(t, true, tracer.currentSpan().getTag("error"))
175+
assert.Equal(t, "internal stuff went wrong", tracer.currentSpan().getTag("error.message"))
176+
})
129177
}
130178

131179
func TestTraceWithConfig(t *testing.T) {
@@ -134,7 +182,7 @@ func TestTraceWithConfig(t *testing.T) {
134182
e := echo.New()
135183
e.Use(TraceWithConfig(TraceConfig{
136184
Tracer: tracer,
137-
componentName: "EchoTracer",
185+
ComponentName: "EchoTracer",
138186
}))
139187
req := httptest.NewRequest(http.MethodGet, "/trace", nil)
140188
rec := httptest.NewRecorder()
@@ -153,7 +201,7 @@ func TestTraceWithConfigOfBodyDump(t *testing.T) {
153201
e := echo.New()
154202
e.Use(TraceWithConfig(TraceConfig{
155203
Tracer: tracer,
156-
componentName: "EchoTracer",
204+
ComponentName: "EchoTracer",
157205
IsBodyDump: true,
158206
}))
159207
e.POST("/trace", func(c echo.Context) error {
@@ -169,6 +217,8 @@ func TestTraceWithConfigOfBodyDump(t *testing.T) {
169217
assert.Equal(t, "/trace", tracer.currentSpan().getTag("http.url"))
170218
assert.Equal(t, `{"name": "Lorem"}`, tracer.currentSpan().getTag("http.req.body"))
171219
assert.Equal(t, `Hi`, tracer.currentSpan().getTag("http.resp.body"))
220+
assert.Equal(t, uint16(200), tracer.currentSpan().getTag("http.status_code"))
221+
assert.Equal(t, nil, tracer.currentSpan().getTag("error"))
172222
assert.Equal(t, true, tracer.hasStartSpanWithOption)
173223

174224
}

0 commit comments

Comments
 (0)