fix(metric, log): merge explicit resource with environment variables

fixes #5764
open-telemetry · Sep 4, 2024 · 2d5dcda · 2d5dcda
1 parent 48fedfa
commit 2d5dcda
Show file tree

Hide file tree

Showing 7 changed files with 140 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 - Fix memory leak in the global `MeterProvider` when identical instruments are repeatedly created. (#5754)
 - Fix panic instruments creation when setting meter provider. (#5758)
+- Fix metric config WithResource function and LoggerProvider#newProviderConfig to merge provided resource with resource information from the environment. (#5773)
 
 ### Removed
 

diff --git a/sdk/log/go.mod b/sdk/log/go.mod
@@ -5,6 +5,7 @@ go 1.22
 require (
 	github.com/go-logr/logr v1.4.2
 	github.com/go-logr/stdr v1.2.2
+	github.com/google/go-cmp v0.6.0
 	github.com/stretchr/testify v1.9.0
 	go.opentelemetry.io/otel v1.29.0
 	go.opentelemetry.io/otel/log v0.5.0

diff --git a/sdk/log/provider.go b/sdk/log/provider.go
@@ -9,6 +9,7 @@ import (
 	"sync"
 	"sync/atomic"
 
+	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/internal/global"
 	"go.opentelemetry.io/otel/log"
 	"go.opentelemetry.io/otel/log/embedded"
@@ -41,6 +42,12 @@ func newProviderConfig(opts []LoggerProviderOption) providerConfig {
 
 	if c.resource == nil {
 		c.resource = resource.Default()
+	} else {
+		var err error
+		c.resource, err = resource.Merge(resource.Environment(), c.resource)
+		if err != nil {
+			otel.Handle(err)
+		}
 	}
 
 	c.attrCntLim = c.attrCntLim.Resolve(

diff --git a/sdk/log/provider_test.go b/sdk/log/provider_test.go
@@ -12,6 +12,7 @@ import (
 
 	"github.com/go-logr/logr"
 	"github.com/go-logr/logr/testr"
+	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -20,10 +21,13 @@ import (
 	"go.opentelemetry.io/otel/internal/global"
 	"go.opentelemetry.io/otel/log"
 	"go.opentelemetry.io/otel/log/noop"
+	ottest "go.opentelemetry.io/otel/sdk/internal/internaltest"
 	"go.opentelemetry.io/otel/sdk/log/internal/x"
 	"go.opentelemetry.io/otel/sdk/resource"
 )
 
+const envVarResourceAttributes = "OTEL_RESOURCE_ATTRIBUTES"
+
 type processor struct {
 	Name string
 	Err  error
@@ -172,6 +176,65 @@ func TestNewLoggerProviderConfiguration(t *testing.T) {
 	}
 }
 
+func mergeResource(t *testing.T, r1, r2 *resource.Resource) *resource.Resource {
+	r, err := resource.Merge(r1, r2)
+	assert.NoError(t, err)
+	return r
+}
+
+func TestWithResource(t *testing.T) {
+	store, err := ottest.SetEnvVariables(map[string]string{
+		envVarResourceAttributes: "key=value,rk5=7",
+	})
+	require.NoError(t, err)
+	defer func() { require.NoError(t, store.Restore()) }()
+
+	cases := []struct {
+		name    string
+		options []LoggerProviderOption
+		want    *resource.Resource
+		msg     string
+	}{
+		{
+			name:    "explicitly empty resource",
+			options: []LoggerProviderOption{WithResource(resource.Empty())},
+			want:    resource.Environment(),
+		},
+		{
+			name:    "uses default if no resource option",
+			options: []LoggerProviderOption{},
+			want:    resource.Default(),
+		},
+		{
+			name:    "explicit resource",
+			options: []LoggerProviderOption{WithResource(resource.NewSchemaless(attribute.String("rk1", "rv1"), attribute.Int64("rk2", 5)))},
+			want:    mergeResource(t, resource.Environment(), resource.NewSchemaless(attribute.String("rk1", "rv1"), attribute.Int64("rk2", 5))),
+		},
+		{
+			name: "last resource wins",
+			options: []LoggerProviderOption{
+				WithResource(resource.NewSchemaless(attribute.String("rk1", "vk1"), attribute.Int64("rk2", 5))),
+				WithResource(resource.NewSchemaless(attribute.String("rk3", "rv3"), attribute.Int64("rk4", 10))),
+			},
+			want: mergeResource(t, resource.Environment(), resource.NewSchemaless(attribute.String("rk3", "rv3"), attribute.Int64("rk4", 10))),
+		},
+		{
+			name:    "overlapping attributes with environment resource",
+			options: []LoggerProviderOption{WithResource(resource.NewSchemaless(attribute.String("rk1", "rv1"), attribute.Int64("rk5", 10)))},
+			want:    mergeResource(t, resource.Environment(), resource.NewSchemaless(attribute.String("rk1", "rv1"), attribute.Int64("rk5", 10))),
+		},
+	}
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			got := newProviderConfig(tc.options).resource
+			if diff := cmp.Diff(got, tc.want); diff != "" {
+				t.Errorf("WithResource:\n  -got +want %s", diff)
+			}
+		})
+	}
+}
+
 func TestLoggerProviderConcurrentSafe(t *testing.T) {
 	const goRoutineN = 10
 

diff --git a/sdk/metric/config.go b/sdk/metric/config.go
@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"sync"
 
+	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/sdk/resource"
 )
 
@@ -103,7 +104,11 @@ func (o optionFunc) apply(conf config) config {
 // go.opentelemetry.io/otel/sdk/resource package will be used.
 func WithResource(res *resource.Resource) Option {
 	return optionFunc(func(conf config) config {
-		conf.res = res
+		var err error
+		conf.res, err = resource.Merge(resource.Environment(), res)
+		if err != nil {
+			otel.Handle(err)
+		}
 		return conf
 	})
 }

diff --git a/sdk/metric/config_test.go b/sdk/metric/config_test.go
@@ -8,9 +8,12 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"go.opentelemetry.io/otel/attribute"
+	ottest "go.opentelemetry.io/otel/sdk/internal/internaltest"
 	"go.opentelemetry.io/otel/sdk/metric/metricdata"
 	"go.opentelemetry.io/otel/sdk/resource"
 )
@@ -25,6 +28,8 @@ type reader struct {
 	shutdownFunc      func(context.Context) error
 }
 
+const envVarResourceAttributes = "OTEL_RESOURCE_ATTRIBUTES"
+
 var _ Reader = (*reader)(nil)
 
 func (r *reader) aggregation(kind InstrumentKind) Aggregation { // nolint:revive  // import-shadow for method scoped by type.
@@ -108,10 +113,63 @@ func TestUnifyMultiError(t *testing.T) {
 	assert.Equal(t, unify(funcs)(context.Background()), target)
 }
 
+func mergeResource(t *testing.T, r1, r2 *resource.Resource) *resource.Resource {
+	r, err := resource.Merge(r1, r2)
+	assert.NoError(t, err)
+	return r
+}
+
 func TestWithResource(t *testing.T) {
-	res := resource.NewSchemaless()
-	c := newConfig([]Option{WithResource(res)})
-	assert.Same(t, res, c.res)
+	store, err := ottest.SetEnvVariables(map[string]string{
+		envVarResourceAttributes: "key=value,rk5=7",
+	})
+	require.NoError(t, err)
+	defer func() { require.NoError(t, store.Restore()) }()
+
+	cases := []struct {
+		name    string
+		options []Option
+		want    *resource.Resource
+		msg     string
+	}{
+		{
+			name:    "explicitly empty resource",
+			options: []Option{WithResource(resource.Empty())},
+			want:    resource.Environment(),
+		},
+		{
+			name:    "uses default if no resource option",
+			options: []Option{},
+			want:    resource.Default(),
+		},
+		{
+			name:    "explicit resource",
+			options: []Option{WithResource(resource.NewSchemaless(attribute.String("rk1", "rv1"), attribute.Int64("rk2", 5)))},
+			want:    mergeResource(t, resource.Environment(), resource.NewSchemaless(attribute.String("rk1", "rv1"), attribute.Int64("rk2", 5))),
+		},
+		{
+			name: "last resource wins",
+			options: []Option{
+				WithResource(resource.NewSchemaless(attribute.String("rk1", "vk1"), attribute.Int64("rk2", 5))),
+				WithResource(resource.NewSchemaless(attribute.String("rk3", "rv3"), attribute.Int64("rk4", 10))),
+			},
+			want: mergeResource(t, resource.Environment(), resource.NewSchemaless(attribute.String("rk3", "rv3"), attribute.Int64("rk4", 10))),
+		},
+		{
+			name:    "overlapping attributes with environment resource",
+			options: []Option{WithResource(resource.NewSchemaless(attribute.String("rk1", "rv1"), attribute.Int64("rk5", 10)))},
+			want:    mergeResource(t, resource.Environment(), resource.NewSchemaless(attribute.String("rk1", "rv1"), attribute.Int64("rk5", 10))),
+		},
+	}
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			got := newConfig(tc.options).res
+			if diff := cmp.Diff(got, tc.want); diff != "" {
+				t.Errorf("WithResource:\n  -got +want %s", diff)
+			}
+		})
+	}
 }
 
 func TestWithReader(t *testing.T) {

diff --git a/sdk/metric/go.mod b/sdk/metric/go.mod
@@ -5,6 +5,7 @@ go 1.22
 require (
 	github.com/go-logr/logr v1.4.2
 	github.com/go-logr/stdr v1.2.2
+	github.com/google/go-cmp v0.6.0
 	github.com/stretchr/testify v1.9.0
 	go.opentelemetry.io/otel v1.29.0
 	go.opentelemetry.io/otel/metric v1.29.0