Repository: honeycombio/refinery (go)

Issue:
Record original user agent in a meta field
Refinery drops the incoming user-agent fields on requests. It's useful to know what was sending the original telemetry, so we should add a metadata field to telemetry that is `meta.refinery.incoming_user_agent`, containing the text of the user agent for http requests, and the equivalent instrumentation library information from OTLP requests.



Pull request:
feat: Record original user agent for spans and logs
## Which problem is this PR solving?

Refinery loses the original user agent of the sending application because it aggregates spans until a trace decision is made. This PR records the original user agent for both HTTP and gRPC requests to the event, batch and OTLP trace/log endpoints in a new metadata field `meta.refinery.incoming_user_agent`.

- Closes #1356

## Short description of the changes
- Update event, batch, OTLP trace/logs endpoints to record original user agent in new field
- Update tests to verify events have the new field set
- Add defer for bad API key test to restore allowed keys so subsequent tests can pass


Patch:
diff --git a/route/otlp_logs.go b/route/otlp_logs.go
index 371e1f1..3b2c2b2 100644
--- a/route/otlp_logs.go
+++ b/route/otlp_logs.go
@@ -38,7 +38,7 @@ func (r *Router) postOTLPLogs(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 
-	if err := r.processOTLPRequest(req.Context(), result.Batches, keyToUse); err != nil {
+	if err := r.processOTLPRequest(req.Context(), result.Batches, keyToUse, ri.UserAgent); err != nil {
 		r.handleOTLPFailureResponse(w, req, huskyotlp.OTLPError{Message: err.Error(), HTTPStatusCode: http.StatusInternalServerError})
 		return
 	}
@@ -74,7 +74,7 @@ func (l *LogsServer) Export(ctx context.Context, req *collectorlogs.ExportLogsSe
 		return nil, huskyotlp.AsGRPCError(err)
 	}
 
-	if err := l.router.processOTLPRequest(ctx, result.Batches, keyToUse); err != nil {
+	if err := l.router.processOTLPRequest(ctx, result.Batches, keyToUse, ri.UserAgent); err != nil {
 		return nil, huskyotlp.AsGRPCError(err)
 	}
 
diff --git a/route/otlp_trace.go b/route/otlp_trace.go
index c0137b0..8fcbdaa 100644
--- a/route/otlp_trace.go
+++ b/route/otlp_trace.go
@@ -38,7 +38,7 @@ func (r *Router) postOTLPTrace(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 
-	if err := r.processOTLPRequest(req.Context(), result.Batches, keyToUse); err != nil {
+	if err := r.processOTLPRequest(req.Context(), result.Batches, keyToUse, ri.UserAgent); err != nil {
 		r.handleOTLPFailureResponse(w, req, huskyotlp.OTLPError{Message: err.Error(), HTTPStatusCode: http.StatusInternalServerError})
 		return
 	}
@@ -74,7 +74,7 @@ func (t *TraceServer) Export(ctx context.Context, req *collectortrace.ExportTrac
 		return nil, huskyotlp.AsGRPCError(err)
 	}
 
-	if err := t.router.processOTLPRequest(ctx, result.Batches, keyToUse); err != nil {
+	if err := t.router.processOTLPRequest(ctx, result.Batches, keyToUse, ri.UserAgent); err != nil {
 		return nil, huskyotlp.AsGRPCError(err)
 	}
 
diff --git a/route/route.go b/route/route.go
index 854c1b7..75dc903 100644
--- a/route/route.go
+++ b/route/route.go
@@ -383,6 +383,7 @@ func (r *Router) event(w http.ResponseWriter, req *http.Request) {
 		r.handlerReturnWithError(w, ErrReqToEvent, err)
 		return
 	}
+	addIncomingUserAgent(ev, getUserAgentFromRequest(req))
 
 	reqID := req.Context().Value(types.RequestIDContextKey{})
 	err = r.processEvent(ev, reqID)
@@ -478,6 +479,7 @@ func (r *Router) batch(w http.ResponseWriter, req *http.Request) {
 		r.handlerReturnWithError(w, ErrReqToEvent, err)
 	}
 
+	userAgent := getUserAgentFromRequest(req)
 	batchedResponses := make([]*BatchResponse, 0, len(batchedEvents))
 	for _, bev := range batchedEvents {
 		ev := &types.Event{
@@ -491,6 +493,7 @@ func (r *Router) batch(w http.ResponseWriter, req *http.Request) {
 			Data:        bev.Data,
 		}
 
+		addIncomingUserAgent(ev, userAgent)
 		err = r.processEvent(ev, reqID)
 
 		var resp BatchResponse
@@ -517,7 +520,8 @@ func (r *Router) batch(w http.ResponseWriter, req *http.Request) {
 func (router *Router) processOTLPRequest(
 	ctx context.Context,
 	batches []huskyotlp.Batch,
-	apiKey string) error {
+	apiKey string,
+	incomingUserAgent string) error {
 
 	var requestID types.RequestIDContextKey
 	apiHost := router.Config.GetHoneycombAPI()
@@ -540,6 +544,7 @@ func (router *Router) processOTLPRequest(
 				Timestamp:   ev.Timestamp,
 				Data:        ev.Attributes,
 			}
+			addIncomingUserAgent(event, incomingUserAgent)
 			if err = router.processEvent(event, requestID); err != nil {
 				router.Logger.Error().Logf("Error processing event: " + err.Error())
 			}
@@ -1052,3 +1057,13 @@ func extractTraceID(traceIdFieldNames []string, ev *types.Event) string {
 
 	return ""
 }
+
+func getUserAgentFromRequest(req *http.Request) string {
+	return req.Header.Get("User-Agent")
+}
+
+func addIncomingUserAgent(ev *types.Event, userAgent string) {
+	if userAgent != "" {
+		ev.Data["meta.refinery.incoming_user_agent"] = userAgent
+	}
+}


Test patch:
diff --git a/app/app_test.go b/app/app_test.go
index b36f6c1..57a3be7 100644
--- a/app/app_test.go
+++ b/app/app_test.go
@@ -10,6 +10,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"runtime"
 	"strconv"
 	"strings"
 	"sync"
@@ -27,6 +28,7 @@ import (
 
 	"github.com/honeycombio/libhoney-go"
 	"github.com/honeycombio/libhoney-go/transmission"
+	"github.com/honeycombio/libhoney-go/version"
 	"github.com/honeycombio/refinery/collect"
 	"github.com/honeycombio/refinery/config"
 	"github.com/honeycombio/refinery/internal/health"
@@ -403,6 +405,7 @@ func TestPeerRouting(t *testing.T) {
 			"field10":                            float64(10),
 			"long":                               "this is a test of the emergency broadcast system",
 			"meta.refinery.original_sample_rate": uint(2),
+			"meta.refinery.incoming_user_agent":  getLibhoneyUserAgent(),
 			"foo":                                "bar",
 		},
 		Metadata: map[string]any{
@@ -535,6 +538,7 @@ func TestEventsEndpoint(t *testing.T) {
 				"trace.trace_id":                     "1",
 				"foo":                                "bar",
 				"meta.refinery.original_sample_rate": uint(10),
+				"meta.refinery.incoming_user_agent":  getLibhoneyUserAgent(),
 			},
 			Metadata: map[string]any{
 				"api_host":    "http://api.honeycomb.io",
@@ -582,6 +586,7 @@ func TestEventsEndpoint(t *testing.T) {
 				"trace.trace_id":                     "1",
 				"foo":                                "bar",
 				"meta.refinery.original_sample_rate": uint(10),
+				"meta.refinery.incoming_user_agent":  getLibhoneyUserAgent(),
 			},
 			Metadata: map[string]any{
 				"api_host":    "http://api.honeycomb.io",
@@ -656,6 +661,7 @@ func TestEventsEndpointWithNonLegacyKey(t *testing.T) {
 				"trace.trace_id":                     traceID,
 				"foo":                                "bar",
 				"meta.refinery.original_sample_rate": uint(10),
+				"meta.refinery.incoming_user_agent":  getLibhoneyUserAgent(),
 			},
 			Metadata: map[string]any{
 				"api_host":    "http://api.honeycomb.io",
@@ -704,6 +710,7 @@ func TestEventsEndpointWithNonLegacyKey(t *testing.T) {
 				"trace.trace_id":                     traceID,
 				"foo":                                "bar",
 				"meta.refinery.original_sample_rate": uint(10),
+				"meta.refinery.incoming_user_agent":  getLibhoneyUserAgent(),
 			},
 			Metadata: map[string]any{
 				"api_host":    "http://api.honeycomb.io",
@@ -773,7 +780,7 @@ func TestPeerRouting_TraceLocalityDisabled(t *testing.T) {
 			"meta.refinery.min_span":       true,
 			"meta.annotation_type":         types.SpanAnnotationTypeUnknown,
 			"meta.refinery.root":           false,
-			"meta.refinery.span_data_size": 157,
+			"meta.refinery.span_data_size": 175,
 		},
 		Metadata: map[string]any{
 			"api_host":    "http://localhost:17001",
@@ -995,3 +1002,11 @@ func BenchmarkDistributedTraces(b *testing.B) {
 		sender.waitForCount(b, b.N)
 	})
 }
+
+// ideally we should get this from libhoney, but we don't have a way to get it yet
+// this can be removed if libhoney does provide it
+func getLibhoneyUserAgent() string {
+	baseUserAgent := fmt.Sprintf("libhoney-go/%s", version.Version)
+	runtimeInfo := fmt.Sprintf("%s (%s/%s)", strings.Replace(runtime.Version(), "go", "go/", 1), runtime.GOOS, runtime.GOARCH)
+	return fmt.Sprintf("%s %s", baseUserAgent, runtimeInfo)
+}
diff --git a/route/otlp_logs_test.go b/route/otlp_logs_test.go
index 5ba74ab..0a0789a 100644
--- a/route/otlp_logs_test.go
+++ b/route/otlp_logs_test.go
@@ -375,6 +375,63 @@ func TestLogsOTLPHandler(t *testing.T) {
 		assert.Equal(t, 0, len(router.Collector.(*collect.MockCollector).Spans))
 		mockCollector.Flush()
 	})
+
+	t.Run("logs record incoming user agent - gRPC", func(t *testing.T) {
+		md := metadata.New(map[string]string{"x-honeycomb-team": legacyAPIKey, "x-honeycomb-dataset": "ds", "user-agent": "my-user-agent"})
+		ctx := metadata.NewIncomingContext(context.Background(), md)
+
+		req := &collectorlogs.ExportLogsServiceRequest{
+			ResourceLogs: []*logs.ResourceLogs{{
+				ScopeLogs: []*logs.ScopeLogs{{
+					LogRecords: createLogsRecords(),
+				}},
+			}},
+		}
+		_, err := logsServer.Export(ctx, req)
+		if err != nil {
+			t.Errorf(`Unexpected error: %s`, err)
+		}
+		assert.Equal(t, 1, len(mockTransmission.Events))
+		event := mockTransmission.Events[0]
+		assert.Equal(t, "my-user-agent", event.Data["meta.refinery.incoming_user_agent"])
+
+		mockTransmission.Flush()
+		assert.Equal(t, 0, len(router.Collector.(*collect.MockCollector).Spans))
+		mockCollector.Flush()
+	})
+
+	t.Run("logs record incoming user agent - HTTP", func(t *testing.T) {
+		req := &collectorlogs.ExportLogsServiceRequest{
+			ResourceLogs: []*logs.ResourceLogs{{
+				ScopeLogs: []*logs.ScopeLogs{{
+					LogRecords: createLogsRecords(),
+				}},
+			}},
+		}
+		body, err := protojson.Marshal(req)
+		if err != nil {
+			t.Error(err)
+		}
+
+		request, _ := http.NewRequest("POST", "/v1/logs", bytes.NewReader(body))
+		request.Header = http.Header{}
+		request.Header.Set("content-type", "application/json")
+		request.Header.Set("x-honeycomb-team", legacyAPIKey)
+		request.Header.Set("x-honeycomb-dataset", "dataset")
+		request.Header.Set("user-agent", "my-user-agent")
+
+		w := httptest.NewRecorder()
+		router.postOTLPLogs(w, request)
+		assert.Equal(t, w.Code, http.StatusOK)
+
+		assert.Equal(t, 1, len(mockTransmission.Events))
+		event := mockTransmission.Events[0]
+		assert.Equal(t, "my-user-agent", event.Data["meta.refinery.incoming_user_agent"])
+
+		mockTransmission.Flush()
+		assert.Equal(t, 0, len(router.Collector.(*collect.MockCollector).Spans))
+		mockCollector.Flush()
+	})
 }
 
 func createLogsRecords() []*logs.LogRecord {
diff --git a/route/otlp_trace_test.go b/route/otlp_trace_test.go
index 93fbb39..ce31894 100644
--- a/route/otlp_trace_test.go
+++ b/route/otlp_trace_test.go
@@ -484,6 +484,13 @@ func TestOTLPHandler(t *testing.T) {
 			ReceiveKeys:          []string{},
 			AcceptOnlyListedKeys: true,
 		}
+		defer func() {
+			router.Config.(*config.MockConfig).GetAccessKeyConfigVal = config.AccessKeyConfig{
+				ReceiveKeys:          []string{legacyAPIKey},
+				AcceptOnlyListedKeys: false,
+			}
+		}()
+
 		req := &collectortrace.ExportTraceServiceRequest{
 			ResourceSpans: []*trace.ResourceSpans{{
 				ScopeSpans: []*trace.ScopeSpans{{
@@ -498,6 +505,57 @@ func TestOTLPHandler(t *testing.T) {
 		assert.Equal(t, 0, len(mockTransmission.Events))
 		mockTransmission.Flush()
 	})
+
+	t.Run("spans record incoming user agent - gRPC", func(t *testing.T) {
+		md := metadata.New(map[string]string{"x-honeycomb-team": legacyAPIKey, "x-honeycomb-dataset": "ds", "user-agent": "my-user-agent"})
+		ctx := metadata.NewIncomingContext(context.Background(), md)
+
+		req := &collectortrace.ExportTraceServiceRequest{
+			ResourceSpans: []*trace.ResourceSpans{{
+				ScopeSpans: []*trace.ScopeSpans{{
+					Spans: helperOTLPRequestSpansWithStatus(),
+				}},
+			}},
+		}
+		traceServer := NewTraceServer(router)
+		_, err := traceServer.Export(ctx, req)
+		if err != nil {
+			t.Errorf(`Unexpected error: %s`, err)
+		}
+		assert.Equal(t, 2, len(mockTransmission.Events))
+		event := mockTransmission.Events[0]
+		assert.Equal(t, "my-user-agent", event.Data["meta.refinery.incoming_user_agent"])
+		mockTransmission.Flush()
+	})
+
+	t.Run("spans record incoming user agent - HTTP", func(t *testing.T) {
+		req := &collectortrace.ExportTraceServiceRequest{
+			ResourceSpans: []*trace.ResourceSpans{{
+				ScopeSpans: []*trace.ScopeSpans{{
+					Spans: helperOTLPRequestSpansWithStatus(),
+				}},
+			}},
+		}
+		body, err := protojson.Marshal(req)
+		if err != nil {
+			t.Error(err)
+		}
+
+		request, _ := http.NewRequest("POST", "/v1/traces", bytes.NewReader(body))
+		request.Header = http.Header{}
+		request.Header.Set("content-type", "application/json")
+		request.Header.Set("x-honeycomb-team", legacyAPIKey)
+		request.Header.Set("x-honeycomb-dataset", "dataset")
+		request.Header.Set("user-agent", "my-user-agent")
+
+		w := httptest.NewRecorder()
+		router.postOTLPTrace(w, request)
+
+		assert.Equal(t, 2, len(mockTransmission.Events))
+		event := mockTransmission.Events[0]
+		assert.Equal(t, "my-user-agent", event.Data["meta.refinery.incoming_user_agent"])
+		mockTransmission.Flush()
+	})
 }
 
 func helperOTLPRequestSpansWithoutStatus() []*trace.Span {