[service] fix bug with inconsistent resources (#11578)

The tracer and logger provider were instantiating different resources
objects that didn't have a `service.instance.id` attribute. This change
fixes that by instantiating the SDK in the service and passing it to the
factory via the telemetry.Settings struct.

This also removes the duplicate code to instantiate the SDK multiple
times, which will be useful when we move to instantiating MeterProvider
via the config SDK. This bug is blocking the change to bump up the
dependency on the config package.

There are a few alternatives that were considered:
1. could set the resource's service.instance.id on the config object
instead. this seemed messy as it would be editing the config struct and
the instantiation of the SDK would remain duplicated.
2. update the factory to pass in a resource object option, i didn't want
to update the NewFactory func.
3. update the CreateLogger, CreateTracerProvider to receive either a
resource or sdk parameter, both of those seemed incorrect as well.

---------

Signed-off-by: Alex Boten <[email protected]>

.chloggen/codeboten_fix-resource-descrepancy.yaml

@@ -0,0 +1,25 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: service
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: ensure traces and logs emitted by the otel go SDK use the same resource information
+
+# One or more tracking issues or pull requests related to the change
+issues: [11578]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: []

service/attributes.go

@@ -0,0 +1,27 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package service // import "go.opentelemetry.io/collector/service"
+
+import (
+	sdkresource "go.opentelemetry.io/otel/sdk/resource"
+
+	"go.opentelemetry.io/collector/service/telemetry"
+)
+
+func attributes(res *sdkresource.Resource, cfg telemetry.Config) map[string]interface{} {
+	attrs := map[string]interface{}{}
+	for _, r := range res.Attributes() {
+		attrs[string(r.Key)] = r.Value.AsString()
+	}
+
+	for k, v := range cfg.Resource {
+		if v != nil {
+			attrs[k] = *v
+		} else {
+			// the new value is nil, delete the existing key
+			delete(attrs, k)
+		}
+	}
+	return attrs
+}

service/telemetry/attributes_test.go → service/attributes_test.go

@@ -1,56 +1,65 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
-package telemetry // import "go.opentelemetry.io/collector/service/telemetry"
+package service // import "go.opentelemetry.io/collector/service"
 
 import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/collector/component"
+	"go.opentelemetry.io/collector/service/internal/resource"
+	"go.opentelemetry.io/collector/service/telemetry"
 )
 
 func TestAttributes(t *testing.T) {
 	tests := []struct {
 		name           string
-		cfg            Config
+		cfg            telemetry.Config
 		buildInfo      component.BuildInfo
 		wantAttributes map[string]interface{}
 	}{
 		{
 			name:           "no build info and no resource config",
-			cfg:            Config{},
-			wantAttributes: map[string]interface{}{"service.name": "", "service.version": ""},
+			cfg:            telemetry.Config{},
+			wantAttributes: map[string]interface{}{"service.name": "", "service.version": "", "service.instance.id": ""},
 		},
 		{
 			name:           "build info and no resource config",
-			cfg:            Config{},
+			cfg:            telemetry.Config{},
 			buildInfo:      component.BuildInfo{Command: "otelcoltest", Version: "0.0.0-test"},
-			wantAttributes: map[string]interface{}{"service.name": "otelcoltest", "service.version": "0.0.0-test"},
+			wantAttributes: map[string]interface{}{"service.name": "otelcoltest", "service.version": "0.0.0-test", "service.instance.id": ""},
 		},
 		{
 			name:           "no build info and resource config",
-			cfg:            Config{Resource: map[string]*string{"service.name": ptr("resource.name"), "service.version": ptr("resource.version"), "test": ptr("test")}},
-			wantAttributes: map[string]interface{}{"service.name": "resource.name", "service.version": "resource.version", "test": "test"},
+			cfg:            telemetry.Config{Resource: map[string]*string{"service.name": newPtr("resource.name"), "service.version": newPtr("resource.version"), "test": newPtr("test")}},
+			wantAttributes: map[string]interface{}{"service.name": "resource.name", "service.version": "resource.version", "test": "test", "service.instance.id": ""},
 		},
 		{
 			name:           "build info and resource config",
 			buildInfo:      component.BuildInfo{Command: "otelcoltest", Version: "0.0.0-test"},
-			cfg:            Config{Resource: map[string]*string{"service.name": ptr("resource.name"), "service.version": ptr("resource.version"), "test": ptr("test")}},
-			wantAttributes: map[string]interface{}{"service.name": "resource.name", "service.version": "resource.version", "test": "test"},
+			cfg:            telemetry.Config{Resource: map[string]*string{"service.name": newPtr("resource.name"), "service.version": newPtr("resource.version"), "test": newPtr("test")}},
+			wantAttributes: map[string]interface{}{"service.name": "resource.name", "service.version": "resource.version", "test": "test", "service.instance.id": ""},
 		},
 		{
 			name:           "deleting a nil value",
 			buildInfo:      component.BuildInfo{Command: "otelcoltest", Version: "0.0.0-test"},
-			cfg:            Config{Resource: map[string]*string{"service.name": nil, "service.version": ptr("resource.version"), "test": ptr("test")}},
-			wantAttributes: map[string]interface{}{"service.version": "resource.version", "test": "test"},
+			cfg:            telemetry.Config{Resource: map[string]*string{"service.name": nil, "service.version": newPtr("resource.version"), "test": newPtr("test")}},
+			wantAttributes: map[string]interface{}{"service.version": "resource.version", "test": "test", "service.instance.id": ""},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			attrs := attributes(Settings{BuildInfo: tt.buildInfo}, tt.cfg)
-			require.Equal(t, tt.wantAttributes, attrs)
+			attrs := attributes(resource.New(tt.buildInfo, tt.cfg.Resource), tt.cfg)
+			require.Len(t, attrs, len(tt.wantAttributes))
+			for k, v := range tt.wantAttributes {
+				if k == "service.instance.id" {
+					require.NotNil(t, attrs[k])
+				} else {
+					require.Equal(t, v, attrs[k])
+				}
+			}
 		})
 	}
 }

