Skip to content

Commit

Permalink
feat: Record original user agent for spans and logs (#1358)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
MikeGoldsmith authored Oct 3, 2024
1 parent 0eac5f0 commit ea00691
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 6 deletions.
17 changes: 16 additions & 1 deletion app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"net/http"
"net/http/httptest"
"os"
"runtime"
"strconv"
"strings"
"sync"
Expand All @@ -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"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)
}
4 changes: 2 additions & 2 deletions route/otlp_logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}

Expand Down
57 changes: 57 additions & 0 deletions route/otlp_logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions route/otlp_trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}

Expand Down
58 changes: 58 additions & 0 deletions route/otlp_trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{{
Expand All @@ -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 {
Expand Down
17 changes: 16 additions & 1 deletion route/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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{
Expand All @@ -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
Expand All @@ -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()
Expand All @@ -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())
}
Expand Down Expand Up @@ -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
}
}

0 comments on commit ea00691

Please sign in to comment.