Skip to content

Commit

Permalink
feat: Sanitise headers to OTel semconv and add as separate fields/att…
Browse files Browse the repository at this point in the history 
…ributes (#296)

## Which problem is this PR solving?
The OTel semantic conventions for headers is to lowercase and replace
`-` with `_` and for each header to be a separate field/attribute. The
logic to do the sanitisation was added in #290 for just the OTel
handler.

This PR moves it to a utility func that both the OTel and libhoney
handlers can use.

## Short description of the changes
- Move sanitisation logic to shared utility func
- Use new func in both otel and libhoney handlers
- Update both handler tests
- Add `-count=1` to test command to clear cache on test runs

## How to verify that this has the expected result
Both libhoney and OTel handlers sanitise headers and add each header as
a separate field / attribute.

---------

Co-authored-by: JamieDanielson <jamieedanielson@gmail.com>
  • Loading branch information
@MikeGoldsmith @JamieDanielson
MikeGoldsmith and JamieDanielson committed Oct 23, 2023
1 parent 34d90a5 commit d8d1fa5
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 101 deletions.
2 changes: 1 addition & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ docker-build:
.PHONY: test
#: run unit tests
test:
go test ./...
go test ./... -count=1

.PHONY: docker-test
#: run unit tests in docker
Expand Down
25 changes: 25 additions & 0 deletions handlers/event_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package handlers

import (
"context"
"fmt"
"net/http"
"strings"
"sync"

"github.com/honeycombio/honeycomb-network-agent/assemblers"
Expand Down Expand Up @@ -31,3 +34,25 @@ func NewEventHandler(config config.Config, cachedK8sClient *utils.CachedK8sClien
}
return eventHandler
}

// sanitizeHeaders takes a map of headers and returns a new map with the keys sanitized
// sanitization involves:
// - converting the keys to lowercase
// - replacing - with _
// - prepending http.request.header or http.response.header
func sanitizeHeaders(isRequest bool, header http.Header) map[string]string {
var prefix string
if isRequest {
prefix = "http.request.header"
} else {
prefix = "http.response.header"
}

headers := make(map[string]string, len(header))
for key, values := range header {
// OTel semantic conventions suggest lowercase, with - characters replaced by _
sanitizedKey := strings.ToLower(strings.Replace(key, "-", "_", -1))
headers[fmt.Sprintf("%s.%s", prefix, sanitizedKey)] = strings.Join(values, ",")
}
return headers
}
8 changes: 6 additions & 2 deletions handlers/libhoney_event_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,9 @@ func (handler *libhoneyEventHandler) addHttpFields(ev *libhoney.Event, event *as
}
// by this point, we've already extracted headers based on HTTP_HEADERS list
// so we can safely add the headers to the event
ev.AddField("http.request.headers", event.Request().Header)
for k, v := range sanitizeHeaders(true, event.Request().Header) {
ev.AddField(k, v)
}
} else {
ev.AddField("name", "HTTP")
ev.AddField("http.request.missing", "no request on this event")
Expand All @@ -205,7 +207,9 @@ func (handler *libhoneyEventHandler) addHttpFields(ev *libhoney.Event, event *as
ev.AddField(string(semconv.HTTPResponseBodySizeKey), event.Response().ContentLength)
// by this point, we've already extracted headers based on HTTP_HEADERS list
// so we can safely add the headers to the event
ev.AddField("http.response.headers", event.Response().Header)
for k, v := range sanitizeHeaders(false, event.Response().Header) {
ev.AddField(k, v)
}
} else {
ev.AddField("http.response.missing", "no response on this event")
}
Expand Down
112 changes: 58 additions & 54 deletions handlers/libhoney_event_handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,33 +91,35 @@ func Test_libhoneyEventHandler_handleEvent(t *testing.T) {
delete(attrs, "meta.response.capture_to_handle.latency_ms")

expectedAttrs := map[string]interface{}{
"name": "HTTP GET",
"client.socket.address": "1.2.3.4",
"server.socket.address": "5.6.7.8",
"meta.stream.ident": "c->s:1->2",
"meta.seqack": int64(0),
"meta.request.packet_count": int(2),
"meta.response.packet_count": int(3),
"http.request.method": "GET",
"url.path": "/check",
"http.request.body.size": int64(42),
"http.request.headers": http.Header{"User-Agent": []string{"teapot-checker/1.0"}, "Connection": []string{"keep-alive"}},
"http.response.headers": http.Header{"Content-Type": []string{"text/plain; charset=utf-8"}, "X-Custom-Header": []string{"tea-party"}},
"http.request.timestamp": requestTimestamp,
"http.response.timestamp": responseTimestamp,
"http.response.status_code": 418,
"http.response.body.size": int64(84),
"error": "HTTP client error",
"duration_ms": int64(3),
"user_agent.original": "teapot-checker/1.0",
"source.k8s.resource.type": "pod",
"source.k8s.namespace.name": "unit-tests",
"source.k8s.pod.name": "src-pod",
"source.k8s.pod.uid": string(srcPod.UID),
"destination.k8s.resource.type": "pod",
"destination.k8s.namespace.name": "unit-tests",
"destination.k8s.pod.name": "dest-pod",
"destination.k8s.pod.uid": string(destPod.UID),
"name": "HTTP GET",
"client.socket.address": "1.2.3.4",
"server.socket.address": "5.6.7.8",
"meta.stream.ident": "c->s:1->2",
"meta.seqack": int64(0),
"meta.request.packet_count": int(2),
"meta.response.packet_count": int(3),
"http.request.method": "GET",
"url.path": "/check",
"http.request.body.size": int64(42),
"http.request.header.user_agent": "teapot-checker/1.0",
"http.request.header.connection": "keep-alive",
"http.response.header.content_type": "text/plain; charset=utf-8",
"http.response.header.x_custom_header": "tea-party",
"http.request.timestamp": requestTimestamp,
"http.response.timestamp": responseTimestamp,
"http.response.status_code": 418,
"http.response.body.size": int64(84),
"error": "HTTP client error",
"duration_ms": int64(3),
"user_agent.original": "teapot-checker/1.0",
"source.k8s.resource.type": "pod",
"source.k8s.namespace.name": "unit-tests",
"source.k8s.pod.name": "src-pod",
"source.k8s.pod.uid": string(srcPod.UID),
"destination.k8s.resource.type": "pod",
"destination.k8s.namespace.name": "unit-tests",
"destination.k8s.pod.name": "dest-pod",
"destination.k8s.pod.uid": string(destPod.UID),
}

assert.Equal(t, expectedAttrs, attrs)
Expand Down Expand Up @@ -240,33 +242,35 @@ func Test_libhoneyEventHandler_handleEvent_routed_to_service(t *testing.T) {
delete(attrs, "meta.response.capture_to_handle.latency_ms")

expectedAttrs := map[string]interface{}{
"name": "HTTP GET",
"client.socket.address": "1.2.3.4",
"server.socket.address": "5.6.7.8",
"meta.stream.ident": "c->s:1->2",
"meta.seqack": int64(0),
"meta.request.packet_count": int(2),
"meta.response.packet_count": int(3),
"http.request.method": "GET",
"url.path": "/check",
"http.request.body.size": int64(42),
"http.request.headers": http.Header{"User-Agent": []string{"teapot-checker/1.0"}, "Connection": []string{"keep-alive"}},
"http.response.headers": http.Header{"Content-Type": []string{"text/plain; charset=utf-8"}, "X-Custom-Header": []string{"tea-party"}},
"http.request.timestamp": requestTimestamp,
"http.response.timestamp": responseTimestamp,
"http.response.status_code": 418,
"http.response.body.size": int64(84),
"error": "HTTP client error",
"duration_ms": int64(3),
"user_agent.original": "teapot-checker/1.0",
"source.k8s.resource.type": "pod",
"source.k8s.namespace.name": "unit-tests",
"source.k8s.pod.name": "src-pod",
"source.k8s.pod.uid": string(srcPod.UID),
"destination.k8s.resource.type": "service",
"destination.k8s.namespace.name": "unit-tests",
"destination.k8s.service.name": "dest-service",
"destination.k8s.service.uid": string(destService.UID),
"name": "HTTP GET",
"client.socket.address": "1.2.3.4",
"server.socket.address": "5.6.7.8",
"meta.stream.ident": "c->s:1->2",
"meta.seqack": int64(0),
"meta.request.packet_count": int(2),
"meta.response.packet_count": int(3),
"http.request.method": "GET",
"url.path": "/check",
"http.request.body.size": int64(42),
"http.request.header.user_agent": "teapot-checker/1.0",
"http.request.header.connection": "keep-alive",
"http.response.header.content_type": "text/plain; charset=utf-8",
"http.response.header.x_custom_header": "tea-party",
"http.request.timestamp": requestTimestamp,
"http.response.timestamp": responseTimestamp,
"http.response.status_code": 418,
"http.response.body.size": int64(84),
"error": "HTTP client error",
"duration_ms": int64(3),
"user_agent.original": "teapot-checker/1.0",
"source.k8s.resource.type": "pod",
"source.k8s.namespace.name": "unit-tests",
"source.k8s.pod.name": "src-pod",
"source.k8s.pod.uid": string(srcPod.UID),
"destination.k8s.resource.type": "service",
"destination.k8s.namespace.name": "unit-tests",
"destination.k8s.service.name": "dest-service",
"destination.k8s.service.uid": string(destService.UID),
}

assert.Equal(t, expectedAttrs, attrs)
Expand Down
15 changes: 2 additions & 13 deletions handlers/otel_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"net/http"
"net/url"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -234,19 +233,9 @@ func (handler *otelHandler) getEventStartEndTimestamps(event assemblers.Event) (

// headerToAttributes converts a http.Header into a slice of OpenTelemetry attributes
func headerToAttributes(isRequest bool, header http.Header) []attribute.KeyValue {
var prefix string
if isRequest {
prefix = "http.request.header"
} else {
prefix = "http.response.header"
}
attrs := []attribute.KeyValue{}
for key, val := range header {
// semantic conventions suggest lowercase, with - characters replaced by _
semconvKey := strings.ToLower(strings.Replace(key, "-", "_", -1))
for _, v := range val {
attrs = append(attrs, attribute.String(fmt.Sprintf("%s.%s", prefix, semconvKey), v))
}
for key, val := range sanitizeHeaders(isRequest, header) {
attrs = append(attrs, attribute.String(key, val))
}
return attrs
}
34 changes: 3 additions & 31 deletions handlers/otel_handler_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
package handlers

import (
"net/http"
"testing"
"time"

"github.com/honeycombio/honeycomb-network-agent/assemblers"
"github.com/stretchr/testify/assert"
"go.opentelemetry.io/otel/attribute"
)
Expand All @@ -14,45 +12,19 @@ import (
func TestHeaderToAttributes(t *testing.T) {
requestTimestamp := time.Now()
responseTimestamp := requestTimestamp.Add(3 * time.Millisecond)
event := createTestOtelEvent(requestTimestamp, responseTimestamp)

reqAttrs := (headerToAttributes(true, event.Request().Header))
event := createTestHttpEvent(requestTimestamp, responseTimestamp)

reqAttrs := headerToAttributes(true, event.Request().Header)
expectedReqAttrs := []attribute.KeyValue{
attribute.String("http.request.header.user_agent", "teapot-checker/1.0"),
attribute.String("http.request.header.connection", "keep-alive"),
}

assert.Equal(t, expectedReqAttrs, reqAttrs)

resAttrs := (headerToAttributes(false, event.Response().Header))
resAttrs := headerToAttributes(false, event.Response().Header)
expectedResAttrs := []attribute.KeyValue{
attribute.String("http.response.header.content_type", "text/plain; charset=utf-8"),
attribute.String("http.response.header.x_custom_header", "tea-party"),
}
assert.Equal(t, expectedResAttrs, resAttrs)
}

func createTestOtelEvent(requestTimestamp, responseTimestamp time.Time) *assemblers.HttpEvent {
return assemblers.NewHttpEvent(
"c->s:1->2",
0,
requestTimestamp,
responseTimestamp,
2,
3,
"1.2.3.4",
"5.6.7.8",
&http.Request{
Method: "GET",
RequestURI: "/check?teapot=true",
ContentLength: 42,
Header: http.Header{"User-Agent": []string{"teapot-checker/1.0"}, "Connection": []string{"keep-alive"}},
},
&http.Response{
StatusCode: 418,
ContentLength: 84,
Header: http.Header{"Content-Type": []string{"text/plain; charset=utf-8"}, "X-Custom-Header": []string{"tea-party"}},
},
)
}

0 comments on commit d8d1fa5

Please sign in to comment.