Skip to content

Commit

Permalink
Fix default merging of resource attributes from environment variable (#…
Browse files Browse the repository at this point in the history
…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 <a9@aneurysm9.com>

* 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 <a9@aneurysm9.com>
  • Loading branch information
dhruv-vora and Aneurysm9 authored Apr 21, 2021
1 parent 96c5e4b commit 0032bd6
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions sdk/metric/controller/basic/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
20 changes: 16 additions & 4 deletions sdk/metric/controller/basic/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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())

Expand All @@ -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
Expand All @@ -66,23 +75,26 @@ 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,
wanted: resource.Default().Encoded(attribute.DefaultEncoder())},
{
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) {
Expand Down
13 changes: 12 additions & 1 deletion sdk/resource/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {

This comment has been minimized.

Copy link
@seh

seh May 20, 2021

Contributor

Perhaps you meant the following instead:

 	if err != nil {

See #1934.

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.
Expand Down
4 changes: 1 addition & 3 deletions sdk/trace/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
19 changes: 16 additions & 3 deletions sdk/trace/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ import (
"go.opentelemetry.io/otel/sdk/resource"
)

const envVar = "OTEL_RESOURCE_ATTRIBUTES"

type storingHandler struct {
errs []error
}
Expand Down Expand Up @@ -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
Expand All @@ -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",
Expand All @@ -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 {
Expand Down

0 comments on commit 0032bd6

Please sign in to comment.