Skip to content

Commit b40c1d6

Browse files
Oloruntobi1Oloruntobi1
committed
[CHORE] fix failing CI/CD
1 parent e81c606 commit b40c1d6

File tree

8 files changed

+127
-26
lines changed

8 files changed

+127
-26
lines changed

‎instrumentation/github.com/labstack/echo/otelecho/echo.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func Middleware(service string, opts ...Option) echo.MiddlewareFunc {
6363
ctx := cfg.Propagators.Extract(savedCtx, propagation.HeaderCarrier(request.Header))
6464
request.Pattern = c.Path()
6565
opts := []oteltrace.SpanStartOption{
66-
oteltrace.WithAttributes(hs.RequestTraceAttrs(service, request)...),
66+
oteltrace.WithAttributes(hs.RequestTraceAttrs(service, request, semconv.RequestTraceAttrsOpts{})...),
6767
oteltrace.WithSpanKind(oteltrace.SpanKindServer),
6868
}
6969
spanName := spanNameFormatter(c)

‎instrumentation/github.com/labstack/echo/otelecho/internal/semconv/bench_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,6 @@ func BenchmarkHTTPServerRequest(b *testing.B) {
4343
b.ReportAllocs()
4444
b.ResetTimer()
4545
for i := 0; i < b.N; i++ {
46-
benchHTTPServerRequestResults = serv.RequestTraceAttrs("", req)
46+
benchHTTPServerRequestResults = serv.RequestTraceAttrs("", req, RequestTraceAttrsOpts{})
4747
}
4848
}

‎instrumentation/github.com/labstack/echo/otelecho/internal/semconv/env.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ type HTTPServer struct {
6161
//
6262
// If the primary server name is not known, server should be an empty string.
6363
// The req Host will be used to determine the server instead.
64-
func (s HTTPServer) RequestTraceAttrs(server string, req *http.Request) []attribute.KeyValue {
64+
func (s HTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts) []attribute.KeyValue {
6565
if s.duplicate {
66-
return append(OldHTTPServer{}.RequestTraceAttrs(server, req), CurrentHTTPServer{}.RequestTraceAttrs(server, req)...)
66+
return append(OldHTTPServer{}.RequestTraceAttrs(server, req), CurrentHTTPServer{}.RequestTraceAttrs(server, req, opts)...)
6767
}
6868
return OldHTTPServer{}.RequestTraceAttrs(server, req)
6969
}

‎instrumentation/github.com/labstack/echo/otelecho/internal/semconv/env_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func TestHTTPServerDoesNotPanic(t *testing.T) {
4242
req, err := http.NewRequest("GET", "http://example.com", nil)
4343
require.NoError(t, err)
4444

45-
_ = tt.server.RequestTraceAttrs("stuff", req)
45+
_ = tt.server.RequestTraceAttrs("stuff", req, RequestTraceAttrsOpts{})
4646
_ = tt.server.ResponseTraceAttrs(ResponseTelemetry{StatusCode: 200})
4747
tt.server.RecordMetrics(context.Background(), ServerMetricData{
4848
ServerName: "stuff",
@@ -250,7 +250,7 @@ func BenchmarkRecordMetrics(b *testing.B) {
250250
for _, bm := range benchmarks {
251251
b.Run(bm.name, func(b *testing.B) {
252252
req, _ := http.NewRequest("GET", "http://example.com", nil)
253-
_ = bm.server.RequestTraceAttrs("stuff", req)
253+
_ = bm.server.RequestTraceAttrs("stuff", req, RequestTraceAttrsOpts{})
254254
_ = bm.server.ResponseTraceAttrs(ResponseTelemetry{StatusCode: 200})
255255
ctx := context.Background()
256256
b.ReportAllocs()

‎instrumentation/github.com/labstack/echo/otelecho/internal/semconv/httpconv.go

+25-9
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ import (
2020
semconvNew "go.opentelemetry.io/otel/semconv/v1.26.0"
2121
)
2222

23+
type RequestTraceAttrsOpts struct {
24+
// If set, this is used as value for the "http.client_ip" attribute.
25+
HTTPClientIP string
26+
}
27+
2328
type CurrentHTTPServer struct{}
2429

2530
// RequestTraceAttrs returns trace attributes for an HTTP request received by a
@@ -38,7 +43,7 @@ type CurrentHTTPServer struct{}
3843
//
3944
// If the primary server name is not known, server should be an empty string.
4045
// The req Host will be used to determine the server instead.
41-
func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request) []attribute.KeyValue {
46+
func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request, opts RequestTraceAttrsOpts) []attribute.KeyValue {
4247
count := 3 // ServerAddress, Method, Scheme
4348

4449
var host string
@@ -65,7 +70,8 @@ func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request) [
6570

6671
scheme := n.scheme(req.TLS != nil)
6772

68-
if peer, peerPort := SplitHostPort(req.RemoteAddr); peer != "" {
73+
peer, peerPort := SplitHostPort(req.RemoteAddr)
74+
if peer != "" {
6975
// The Go HTTP server sets RemoteAddr to "IP:port", this will not be a
7076
// file-path that would be interpreted with a sock family.
7177
count++
@@ -79,7 +85,17 @@ func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request) [
7985
count++
8086
}
8187

82-
clientIP := serverClientIP(req.Header.Get("X-Forwarded-For"))
88+
// For client IP, use, in order:
89+
// 1. The value passed in the options
90+
// 2. The value in the X-Forwarded-For header
91+
// 3. The peer address
92+
clientIP := opts.HTTPClientIP
93+
if clientIP == "" {
94+
clientIP = serverClientIP(req.Header.Get("X-Forwarded-For"))
95+
if clientIP == "" {
96+
clientIP = peer
97+
}
98+
}
8399
if clientIP != "" {
84100
count++
85101
}
@@ -96,8 +112,8 @@ func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request) [
96112
count++
97113
}
98114

99-
httpRoute := req.Pattern
100-
if httpRoute != "" {
115+
route := httpRoute(req.Pattern)
116+
if route != "" {
101117
count++
102118
}
103119

@@ -143,14 +159,14 @@ func (n CurrentHTTPServer) RequestTraceAttrs(server string, req *http.Request) [
143159
attrs = append(attrs, semconvNew.NetworkProtocolVersion(protoVersion))
144160
}
145161

146-
if httpRoute != "" {
147-
attrs = append(attrs, n.Route(httpRoute))
162+
if route != "" {
163+
attrs = append(attrs, n.Route(route))
148164
}
149165

150166
return attrs
151167
}
152168

153-
func (o CurrentHTTPServer) NetworkTransportAttr(network string) attribute.KeyValue {
169+
func (n CurrentHTTPServer) NetworkTransportAttr(network string) attribute.KeyValue {
154170
switch network {
155171
case "tcp", "tcp4", "tcp6":
156172
return semconvNew.NetworkTransportTCP
@@ -185,7 +201,7 @@ func (n CurrentHTTPServer) scheme(https bool) attribute.KeyValue { // nolint:rev
185201
return semconvNew.URLScheme("http")
186202
}
187203

188-
// TraceResponse returns trace attributes for telemetry from an HTTP response.
204+
// ResponseTraceAttrs returns trace attributes for telemetry from an HTTP response.
189205
//
190206
// If any of the fields in the ResponseTelemetry are not set the attribute will be omitted.
191207
func (n CurrentHTTPServer) ResponseTraceAttrs(resp ResponseTelemetry) []attribute.KeyValue {

‎instrumentation/github.com/labstack/echo/otelecho/internal/semconv/httpconv_test.go

+88-10
Original file line numberDiff line numberDiff line change
@@ -259,16 +259,94 @@ func TestCurrentHttpClient_MetricAttributes(t *testing.T) {
259259
}
260260

261261
func TestRequestTraceAttrs_HTTPRoute(t *testing.T) {
262-
req := httptest.NewRequest("GET", "/high/cardinality/path/abc123", nil)
263-
req.Pattern = "/high/cardinality/path/{id}"
262+
tests := []struct {
263+
name string
264+
pattern string
265+
wantRoute string
266+
}{
267+
{
268+
name: "only path",
269+
pattern: "/path/{id}",
270+
wantRoute: "/path/{id}",
271+
},
272+
{
273+
name: "with method",
274+
pattern: "GET /path/{id}",
275+
wantRoute: "/path/{id}",
276+
},
277+
{
278+
name: "with domain",
279+
pattern: "example.com/path/{id}",
280+
wantRoute: "/path/{id}",
281+
},
282+
}
283+
284+
for _, tt := range tests {
285+
t.Run(tt.name, func(t *testing.T) {
286+
req := httptest.NewRequest(http.MethodGet, "/path/abc123", nil)
287+
req.Pattern = tt.pattern
288+
289+
attrs := (CurrentHTTPServer{}).RequestTraceAttrs("", req, RequestTraceAttrsOpts{})
290+
291+
var gotRoute string
292+
for _, attr := range attrs {
293+
if attr.Key == "http.route" {
294+
gotRoute = attr.Value.AsString()
295+
break
296+
}
297+
}
298+
require.Equal(t, tt.wantRoute, gotRoute)
299+
})
300+
}
301+
}
264302

265-
var found bool
266-
for _, attr := range (CurrentHTTPServer{}).RequestTraceAttrs("", req) {
267-
if attr.Key != "http.route" {
268-
continue
269-
}
270-
found = true
271-
assert.Equal(t, req.Pattern, attr.Value.AsString())
303+
func TestRequestTraceAttrs_ClientIP(t *testing.T) {
304+
for _, tt := range []struct {
305+
name string
306+
requestModifierFn func(r *http.Request)
307+
requestTraceOpts RequestTraceAttrsOpts
308+
309+
wantClientIP string
310+
}{
311+
{
312+
name: "with a client IP from the network",
313+
wantClientIP: "1.2.3.4",
314+
},
315+
{
316+
name: "with a client IP from x-forwarded-for header",
317+
requestModifierFn: func(r *http.Request) {
318+
r.Header.Add("X-Forwarded-For", "5.6.7.8")
319+
},
320+
wantClientIP: "5.6.7.8",
321+
},
322+
{
323+
name: "with a client IP in options",
324+
requestModifierFn: func(r *http.Request) {
325+
r.Header.Add("X-Forwarded-For", "5.6.7.8")
326+
},
327+
requestTraceOpts: RequestTraceAttrsOpts{
328+
HTTPClientIP: "9.8.7.6",
329+
},
330+
wantClientIP: "9.8.7.6",
331+
},
332+
} {
333+
t.Run(tt.name, func(t *testing.T) {
334+
req := httptest.NewRequest("GET", "/example", nil)
335+
req.RemoteAddr = "1.2.3.4:5678"
336+
337+
if tt.requestModifierFn != nil {
338+
tt.requestModifierFn(req)
339+
}
340+
341+
var found bool
342+
for _, attr := range (CurrentHTTPServer{}).RequestTraceAttrs("", req, tt.requestTraceOpts) {
343+
if attr.Key != "client.address" {
344+
continue
345+
}
346+
found = true
347+
assert.Equal(t, tt.wantClientIP, attr.Value.AsString())
348+
}
349+
require.True(t, found)
350+
})
272351
}
273-
require.True(t, found)
274352
}

‎instrumentation/github.com/labstack/echo/otelecho/internal/semconv/util.go

+7
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,13 @@ func serverClientIP(xForwardedFor string) string {
7676
return xForwardedFor
7777
}
7878

79+
func httpRoute(pattern string) string {
80+
if idx := strings.IndexByte(pattern, '/'); idx >= 0 {
81+
return pattern[idx:]
82+
}
83+
return ""
84+
}
85+
7986
func netProtocol(proto string) (name string, version string) {
8087
name, version, _ = strings.Cut(proto, "/")
8188
switch name {

‎instrumentation/github.com/labstack/echo/otelecho/internal/semconv/v1.20.0.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ type OldHTTPServer struct{}
3838
// If the primary server name is not known, server should be an empty string.
3939
// The req Host will be used to determine the server instead.
4040
func (o OldHTTPServer) RequestTraceAttrs(server string, req *http.Request) []attribute.KeyValue {
41-
return semconvutil.HTTPServerRequest(server, req)
41+
return semconvutil.HTTPServerRequest(server, req, semconvutil.HTTPServerRequestOptions{})
4242
}
4343

4444
func (o OldHTTPServer) NetworkTransportAttr(network string) attribute.KeyValue {

0 commit comments

Comments
 (0)