From 05aca23c1941775d1db7fc89243b2c30c970e0a4 Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Wed, 19 Oct 2022 12:13:20 -0400 Subject: [PATCH] Decode values from OTEL_RESOURCE_ATTRIBUTES (#2963) * Decode values from OTEL_RESOURCE_ATTRIBUTES The W3C spec specifies that values must be percent-encoded so when reading the environment variable `OTEL_RESOURCE_ATTRIBUTES` the SDK should decode them. This is done by the `baggage` package, but its behaviour in case of errors is slightly different from the current implementation of the SDK, more specifically in cases where a key is missing a value. The SDK returns a partial resource while the `bagage` package returns nil. This may be considered a breaking change, so this commit fixes the current implementation instead of using `baggage.Parse`. * Add changelog entry for #2963 * Use otel.Handle on OTEL_RESOURCE_ATTRIBUTES decode error * retain original value when decoding fails * docs: update CHANGELOG Co-authored-by: Chester Cheung Co-authored-by: Tyler Yahn --- CHANGELOG.md | 1 + sdk/resource/env.go | 11 ++++++++++- sdk/resource/env_test.go | 22 +++++++++++++++++++--- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4656e968b4a..e1da6302645 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Changed +- Decode urlencoded values from the `OTEL_RESOURCE_ATTRIBUTES` environment variable. (#2963) - `sdktrace.TraceProvider.Shutdown` and `sdktrace.TraceProvider.ForceFlush` to not return error when no processor register. (#3268) - The `"go.opentelemetry.io/otel/exporters/prometheus".New` now also returns an error indicating the failure to register the exporter with Prometheus. (#3239) - The prometheus exporter will no longer try to enumerate the metrics it will send to prometheus on startup. diff --git a/sdk/resource/env.go b/sdk/resource/env.go index eb22d007922..1c349247b0a 100644 --- a/sdk/resource/env.go +++ b/sdk/resource/env.go @@ -17,9 +17,11 @@ package resource // import "go.opentelemetry.io/otel/sdk/resource" import ( "context" "fmt" + "net/url" "os" "strings" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" semconv "go.opentelemetry.io/otel/semconv/v1.12.0" ) @@ -88,7 +90,14 @@ func constructOTResources(s string) (*Resource, error) { invalid = append(invalid, p) continue } - k, v := strings.TrimSpace(field[0]), strings.TrimSpace(field[1]) + k := strings.TrimSpace(field[0]) + v, err := url.QueryUnescape(strings.TrimSpace(field[1])) + if err != nil { + // Retain original value if decoding fails, otherwise it will be + // an empty string. + v = field[1] + otel.Handle(err) + } attrs = append(attrs, attribute.String(k, v)) } var err error diff --git a/sdk/resource/env_test.go b/sdk/resource/env_test.go index c50a0031c9b..62de6729e0a 100644 --- a/sdk/resource/env_test.go +++ b/sdk/resource/env_test.go @@ -43,7 +43,7 @@ func TestDetectOnePair(t *testing.T) { func TestDetectMultiPairs(t *testing.T) { store, err := ottest.SetEnvVariables(map[string]string{ "x": "1", - resourceAttrKey: "key=value, k = v , a= x, a=z", + resourceAttrKey: "key=value, k = v , a= x, a=z, b=c%2Fd", }) require.NoError(t, err) defer func() { require.NoError(t, store.Restore()) }() @@ -51,12 +51,13 @@ func TestDetectMultiPairs(t *testing.T) { detector := &fromEnv{} res, err := detector.Detect(context.Background()) require.NoError(t, err) - assert.Equal(t, res, NewSchemaless( + assert.Equal(t, NewSchemaless( attribute.String("key", "value"), attribute.String("k", "v"), attribute.String("a", "x"), attribute.String("a", "z"), - )) + attribute.String("b", "c/d"), + ), res) } func TestEmpty(t *testing.T) { @@ -102,6 +103,21 @@ func TestMissingKeyError(t *testing.T) { )) } +func TestInvalidPercentDecoding(t *testing.T) { + store, err := ottest.SetEnvVariables(map[string]string{ + resourceAttrKey: "key=%invalid", + }) + require.NoError(t, err) + defer func() { require.NoError(t, store.Restore()) }() + + detector := &fromEnv{} + res, err := detector.Detect(context.Background()) + assert.NoError(t, err) + assert.Equal(t, NewSchemaless( + attribute.String("key", "%invalid"), + ), res) +} + func TestDetectServiceNameFromEnv(t *testing.T) { store, err := ottest.SetEnvVariables(map[string]string{ resourceAttrKey: "key=value,service.name=foo",