From 0032bd6499c650801eb69fcb020e4177c80116b9 Mon Sep 17 00:00:00 2001 From: Dhruv Vora <32409412+dhruv-vora@users.noreply.github.com> Date: Wed, 21 Apr 2021 11:53:12 -0700 Subject: [PATCH] Fix default merging of resource attributes from environment variable (#1785) * updated controller to merge default resource with environment resource * updated TracerProvider to merge default resource with environment resource * Added Changelog entry * Added resource.Environment(), modified resource.Default() for environment vairable and WithResource() configuration for TracerProvider and Controller * Update CHANGELOG.md Co-authored-by: Anthony Mirabella * Moved environment detector to defaultResource initialization, added test cases * Changes to default resource initialization * made changes to the test cases * added merging of resource with environment resource Co-authored-by: Anthony Mirabella --- CHANGELOG.md | 1 + sdk/metric/controller/basic/config.go | 6 ++++-- .../controller/basic/controller_test.go | 20 +++++++++++++++---- sdk/resource/resource.go | 13 +++++++++++- sdk/trace/provider.go | 4 +--- sdk/trace/trace_test.go | 19 +++++++++++++++--- 6 files changed, 50 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae562cbd80f..479d8718488 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added +- - Extract resource attributes from the `OTEL_RESOURCE_ATTRIBUTES` environment variable and merge them with the `resource.Default` resource as well as resources provided to the `TracerProvider` and metric `Controller`. (#1785) - Added Jaeger Environment variables: `OTEL_EXPORTER_JAEGER_AGENT_HOST`, `OTEL_EXPORTER_JAEGER_AGENT_PORT` These environment variables can be used to override Jaeger agent hostname and port (#1752) - The OTLP exporter now has two new convenience functions, `NewExportPipeline` and `InstallNewPipeline`, setup and install the exporter in tracing and metrics pipelines. (#1373) diff --git a/sdk/metric/controller/basic/config.go b/sdk/metric/controller/basic/config.go index 803dd9c9fd8..87be85b9000 100644 --- a/sdk/metric/controller/basic/config.go +++ b/sdk/metric/controller/basic/config.go @@ -64,9 +64,11 @@ type Option interface { Apply(*Config) } -// WithResource sets the Resource configuration option of a Config. +// WithResource sets the Resource configuration option of a Config by merging it +// with the Resource configuration in the environment. func WithResource(r *resource.Resource) Option { - return resourceOption{r} + res := resource.Merge(resource.Environment(), r) + return resourceOption{res} } type resourceOption struct{ *resource.Resource } diff --git a/sdk/metric/controller/basic/controller_test.go b/sdk/metric/controller/basic/controller_test.go index 447b831f8f4..ae4725546cf 100644 --- a/sdk/metric/controller/basic/controller_test.go +++ b/sdk/metric/controller/basic/controller_test.go @@ -24,6 +24,7 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/attribute" + ottest "go.opentelemetry.io/otel/internal/internaltest" "go.opentelemetry.io/otel/metric" export "go.opentelemetry.io/otel/sdk/export/metric" "go.opentelemetry.io/otel/sdk/export/metric/aggregation" @@ -34,6 +35,8 @@ import ( "go.opentelemetry.io/otel/sdk/resource" ) +const envVar = "OTEL_RESOURCE_ATTRIBUTES" + func getMap(t *testing.T, cont *controller.Controller) map[string]float64 { out := processortest.NewOutput(attribute.DefaultEncoder()) @@ -58,6 +61,12 @@ func checkTestContext(t *testing.T, ctx context.Context) { } func TestControllerUsesResource(t *testing.T) { + store, err := ottest.SetEnvVariables(map[string]string{ + envVar: "key=value,T=U", + }) + require.NoError(t, err) + defer func() { require.NoError(t, store.Restore()) }() + cases := []struct { name string options []controller.Option @@ -66,7 +75,7 @@ func TestControllerUsesResource(t *testing.T) { { name: "explicitly empty resource", options: []controller.Option{controller.WithResource(resource.Empty())}, - wanted: ""}, + wanted: resource.Environment().Encoded(attribute.DefaultEncoder())}, { name: "uses default if no resource option", options: nil, @@ -74,15 +83,18 @@ func TestControllerUsesResource(t *testing.T) { { name: "explicit resource", options: []controller.Option{controller.WithResource(resource.NewWithAttributes(attribute.String("R", "S")))}, - wanted: "R=S"}, + wanted: "R=S,T=U,key=value"}, { name: "last resource wins", options: []controller.Option{ controller.WithResource(resource.Default()), controller.WithResource(resource.NewWithAttributes(attribute.String("R", "S"))), }, - wanted: "R=S", - }, + wanted: "R=S,T=U,key=value"}, + { + name: "overlapping attributes with environment resource", + options: []controller.Option{controller.WithResource(resource.NewWithAttributes(attribute.String("T", "V")))}, + wanted: "T=V,key=value"}, } for _, c := range cases { t.Run(fmt.Sprintf("case-%s", c.name), func(t *testing.T) { diff --git a/sdk/resource/resource.go b/sdk/resource/resource.go index a2db66cf3b5..2e5d10151cc 100644 --- a/sdk/resource/resource.go +++ b/sdk/resource/resource.go @@ -40,7 +40,7 @@ var ( otel.Handle(err) } return r - }(Detect(context.Background(), defaultServiceNameDetector{}, TelemetrySDK{})) + }(Detect(context.Background(), defaultServiceNameDetector{}, FromEnv{}, TelemetrySDK{})) ) // NewWithAttributes creates a resource from attrs. If attrs contains @@ -144,6 +144,17 @@ func Default() *Resource { return defaultResource } +// Environment returns an instance of Resource with attributes +// extracted from the OTEL_RESOURCE_ATTRIBUTES environment variable. +func Environment() *Resource { + detector := &FromEnv{} + resource, err := detector.Detect(context.Background()) + if err == nil { + otel.Handle(err) + } + return resource +} + // Equivalent returns an object that can be compared for equality // between two resources. This value is suitable for use as a key in // a map. diff --git a/sdk/trace/provider.go b/sdk/trace/provider.go index ac56af73bba..601c239c0e0 100644 --- a/sdk/trace/provider.go +++ b/sdk/trace/provider.go @@ -262,9 +262,7 @@ func WithSpanProcessor(sp SpanProcessor) TracerProviderOption { // resource.Default() Resource by default. func WithResource(r *resource.Resource) TracerProviderOption { return func(opts *TracerProviderConfig) { - if r != nil { - opts.resource = r - } + opts.resource = resource.Merge(resource.Environment(), r) } } diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 014329219fc..efffde19366 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -43,6 +43,8 @@ import ( "go.opentelemetry.io/otel/sdk/resource" ) +const envVar = "OTEL_RESOURCE_ATTRIBUTES" + type storingHandler struct { errs []error } @@ -1203,6 +1205,12 @@ func TestWithSpanKind(t *testing.T) { } func TestWithResource(t *testing.T) { + store, err := ottest.SetEnvVariables(map[string]string{ + envVar: "key=value,rk5=7", + }) + require.NoError(t, err) + defer func() { require.NoError(t, store.Restore()) }() + cases := []struct { name string options []TracerProviderOption @@ -1212,7 +1220,7 @@ func TestWithResource(t *testing.T) { { name: "explicitly empty resource", options: []TracerProviderOption{WithResource(resource.Empty())}, - want: resource.Empty(), + want: resource.Environment(), }, { name: "uses default if no resource option", @@ -1222,14 +1230,19 @@ func TestWithResource(t *testing.T) { { name: "explicit resource", options: []TracerProviderOption{WithResource(resource.NewWithAttributes(attribute.String("rk1", "rv1"), attribute.Int64("rk2", 5)))}, - want: resource.NewWithAttributes(attribute.String("rk1", "rv1"), attribute.Int64("rk2", 5)), + want: resource.Merge(resource.Environment(), resource.NewWithAttributes(attribute.String("rk1", "rv1"), attribute.Int64("rk2", 5))), }, { name: "last resource wins", options: []TracerProviderOption{ WithResource(resource.NewWithAttributes(attribute.String("rk1", "vk1"), attribute.Int64("rk2", 5))), WithResource(resource.NewWithAttributes(attribute.String("rk3", "rv3"), attribute.Int64("rk4", 10)))}, - want: resource.NewWithAttributes(attribute.String("rk3", "rv3"), attribute.Int64("rk4", 10)), + want: resource.Merge(resource.Environment(), resource.NewWithAttributes(attribute.String("rk3", "rv3"), attribute.Int64("rk4", 10))), + }, + { + name: "overlapping attributes with environment resource", + options: []TracerProviderOption{WithResource(resource.NewWithAttributes(attribute.String("rk1", "rv1"), attribute.Int64("rk5", 10)))}, + want: resource.Merge(resource.Environment(), resource.NewWithAttributes(attribute.String("rk1", "rv1"), attribute.Int64("rk5", 10))), }, } for _, tc := range cases {