Skip to content

Commit

Permalink
Decode values from OTEL_RESOURCE_ATTRIBUTES (#2963)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
Co-authored-by: Tyler Yahn <[email protected]>
  • Loading branch information
3 people authored Oct 19, 2022
1 parent 430f558 commit 05aca23
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 10 additions & 1 deletion sdk/resource/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand Down
22 changes: 19 additions & 3 deletions sdk/resource/env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,21 @@ 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()) }()

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) {
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 05aca23

Please sign in to comment.