From 65ea48118fc52264180583f807ff095d60ff4e35 Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Mon, 13 Oct 2025 13:16:54 +0200 Subject: [PATCH] fix(tracing): Fix formatting of `url.URL` attributes. This changes switches to a type casae, which allows us to match on interfaces, particularly the `fmt.Stringer` interface. This is also more efficient, especially for string types. While there, I also changed the behavior of unsupported types: instead of adding an error message in the attribute, the code uses `fmt.Sprint` to format the value and adds a `.error` attribute with the error message instead. Likewise, string attributes that are too long (more than 256 bytes) also get an `.error` attribute explaining what is happening. Issue: gitlab-org/labkit#78 --- tracing/impl/stackdriver_tracer.go | 63 ++++++++++++++++--------- tracing/impl/stackdriver_tracer_test.go | 21 ++++++++- 2 files changed, 61 insertions(+), 23 deletions(-) diff --git a/tracing/impl/stackdriver_tracer.go b/tracing/impl/stackdriver_tracer.go index c705ba4..56cc579 100644 --- a/tracing/impl/stackdriver_tracer.go +++ b/tracing/impl/stackdriver_tracer.go @@ -154,32 +154,51 @@ func (span *adapterSpan) SetOperationName(operationName string) opentracing.Span return span } -func castToAttribute(key string, value interface{}) []trace.Attribute { - switch v := reflect.ValueOf(value); v.Kind() { - case reflect.Bool: - return []trace.Attribute{trace.BoolAttribute(key, v.Bool())} - case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: - return []trace.Attribute{trace.Int64Attribute(key, v.Int())} - case reflect.Float32, reflect.Float64: - return []trace.Attribute{trace.Float64Attribute(key, v.Float())} - case reflect.String: - chunks := chunkString(v.String(), 256) - attrs := []trace.Attribute{} - for i, chunk := range chunks { - var k string - if i == 0 { - k = key - } else { - k = key + "." + strconv.Itoa(i) - } - attrs = append(attrs, trace.StringAttribute(k, chunk)) - } - return attrs +func castToAttribute(key string, value any) []trace.Attribute { + switch v := value.(type) { + case bool: + return []trace.Attribute{trace.BoolAttribute(key, v)} + case int, int8, int16, int32, int64: + return []trace.Attribute{trace.Int64Attribute(key, reflect.ValueOf(v).Int())} + case uint, uint8, uint16, uint32, uint64: + return []trace.Attribute{trace.Int64Attribute(key, int64(reflect.ValueOf(v).Uint()))} + case float32, float64: + return []trace.Attribute{trace.Float64Attribute(key, reflect.ValueOf(v).Float())} + case string: + return newStringAttribute(key, v) + case fmt.Stringer: + return newStringAttribute(key, v.String()) default: - return []trace.Attribute{trace.StringAttribute(key, fmt.Sprintf("castToAttribute not implemented for type %+v", v.Kind()))} + return []trace.Attribute{ + trace.StringAttribute(key, fmt.Sprint(value)), + trace.StringAttribute(key+".error", fmt.Sprintf("unknown type: %T", value)), + } } } +// stackdriver limits attribute values to 256 bytes +const stringChunkSize = 256 + +func newStringAttribute(key, value string) []trace.Attribute { + var attrs []trace.Attribute + + for i, chunk := range chunkString(value, stringChunkSize) { + k := key + if i != 0 { + k = key + "." + strconv.Itoa(i) + } + attrs = append(attrs, trace.StringAttribute(k, chunk)) + } + + if len(attrs) > 1 { + attrs = append(attrs, + trace.StringAttribute(key+".error", + fmt.Sprintf("string is exceeding the maximum attribute length of %d; it has been chunked", stringChunkSize))) + } + + return attrs +} + // stackdriver limits attribute values to 256 bytes // we use runes here, hopefully that's close enough // values get truncated silently, so we can set $key to the full value diff --git a/tracing/impl/stackdriver_tracer_test.go b/tracing/impl/stackdriver_tracer_test.go index 6cb9dc3..66fff08 100644 --- a/tracing/impl/stackdriver_tracer_test.go +++ b/tracing/impl/stackdriver_tracer_test.go @@ -4,6 +4,7 @@ package impl import ( "fmt" + "net/url" "reflect" "strings" "testing" @@ -20,6 +21,8 @@ import ( type unsupportedType struct{} func TestOcSpanAdapterCastToAttribute(t *testing.T) { + const errChunked = "string is exceeding the maximum attribute length of 256; it has been chunked" + tests := []struct { name string key string @@ -87,6 +90,7 @@ func TestOcSpanAdapterCastToAttribute(t *testing.T) { want: []trace.Attribute{ trace.StringAttribute("foo", strings.Repeat("a", 256)), trace.StringAttribute("foo.1", strings.Repeat("a", 1)), + trace.StringAttribute("foo.error", errChunked), }, }, { @@ -96,6 +100,7 @@ func TestOcSpanAdapterCastToAttribute(t *testing.T) { want: []trace.Attribute{ trace.StringAttribute("foo", strings.Repeat("a", 256)), trace.StringAttribute("foo.1", strings.Repeat("a", 255)), + trace.StringAttribute("foo.error", errChunked), }, }, { @@ -105,6 +110,7 @@ func TestOcSpanAdapterCastToAttribute(t *testing.T) { want: []trace.Attribute{ trace.StringAttribute("foo", strings.Repeat("a", 256)), trace.StringAttribute("foo.1", strings.Repeat("a", 256)), + trace.StringAttribute("foo.error", errChunked), }, }, { @@ -115,6 +121,7 @@ func TestOcSpanAdapterCastToAttribute(t *testing.T) { trace.StringAttribute("foo", strings.Repeat("a", 256)), trace.StringAttribute("foo.1", strings.Repeat("a", 256)), trace.StringAttribute("foo.2", strings.Repeat("a", 1)), + trace.StringAttribute("foo.error", errChunked), }, }, { @@ -122,7 +129,19 @@ func TestOcSpanAdapterCastToAttribute(t *testing.T) { key: "foo", value: unsupportedType{}, want: []trace.Attribute{ - trace.StringAttribute("foo", "castToAttribute not implemented for type struct"), + trace.StringAttribute("foo", "{}"), + trace.StringAttribute("foo.error", "unknown type: impl.unsupportedType"), + }, + }, + { + name: "net.url type", + key: "foo", + value: func() *url.URL { + u, _ := url.Parse("https://example.com") + return u + }(), + want: []trace.Attribute{ + trace.StringAttribute("foo", "https://example.com"), }, }, } -- GitLab