Skip to content

Commit b412cc1

Browse files
f0sself0ssel
authored
fix: use correct response writer for tracing middle (#3693)
1 parent 78a2494 commit b412cc1

File tree

3 files changed

+42
-27
lines changed

3 files changed

+42
-27
lines changed

‎coderd/tracing/httpmw.go

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"github.com/go-chi/chi/v5"
99
sdktrace "go.opentelemetry.io/otel/sdk/trace"
1010
semconv "go.opentelemetry.io/otel/semconv/v1.10.0"
11+
"go.opentelemetry.io/otel/trace"
1112
)
1213

1314
// HTTPMW adds tracing to http routes.
@@ -27,28 +28,37 @@ func HTTPMW(tracerProvider *sdktrace.TracerProvider, name string) func(http.Hand
2728
wrw := middleware.NewWrapResponseWriter(rw, r.ProtoMajor)
2829

2930
// pass the span through the request context and serve the request to the next middleware
30-
next.ServeHTTP(rw, r)
31+
next.ServeHTTP(wrw, r)
3132

32-
// set the resource name as we get it only once the handler is executed
33-
route := chi.RouteContext(r.Context()).RoutePattern()
34-
if route != "" {
35-
span.SetName(fmt.Sprintf("%s %s", r.Method, route))
36-
}
37-
span.SetName(fmt.Sprintf("%s %s", r.Method, route))
38-
span.SetAttributes(semconv.NetAttributesFromHTTPRequest("tcp", r)...)
39-
span.SetAttributes(semconv.EndUserAttributesFromHTTPRequest(r)...)
40-
span.SetAttributes(semconv.HTTPServerAttributesFromHTTPRequest("", route, r)...)
41-
span.SetAttributes(semconv.HTTPRouteKey.String(route))
42-
43-
// set the status code
44-
status := wrw.Status()
45-
// 0 status means one has not yet been sent in which case net/http library will write StatusOK
46-
if status == 0 {
47-
status = http.StatusOK
48-
}
49-
span.SetAttributes(semconv.HTTPStatusCodeKey.Int(status))
50-
spanStatus, spanMessage := semconv.SpanStatusFromHTTPStatusCode(status)
51-
span.SetStatus(spanStatus, spanMessage)
33+
// capture response data
34+
EndHTTPSpan(r, wrw.Status())
5235
})
5336
}
5437
}
38+
39+
// EndHTTPSpan captures request and response data after the handler is done.
40+
func EndHTTPSpan(r *http.Request, status int) {
41+
span := trace.SpanFromContext(r.Context())
42+
43+
// set the resource name as we get it only once the handler is executed
44+
route := chi.RouteContext(r.Context()).RoutePattern()
45+
if route != "" {
46+
span.SetName(fmt.Sprintf("%s %s", r.Method, route))
47+
}
48+
span.SetName(fmt.Sprintf("%s %s", r.Method, route))
49+
span.SetAttributes(semconv.NetAttributesFromHTTPRequest("tcp", r)...)
50+
span.SetAttributes(semconv.EndUserAttributesFromHTTPRequest(r)...)
51+
span.SetAttributes(semconv.HTTPServerAttributesFromHTTPRequest("", route, r)...)
52+
span.SetAttributes(semconv.HTTPRouteKey.String(route))
53+
54+
// 0 status means one has not yet been sent in which case net/http library will write StatusOK
55+
if status == 0 {
56+
status = http.StatusOK
57+
}
58+
span.SetAttributes(semconv.HTTPStatusCodeKey.Int(status))
59+
spanStatus, spanMessage := semconv.SpanStatusFromHTTPStatusCode(status)
60+
span.SetStatus(spanStatus, spanMessage)
61+
62+
// finally end span
63+
span.End()
64+
}

‎coderd/workspaceagents.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"github.com/google/uuid"
1515
"github.com/hashicorp/yamux"
1616
"github.com/tabbed/pqtype"
17-
"go.opentelemetry.io/otel/trace"
1817
"golang.org/x/xerrors"
1918
"inet.af/netaddr"
2019
"nhooyr.io/websocket"
@@ -27,6 +26,7 @@ import (
2726
"github.com/coder/coder/coderd/httpapi"
2827
"github.com/coder/coder/coderd/httpmw"
2928
"github.com/coder/coder/coderd/rbac"
29+
"github.com/coder/coder/coderd/tracing"
3030
"github.com/coder/coder/coderd/turnconn"
3131
"github.com/coder/coder/codersdk"
3232
"github.com/coder/coder/peer"
@@ -111,7 +111,7 @@ func (api *API) workspaceAgentDial(rw http.ResponseWriter, r *http.Request) {
111111
}
112112

113113
// end span so we don't get long lived trace data
114-
trace.SpanFromContext(ctx).End()
114+
tracing.EndHTTPSpan(r, 200)
115115

116116
err = peerbroker.ProxyListen(ctx, session, peerbroker.ProxyOptions{
117117
ChannelID: workspaceAgent.ID.String(),
@@ -276,7 +276,7 @@ func (api *API) workspaceAgentListen(rw http.ResponseWriter, r *http.Request) {
276276
}
277277

278278
// end span so we don't get long lived trace data
279-
trace.SpanFromContext(ctx).End()
279+
tracing.EndHTTPSpan(r, 200)
280280

281281
api.Logger.Info(ctx, "accepting agent", slog.F("resource", resource), slog.F("agent", workspaceAgent))
282282

@@ -365,8 +365,8 @@ func (api *API) workspaceAgentTurn(rw http.ResponseWriter, r *http.Request) {
365365
}
366366

367367
ctx, wsNetConn := websocketNetConn(r.Context(), wsConn, websocket.MessageBinary)
368-
defer wsNetConn.Close() // Also closes conn.
369-
trace.SpanFromContext(ctx).End() // end span so we don't get long lived trace data
368+
defer wsNetConn.Close() // Also closes conn.
369+
tracing.EndHTTPSpan(r, 200) // end span so we don't get long lived trace data
370370

371371
api.Logger.Debug(ctx, "accepting turn connection", slog.F("remote-address", r.RemoteAddr), slog.F("local-address", localAddress))
372372
select {
@@ -581,7 +581,7 @@ func (api *API) workspaceAgentWireguardListener(rw http.ResponseWriter, r *http.
581581
defer subCancel()
582582

583583
// end span so we don't get long lived trace data
584-
trace.SpanFromContext(ctx).End()
584+
tracing.EndHTTPSpan(r, 200)
585585

586586
// Wait for the connection to close or the client to send a message.
587587
//nolint:dogsled

‎coderd/workspaceapps.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/coder/coder/coderd/httpapi"
1717
"github.com/coder/coder/coderd/httpmw"
1818
"github.com/coder/coder/coderd/rbac"
19+
"github.com/coder/coder/coderd/tracing"
1920
"github.com/coder/coder/codersdk"
2021
"github.com/coder/coder/site"
2122
)
@@ -177,5 +178,9 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request)
177178
r.Header.Add("Cookie", httpapi.StripCoderCookies(cookieHeader))
178179
}
179180
proxy.Transport = conn.HTTPTransport()
181+
182+
// end span so we don't get long lived trace data
183+
tracing.EndHTTPSpan(r, 200)
184+
180185
proxy.ServeHTTP(rw, r)
181186
}

0 commit comments

Comments
 (0)