From 0d6254460e83ec53247b7374cfa4021c64b5a4b9 Mon Sep 17 00:00:00 2001 From: Robb Kidd Date: Wed, 24 Aug 2022 13:31:32 -0400 Subject: [PATCH] Add more request header information to GRPC handler spans (#341) - Adds the following fields to a GRPC handler span: - request.header.x_forwarded_for - request.header.x_forwarded_proto - request.remote_addr request.remote_addr is retrieved from the GRPC peer which may not be the origin client IP when a proxy is in the middle. The field name used is the equivalent from HTTP instrumentation. - Refactored the existing if-chain to a loop of headers to check and add to span if present. The list of request headers to put on the span is captured in a package variable. It's the closest to a constant map we've got before getting into Go shenanigans. Co-authored-by: Kent Quirk --- wrappers/hnygrpc/grpc.go | 39 ++++++++++++++++++++++++++++------- wrappers/hnygrpc/grpc_test.go | 16 +++++++++++--- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/wrappers/hnygrpc/grpc.go b/wrappers/hnygrpc/grpc.go index 116d63e4..cf840d43 100644 --- a/wrappers/hnygrpc/grpc.go +++ b/wrappers/hnygrpc/grpc.go @@ -2,6 +2,7 @@ package hnygrpc import ( "context" + "net" "reflect" "runtime" @@ -13,9 +14,27 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/metadata" + "google.golang.org/grpc/peer" "google.golang.org/grpc/status" ) +// This is a map of GRPC request header names whose values will be retrieved +// and added to handler spans as fields with the corresponding name. +// +// Header names must be lowercase as the metadata.MD API will have normalized +// incoming headers to lower. +// +// The field names should turn dashes (-) into underscores (_) to follow +// precident in HTTP request headers and the patterns established and in +// naming patterns in OTel attributes for requests. +var headersToFields = map[string]string{ + "content-type": "request.content_type", + ":authority": "request.header.authority", + "user-agent": "request.header.user_agent", + "x-forwarded-for": "request.header.x_forwarded_for", + "x-forwarded-proto": "request.header.x_forwarded_proto", +} + // getMetadataStringValue is a simpler helper method that checks the provided // metadata for a value associated with the provided key. If the value exists, // it is returned. If the value does not exist, an empty string is returned. @@ -72,16 +91,20 @@ func addFields(ctx context.Context, info *grpc.UnaryServerInfo, handler grpc.Una span.AddField("handler.name", handlerName) span.AddField("handler.method", info.FullMethod) - md, ok := metadata.FromIncomingContext(ctx) + pr, ok := peer.FromContext(ctx) if ok { - if val, ok := md["content-type"]; ok { - span.AddField("request.content_type", val[0]) - } - if val, ok := md[":authority"]; ok { - span.AddField("request.header.authority", val[0]) + // if we have an address, put it on the span + if pr.Addr != net.Addr(nil) { + span.AddField("request.remote_addr", pr.Addr.String()) } - if val, ok := md["user-agent"]; ok { - span.AddField("request.header.user_agent", val[0]) + } + + md, ok := metadata.FromIncomingContext(ctx) + if ok { + for headerName, fieldName := range headersToFields { + if val, ok := md[headerName]; ok { + span.AddField(fieldName, val[0]) + } } } } diff --git a/wrappers/hnygrpc/grpc_test.go b/wrappers/hnygrpc/grpc_test.go index 09fd37ce..a82b38b8 100644 --- a/wrappers/hnygrpc/grpc_test.go +++ b/wrappers/hnygrpc/grpc_test.go @@ -78,9 +78,11 @@ func TestUnaryInterceptor(t *testing.T) { beeline.Init(beeline.Config{Client: client}) md := metadata.New(map[string]string{ - "content-type": "application/grpc", - ":authority": "api.honeycomb.io:443", - "user-agent": "testing-is-fun", + "content-type": "application/grpc", + ":authority": "api.honeycomb.io:443", + "user-agent": "testing-is-fun", + "X-Forwarded-For": "10.11.12.13", // headers are Kabob-Title-Case from clients + "X-Forwarded-Proto": "https", }) ctx := metadata.NewIncomingContext(context.Background(), md) handler := func(ctx context.Context, req interface{}) (interface{}, error) { @@ -111,6 +113,14 @@ func TestUnaryInterceptor(t *testing.T) { assert.True(t, ok, "user-agent expected to exist on middleware generated event") assert.Equal(t, "testing-is-fun", userAgent, "user-agent should be set") + xForwardedFor, ok := successfulFields["request.header.x_forwarded_for"] + assert.True(t, ok, "x_forwarded_for expected to exist on middleware generated event") + assert.Equal(t, "10.11.12.13", xForwardedFor, "x_forwarded_for should be set") + + xForwardedProto, ok := successfulFields["request.header.x_forwarded_proto"] + assert.True(t, ok, "x_forwarded_proto expected to exist on middleware generated event") + assert.Equal(t, "https", xForwardedProto, "x_forwarded_proto should be set") + method, ok := successfulFields["handler.method"] assert.True(t, ok, "method name should be set") assert.Equal(t, "test.method", method, "method name should be set")