Skip to content

Commit

Permalink
feat(internal/metadataproviders/system): do not return empty host.id (#…
Browse files Browse the repository at this point in the history
…24239)

**Description:**

Do not return empty host ID

**Link to tracking Issue:** #24230

**Testing:** Unit tests

**Documentation:** N/A

---------

Signed-off-by: Dominik Rosiek <drosiek@sumologic.com>
Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
  • Loading branch information
sumo-drosiek and mx-psi authored Jul 14, 2023
1 parent e426c2d commit 7e6145c
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 4 deletions.
20 changes: 20 additions & 0 deletions .chloggen/drosiek-host-id.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# 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: resourcedetectionprocessor

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "avoid returning empty host.id on the `system` detector"

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [24230]

# (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:
2 changes: 1 addition & 1 deletion internal/metadataproviders/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
github.com/hashicorp/consul/api v1.22.0
github.com/stretchr/testify v1.8.4
go.opentelemetry.io/collector/semconv v0.81.0
go.opentelemetry.io/otel v1.16.0
go.opentelemetry.io/otel/sdk v1.16.0
)

Expand Down Expand Up @@ -41,7 +42,6 @@ require (
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
github.com/stretchr/objx v0.5.0 // indirect
go.opentelemetry.io/otel v1.16.0 // indirect
go.opentelemetry.io/otel/metric v1.16.0 // indirect
go.opentelemetry.io/otel/trace v1.16.0 // indirect
golang.org/x/exp v0.0.0-20230321023759-10a507213a29 // indirect
Expand Down
12 changes: 9 additions & 3 deletions internal/metadataproviders/system/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,11 @@ type Provider interface {

type systemMetadataProvider struct {
nameInfoProvider
newResource func(context.Context, ...resource.Option) (*resource.Resource, error)
}

func NewProvider() Provider {
return systemMetadataProvider{nameInfoProvider: newNameInfoProvider()}
return systemMetadataProvider{nameInfoProvider: newNameInfoProvider(), newResource: resource.New}
}

func (systemMetadataProvider) OSType() (string, error) {
Expand Down Expand Up @@ -120,7 +121,7 @@ func (p systemMetadataProvider) reverseLookup(ipAddresses []string) (string, err
}

func (p systemMetadataProvider) HostID(ctx context.Context) (string, error) {
res, err := resource.New(ctx,
res, err := p.newResource(ctx,
resource.WithHostID(),
)

Expand All @@ -132,7 +133,12 @@ func (p systemMetadataProvider) HostID(ctx context.Context) (string, error) {

for iter.Next() {
if iter.Attribute().Key == conventions.AttributeHostID {
return iter.Attribute().Value.Emit(), nil
v := iter.Attribute().Value.Emit()

if v == "" {
return "", fmt.Errorf("empty host id")
}
return v, nil
}
}

Expand Down
62 changes: 62 additions & 0 deletions internal/metadataproviders/system/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@
package system

import (
"context"
"fmt"
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/sdk/resource"
)

func TestLookupCNAME_Linux(t *testing.T) {
Expand Down Expand Up @@ -40,6 +43,65 @@ func TestReverseLookupHost_Windows(t *testing.T) {
assert.Equal(t, "my-windows-vm.abcdefghijklmnopqrstuvwxyz.xx.internal.foo.net", fqdn)
}

func TestHostID(t *testing.T) {
tests := []struct {
name string
resValue string
resError error
fakeResource func(context.Context, ...resource.Option) (*resource.Resource, error)
err string
expected string
}{
{
name: "valid host.id",
resValue: "my-linux-host-id",
resError: nil,
expected: "my-linux-host-id",
},
{
name: "empty host.id",
resValue: "",
resError: nil,
err: "failed to obtain host id",
expected: "",
},
{
name: "error",
resValue: "",
resError: fmt.Errorf("some error"),
err: "failed to obtain host id: some error",
expected: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
p := fakeLinuxSystemMetadataProvider()
p.newResource = func(ctx context.Context, o ...resource.Option) (*resource.Resource, error) {
if tt.resValue == "" {
return resource.NewSchemaless(), tt.resError
}

v := attribute.KeyValue{
Key: "host.id",
Value: attribute.StringValue(tt.resValue),
}
ret := resource.NewSchemaless(v)

return ret, tt.resError
}
id, err := p.HostID(context.Background())

if tt.err != "" {
require.EqualError(t, err, tt.err)
return
}

require.NoError(t, err)
assert.Equal(t, tt.expected, id)
})
}
}

func fakeLinuxSystemMetadataProvider() *systemMetadataProvider {
return &systemMetadataProvider{
nameInfoProvider: fakeLinuxNameInfoProvider(),
Expand Down

0 comments on commit 7e6145c

Please sign in to comment.