[processor/resourcedetection] Fix system detector not setting attributes (#24670)

Author: dmitryax

.chloggen/res-attr-dont-drop-system-attributes.yaml

@@ -0,0 +1,15 @@
+# Use this changelog template to create an entry for release notes.
+# If your change doesn't affect end users, such as a test fix or a tooling change,
+# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
+
+# 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. filelogreceiver)
+component: processor/resourcedetection
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Do not drop all system attributes if `host.id` cannot be fetched.
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [24669]

processor/resourcedetectionprocessor/internal/system/system.go

@@ -34,10 +34,10 @@ var _ internal.Detector = (*Detector)(nil)
 
 // Detector is a system metadata detector
 type Detector struct {
-	provider        system.Provider
-	logger          *zap.Logger
-	hostnameSources []string
-	rb              *metadata.ResourceBuilder
+	provider system.Provider
+	logger   *zap.Logger
+	cfg      Config
+	rb       *metadata.ResourceBuilder
 }
 
 // NewDetector creates a new system metadata detector
@@ -48,10 +48,10 @@ func NewDetector(p processor.CreateSettings, dcfg internal.DetectorConfig) (inte
 	}
 
 	return &Detector{
-		provider:        system.NewProvider(),
-		logger:          p.Logger,
-		hostnameSources: cfg.HostnameSources,
-		rb:              metadata.NewResourceBuilder(cfg.ResourceAttributes),
+		provider: system.NewProvider(),
+		logger:   p.Logger,
+		cfg:      cfg,
+		rb:       metadata.NewResourceBuilder(cfg.ResourceAttributes),
 	}, nil
 }
 
@@ -64,23 +64,24 @@ func (d *Detector) Detect(ctx context.Context) (resource pcommon.Resource, schem
 		return pcommon.NewResource(), "", fmt.Errorf("failed getting OS type: %w", err)
 	}
 
-	hostID, err := d.provider.HostID(ctx)
-	if err != nil {
-		return pcommon.NewResource(), "", fmt.Errorf("failed getting host ID: %w", err)
-	}
-
 	hostArch, err := d.provider.HostArch()
 	if err != nil {
 		return pcommon.NewResource(), "", fmt.Errorf("failed getting host architecture: %w", err)
 	}
 