service/service.go

@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"runtime"
 
+	"go.opentelemetry.io/contrib/config"
 	"go.opentelemetry.io/otel/log"
 	"go.opentelemetry.io/otel/metric"
 	"go.opentelemetry.io/otel/metric/noop"
@@ -28,6 +29,7 @@ import (
 	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/processor"
 	"go.opentelemetry.io/collector/receiver"
+	semconv "go.opentelemetry.io/collector/semconv/v1.26.0"
 	"go.opentelemetry.io/collector/service/extensions"
 	"go.opentelemetry.io/collector/service/internal/builders"
 	"go.opentelemetry.io/collector/service/internal/graph"
@@ -118,10 +120,36 @@ func New(ctx context.Context, set Settings, cfg Config) (*Service, error) {
 	res := resource.New(set.BuildInfo, cfg.Telemetry.Resource)
 	pcommonRes := pdataFromSdk(res)
 
+	sch := semconv.SchemaURL
+	cfgRes := config.Resource{
+		SchemaUrl:  &sch,
+		Attributes: attributes(res, cfg.Telemetry),
+	}
+
+	sdk, err := config.NewSDK(
+		config.WithContext(ctx),
+		config.WithOpenTelemetryConfiguration(
+			config.OpenTelemetryConfiguration{
+				LoggerProvider: &config.LoggerProvider{
+					Processors: cfg.Telemetry.Logs.Processors,
+				},
+				TracerProvider: &config.TracerProvider{
+					Processors: cfg.Telemetry.Traces.Processors,
+				},
+				Resource: &cfgRes,
+			},
+		),
+	)
+
+	if err != nil {
+		return nil, fmt.Errorf("failed to create SDK: %w", err)
+	}
+
 	telFactory := telemetry.NewFactory()
 	telset := telemetry.Settings{
 		BuildInfo:  set.BuildInfo,
 		ZapOptions: set.LoggingOptions,
+		SDK:        &sdk,
 	}
 
 	logger, lp, err := telFactory.CreateLogger(ctx, telset, &cfg.Telemetry)

service/service_test.go

@@ -469,6 +469,46 @@ func TestServiceFatalError(t *testing.T) {
 	require.ErrorIs(t, err, assert.AnError)
 }
 
+func TestServiceInvalidTelemetryConfiguration(t *testing.T) {
+	tests := []struct {
+		name    string
+		wantErr error
+		cfg     telemetry.Config
+	}{
+		{
+			name: "log config with processors and invalid config",
+			cfg: telemetry.Config{
+				Logs: telemetry.LogsConfig{
+					Encoding: "console",
+					Processors: []config.LogRecordProcessor{
+						{
+							Batch: &config.BatchLogRecordProcessor{
+								Exporter: config.LogRecordExporter{
+									OTLP: &config.OTLP{},
+								},
+							},
+						},
+					},
+				},
+			},
+			wantErr: errors.New("unsupported protocol \"\""),
+		},
+	}
+	for _, tt := range tests {
+		set := newNopSettings()
+		set.AsyncErrorChannel = make(chan error)
+
+		cfg := newNopConfig()
+		cfg.Telemetry = tt.cfg
+		_, err := New(context.Background(), set, cfg)
+		if tt.wantErr != nil {
+			require.ErrorContains(t, err, tt.wantErr.Error())
+		} else {
+			require.NoError(t, err)
+		}
+	}
+}
+
 func assertResourceLabels(t *testing.T, res pcommon.Resource, expectedLabels map[string]labelValue) {
 	for key, labelValue := range expectedLabels {
 		lookupKey, ok := prometheusToOtelConv[key]

service/telemetry/factory.go

@@ -40,6 +40,7 @@ type Settings struct {
 	BuildInfo         component.BuildInfo
 	AsyncErrorChannel chan error
 	ZapOptions        []zap.Option
+	SDK               *config.SDK
 }
 
 // Factory is factory interface for telemetry.
@@ -66,13 +67,13 @@ type Factory interface {
 // NewFactory creates a new Factory.
 func NewFactory() Factory {
 	return newFactory(createDefaultConfig,
-		withLogger(func(ctx context.Context, set Settings, cfg component.Config) (*zap.Logger, log.LoggerProvider, error) {
+		withLogger(func(_ context.Context, set Settings, cfg component.Config) (*zap.Logger, log.LoggerProvider, error) {
 			c := *cfg.(*Config)
-			return newLogger(ctx, set, c)
+			return newLogger(set, c)
 		}),
-		withTracerProvider(func(ctx context.Context, set Settings, cfg component.Config) (trace.TracerProvider, error) {
+		withTracerProvider(func(_ context.Context, set Settings, cfg component.Config) (trace.TracerProvider, error) {
 			c := *cfg.(*Config)
-			return newTracerProvider(ctx, set, c)
+			return newTracerProvider(set, c)
 		}),
 		withMeterProvider(func(_ context.Context, set Settings, cfg component.Config) (metric.MeterProvider, error) {
 			c := *cfg.(*Config)

service/telemetry/logger.go

@@ -4,18 +4,14 @@
 package telemetry // import "go.opentelemetry.io/collector/service/telemetry"
 
 import (
-	"context"
-
 	"go.opentelemetry.io/contrib/bridges/otelzap"
-	"go.opentelemetry.io/contrib/config"
 	"go.opentelemetry.io/otel/log"
-	semconv "go.opentelemetry.io/otel/semconv/v1.26.0"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
 )
 
 // newLogger creates a Logger and a LoggerProvider from Config.
-func newLogger(ctx context.Context, set Settings, cfg Config) (*zap.Logger, log.LoggerProvider, error) {
+func newLogger(set Settings, cfg Config) (*zap.Logger, log.LoggerProvider, error) {
 	// Copied from NewProductionConfig.
 	zapCfg := &zap.Config{
 		Level:             zap.NewAtomicLevelAt(cfg.Logs.Level),
@@ -42,29 +38,8 @@ func newLogger(ctx context.Context, set Settings, cfg Config) (*zap.Logger, log.
 
 	var lp log.LoggerProvider
 
-	if len(cfg.Logs.Processors) > 0 {
-		sch := semconv.SchemaURL
-		res := config.Resource{
-			SchemaUrl:  &sch,
-			Attributes: attributes(set, cfg),
-		}
-		sdk, err := config.NewSDK(
-			config.WithContext(ctx),
-			config.WithOpenTelemetryConfiguration(
-				config.OpenTelemetryConfiguration{
-					LoggerProvider: &config.LoggerProvider{
-						Processors: cfg.Logs.Processors,
-					},
-					Resource: &res,
-				},
-			),
-		)
-
-		if err != nil {
-			return nil, nil, err
-		}
-
-		lp = sdk.LoggerProvider()
+	if len(cfg.Logs.Processors) > 0 && set.SDK != nil {
+		lp = set.SDK.LoggerProvider()
 
 		logger = logger.WithOptions(zap.WrapCore(func(c zapcore.Core) zapcore.Core {
 			return zapcore.NewTee(

service/telemetry/logger_test.go

@@ -41,24 +41,6 @@ func TestNewLogger(t *testing.T) {
 			},
 			wantCoreType: "*zapcore.ioCore",
 		},
-		{
-			name: "log config with processors and invalid config",
-			cfg: Config{
-				Logs: LogsConfig{
-					Encoding: "console",
-					Processors: []config.LogRecordProcessor{
-						{
-							Batch: &config.BatchLogRecordProcessor{
-								Exporter: config.LogRecordExporter{
-									OTLP: &config.OTLP{},
-								},
-							},
-						},
-					},
-				},
-			},
-			wantErr: errors.New("unsupported protocol \"\""),
-		},
 		{
 			name: "log config with processors",
 			cfg: Config{
@@ -85,7 +67,11 @@ func TestNewLogger(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			l, lp, err := newLogger(context.Background(), Settings{}, tt.cfg)
+			sdk, _ := config.NewSDK(config.WithOpenTelemetryConfiguration(config.OpenTelemetryConfiguration{LoggerProvider: &config.LoggerProvider{
+				Processors: tt.cfg.Logs.Processors,
+			}}))
+
+			l, lp, err := newLogger(Settings{SDK: &sdk}, tt.cfg)
 			if tt.wantErr != nil {
 				require.ErrorContains(t, err, tt.wantErr.Error())
 				require.Nil(t, tt.wantCoreType)