-	for _, source := range d.hostnameSources {
+	for _, source := range d.cfg.HostnameSources {
 		getHostFromSource := hostnameSourcesMap[source]
 		hostname, err = getHostFromSource(d)
 		if err == nil {
 			d.rb.SetHostName(hostname)
 			d.rb.SetOsType(osType)
-			d.rb.SetHostID(hostID)
+			if d.cfg.ResourceAttributes.HostID.Enabled {
+				if hostID, hostIDErr := d.provider.HostID(ctx); hostIDErr == nil {
+					d.rb.SetHostID(hostID)
+				} else {
+					d.logger.Warn("failed to get host ID", zap.Error(hostIDErr))
+				}
+			}
 			d.rb.SetHostArch(hostArch)
 			return d.rb.Emit(), conventions.SchemaURL, nil
 		}

processor/resourcedetectionprocessor/internal/system/system_test.go

@@ -102,8 +102,7 @@ func TestDetectFQDNAvailable(t *testing.T) {
 	md.On("HostID").Return("2", nil)
 	md.On("HostArch").Return("amd64", nil)
 
-	detector := &Detector{provider: md, logger: zap.NewNop(), hostnameSources: []string{"dns"},
-		rb: metadata.NewResourceBuilder(allEnabledConfig())}
+	detector := newTestDetector(md, []string{"dns"}, allEnabledConfig())
 	res, schemaURL, err := detector.Detect(context.Background())
 	require.NoError(t, err)
 	assert.Equal(t, conventions.SchemaURL, schemaURL)
@@ -125,15 +124,14 @@ func TestFallbackHostname(t *testing.T) {
 	mdHostname.On("Hostname").Return("hostname", nil)
 	mdHostname.On("FQDN").Return("", errors.New("err"))
 	mdHostname.On("OSType").Return("darwin", nil)
-	mdHostname.On("HostID").Return("3", nil)
 	mdHostname.On("HostArch").Return("amd64", nil)
 
-	detector := &Detector{provider: mdHostname, logger: zap.NewNop(), hostnameSources: []string{"dns", "os"},
-		rb: metadata.NewResourceBuilder(metadata.DefaultResourceAttributesConfig())}
+	detector := newTestDetector(mdHostname, []string{"dns", "os"}, metadata.DefaultResourceAttributesConfig())
 	res, schemaURL, err := detector.Detect(context.Background())
 	require.NoError(t, err)
 	assert.Equal(t, conventions.SchemaURL, schemaURL)
 	mdHostname.AssertExpectations(t)
+	mdHostname.AssertNotCalled(t, "HostID")
 
 	expected := map[string]any{
 		conventions.AttributeHostName: "hostname",
@@ -151,8 +149,7 @@ func TestEnableHostID(t *testing.T) {
 	mdHostname.On("HostID").Return("3", nil)
 	mdHostname.On("HostArch").Return("amd64", nil)
 
-	detector := &Detector{provider: mdHostname, logger: zap.NewNop(), hostnameSources: []string{"dns", "os"},
-		rb: metadata.NewResourceBuilder(allEnabledConfig())}
+	detector := newTestDetector(mdHostname, []string{"dns", "os"}, allEnabledConfig())
 	res, schemaURL, err := detector.Detect(context.Background())
 	require.NoError(t, err)
 	assert.Equal(t, conventions.SchemaURL, schemaURL)
@@ -175,8 +172,7 @@ func TestUseHostname(t *testing.T) {
 	mdHostname.On("HostID").Return("1", nil)
 	mdHostname.On("HostArch").Return("amd64", nil)
 
-	detector := &Detector{provider: mdHostname, logger: zap.NewNop(), hostnameSources: []string{"os"},
-		rb: metadata.NewResourceBuilder(allEnabledConfig())}
+	detector := newTestDetector(mdHostname, []string{"os"}, allEnabledConfig())
 	res, schemaURL, err := detector.Detect(context.Background())
 	require.NoError(t, err)
 	assert.Equal(t, conventions.SchemaURL, schemaURL)
@@ -201,8 +197,7 @@ func TestDetectError(t *testing.T) {
 	mdFQDN.On("HostID").Return("", errors.New("err"))
 	mdFQDN.On("HostArch").Return("amd64", nil)
 
-	detector := &Detector{provider: mdFQDN, logger: zap.NewNop(), hostnameSources: []string{"dns"},
-		rb: metadata.NewResourceBuilder(allEnabledConfig())}
+	detector := newTestDetector(mdFQDN, []string{"dns"}, allEnabledConfig())
 	res, schemaURL, err := detector.Detect(context.Background())
 	assert.Error(t, err)
 	assert.Equal(t, "", schemaURL)
@@ -215,8 +210,7 @@ func TestDetectError(t *testing.T) {
 	mdHostname.On("HostID").Return("", errors.New("err"))
 	mdHostname.On("HostArch").Return("amd64", nil)
 
-	detector = &Detector{provider: mdHostname, logger: zap.NewNop(), hostnameSources: []string{"os"},
-		rb: metadata.NewResourceBuilder(allEnabledConfig())}
+	detector = newTestDetector(mdHostname, []string{"os"}, allEnabledConfig())
 	res, schemaURL, err = detector.Detect(context.Background())
 	assert.Error(t, err)
 	assert.Equal(t, "", schemaURL)
@@ -226,13 +220,38 @@ func TestDetectError(t *testing.T) {
 	mdOSType := &mockMetadata{}
 	mdOSType.On("FQDN").Return("fqdn", nil)
 	mdOSType.On("OSType").Return("", errors.New("err"))
-	mdOSType.On("HostID").Return("", errors.New("err"))
+	mdOSType.On("HostID").Return("1", nil)
 	mdOSType.On("HostArch").Return("amd64", nil)
 
-	detector = &Detector{provider: mdOSType, logger: zap.NewNop(), hostnameSources: []string{"dns"},
-		rb: metadata.NewResourceBuilder(allEnabledConfig())}
+	detector = newTestDetector(mdOSType, []string{"os"}, allEnabledConfig())
 	res, schemaURL, err = detector.Detect(context.Background())
 	assert.Error(t, err)
 	assert.Equal(t, "", schemaURL)
 	assert.True(t, internal.IsEmptyResource(res))
+
+	// Host ID fails. All other attributes should be set.
+	mdHostID := &mockMetadata{}
+	mdHostID.On("Hostname").Return("hostname", nil)
+	mdHostID.On("OSType").Return("linux", nil)
+	mdHostID.On("HostID").Return("", errors.New("err"))
+	mdHostID.On("HostArch").Return("arm64", nil)
+
+	detector = newTestDetector(mdHostID, []string{"os"}, allEnabledConfig())
+	res, schemaURL, err = detector.Detect(context.Background())
+	assert.NoError(t, err)
+	assert.Equal(t, conventions.SchemaURL, schemaURL)
+	assert.Equal(t, map[string]any{
+		conventions.AttributeHostName: "hostname",
+		conventions.AttributeOSType:   "linux",
+		conventions.AttributeHostArch: conventions.AttributeHostArchARM64,
+	}, res.Attributes().AsRaw())
+}
+
+func newTestDetector(mock *mockMetadata, hostnameSources []string, resCfg metadata.ResourceAttributesConfig) *Detector {
+	return &Detector{
+		provider: mock,
+		logger:   zap.NewNop(),
+		cfg:      Config{HostnameSources: hostnameSources, ResourceAttributes: resCfg},
+		rb:       metadata.NewResourceBuilder(resCfg),
+	}
 }