NR-250191 feat: add host.id to connect metadata (#1838)

* NR-250191 feat: add host.id to connect metadata

* make the linter happy

* fix tests

* linters

* remove unnecessary nolint directive

internal/agent/agent.go

@@ -340,7 +340,9 @@ func NewAgent(
 		return nil, err
 	}
 
-	connectSrv := NewIdentityConnectService(connectClient, fpHarvester)
+	connectMetadataHarvester := identityapi.NewMetadataHarvesterDefault()
+
+	connectSrv := NewIdentityConnectService(connectClient, fpHarvester, connectMetadataHarvester)
 
 	// notificationHandler will map ipc messages to functions
 	notificationHandler := ctl.NewNotificationHandlerWithCancellation(ctx.Ctx)

internal/agent/agent_test.go

@@ -1,14 +1,14 @@
 // Copyright 2020 New Relic Corporation. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
-
+//
+// nolint:exhaustruct
 package agent
 
 import (
 	"bytes"
 	context2 "context"
 	"encoding/json"
 	"fmt"
-	"github.com/newrelic/infrastructure-agent/pkg/metrics/types"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -37,6 +37,7 @@ import (
 	"github.com/newrelic/infrastructure-agent/pkg/helpers/fingerprint"
 	"github.com/newrelic/infrastructure-agent/pkg/helpers/metric"
 	"github.com/newrelic/infrastructure-agent/pkg/log"
+	"github.com/newrelic/infrastructure-agent/pkg/metrics/types" //nolint:depguard
 	"github.com/newrelic/infrastructure-agent/pkg/plugins/ids"
 	"github.com/newrelic/infrastructure-agent/pkg/sample"
 	"github.com/newrelic/infrastructure-agent/pkg/sysinfo"
@@ -71,8 +72,11 @@ func newTesting(cfg *config.Config) *Agent {
 		panic(err)
 	}
 
+	metadataHarvester := &identityapi.MetadataHarvesterMock{}
+	metadataHarvester.ShouldHarvest(identityapi.Metadata{})
+
 	registerClient := &identityapi.RegisterClientMock{}
-	connectSrv := NewIdentityConnectService(&MockIdentityConnectClient{}, fpHarvester)
+	connectSrv := NewIdentityConnectService(&MockIdentityConnectClient{}, fpHarvester, metadataHarvester)
 	provideIDs := NewProvideIDs(registerClient, state.NewRegisterSM())
 
 	a, err := New(
@@ -89,7 +93,6 @@ func newTesting(cfg *config.Config) *Agent {
 		fpHarvester,
 		ctl.NewNotificationHandlerWithCancellation(nil),
 	)
-
 	if err != nil {
 		panic(err)
 	}
@@ -143,7 +146,6 @@ func TestIgnoreInventory(t *testing.T) {
 }
 
 func TestServicePidMap(t *testing.T) {
-
 	ctx := NewContext(&config.Config{}, "", testhelpers.NullHostnameResolver, NilIDLookup, matcher)
 	svc, ok := ctx.GetServiceForPid(1)
 	assert.False(t, ok)
@@ -325,16 +327,16 @@ func TestRemoveOutdatedEntities(t *testing.T) {
 	// With a set of registered entities
 	for _, id := range []string{"entity:1", "entity:2", "entity:3"} {
 		agent.registerEntityInventory(entity.NewFromNameWithoutID(id))
-		assert.NoError(t, os.MkdirAll(filepath.Join(dataDir, aPlugin, helpers.SanitizeFileName(id)), 0755))
-		assert.NoError(t, os.MkdirAll(filepath.Join(dataDir, anotherPlugin, helpers.SanitizeFileName(id)), 0755))
+		require.NoError(t, os.MkdirAll(filepath.Join(dataDir, aPlugin, helpers.SanitizeFileName(id)), 0o755))
+		require.NoError(t, os.MkdirAll(filepath.Join(dataDir, anotherPlugin, helpers.SanitizeFileName(id)), 0o755))
 	}
 	// With some entity inventory folders from previous executions
-	assert.NoError(t, os.MkdirAll(filepath.Join(dataDir, aPlugin, "entity4"), 0755))
-	assert.NoError(t, os.MkdirAll(filepath.Join(dataDir, aPlugin, "entity5"), 0755))
-	assert.NoError(t, os.MkdirAll(filepath.Join(dataDir, aPlugin, "entity6"), 0755))
-	assert.NoError(t, os.MkdirAll(filepath.Join(dataDir, anotherPlugin, "entity4"), 0755))
-	assert.NoError(t, os.MkdirAll(filepath.Join(dataDir, anotherPlugin, "entity5"), 0755))
-	assert.NoError(t, os.MkdirAll(filepath.Join(dataDir, anotherPlugin, "entity6"), 0755))
+	require.NoError(t, os.MkdirAll(filepath.Join(dataDir, aPlugin, "entity4"), 0o755))
+	require.NoError(t, os.MkdirAll(filepath.Join(dataDir, aPlugin, "entity5"), 0o755))
+	require.NoError(t, os.MkdirAll(filepath.Join(dataDir, aPlugin, "entity6"), 0o755))
+	require.NoError(t, os.MkdirAll(filepath.Join(dataDir, anotherPlugin, "entity4"), 0o755))
+	require.NoError(t, os.MkdirAll(filepath.Join(dataDir, anotherPlugin, "entity5"), 0o755))
+	require.NoError(t, os.MkdirAll(filepath.Join(dataDir, anotherPlugin, "entity6"), 0o755))
 
 	// When not all the entities reported information during the last period
 	entitiesThatReported := map[string]bool{
@@ -585,11 +587,11 @@ func TestAgent_Run_DontSendInventoryIfFwdOnly(t *testing.T) {
 				SendInterval:      tt.sendInterval,
 			}
 			a := newTesting(cfg)
-			//Give time to at least send one request
+			// Give time to at least send one request
 			ctxTimeout, _ := context2.WithTimeout(a.Context.Ctx, time.Millisecond*10)
 			a.Context.Ctx = ctxTimeout
 
-			//Inventory recording calls
+			// Inventory recording calls
 			snd := &patchSenderCallRecorder{}
 			a.inventories = map[string]*inventoryEntity{"test": {sender: snd}}
 
@@ -913,8 +915,7 @@ func Test_ProcessSampling(t *testing.T) {
 	}
 }
 
-type fakeEventSender struct {
-}
+type fakeEventSender struct{}
 
 func (f fakeEventSender) QueueEvent(_ sample.Event, _ entity.Key) error {
 	return nil
@@ -929,7 +930,7 @@ func (f fakeEventSender) Stop() error {
 }
 
 func TestContext_SendEvent_LogTruncatedEvent(t *testing.T) {
-	//Capture the logs
+	// Capture the logs
 	var output bytes.Buffer
 	log.SetOutput(&output)
 	log.EnableSmartVerboseMode(1000)

internal/agent/connect_service.go

@@ -5,12 +5,13 @@ package agent
 import (
 	goContext "context"
 	"errors"
-	"github.com/newrelic/infrastructure-agent/pkg/config"
+	"fmt"
 	"time"
 
 	"github.com/newrelic/infrastructure-agent/internal/agent/instrumentation"
 	"github.com/newrelic/infrastructure-agent/pkg/backend/backoff"
 	"github.com/newrelic/infrastructure-agent/pkg/backend/identityapi"
+	"github.com/newrelic/infrastructure-agent/pkg/config" //nolint:depguard
 	"github.com/newrelic/infrastructure-agent/pkg/entity"
 	"github.com/newrelic/infrastructure-agent/pkg/helpers/fingerprint"
 	"github.com/newrelic/infrastructure-agent/pkg/log"
@@ -20,17 +21,20 @@ type identityConnectService struct {
 	fingerprintHarvest fingerprint.Harvester
 	lastFingerprint    fingerprint.Fingerprint
 	client             identityapi.IdentityConnectClient
+	metadataHarvester  identityapi.MetadataHarvester
 }
 
 // ErrEmptyEntityID is returned when the entityID is empty.
 var ErrEmptyEntityID = errors.New("the agentID provided is empty. make sure you have connected if this is not expected")
 
 var logger = log.WithComponent("IdentityConnectService")
 
-func NewIdentityConnectService(client identityapi.IdentityConnectClient, fingerprintHarvest fingerprint.Harvester) *identityConnectService {
+//nolint:revive
+func NewIdentityConnectService(client identityapi.IdentityConnectClient, fingerprintHarvest fingerprint.Harvester, metadataHarvester identityapi.MetadataHarvester) *identityConnectService {
 	return &identityConnectService{
 		fingerprintHarvest: fingerprintHarvest,
 		client:             client,
+		metadataHarvester:  metadataHarvester,
 	}
 }
 
@@ -50,7 +54,15 @@ func (ic *identityConnectService) Connect() entity.Identity {
 
 		logger.WithField(config.TracesFieldName, config.FeatureTrace).Tracef("connect request with fingerprint: %+v", f)
 
-		ids, retry, err := ic.client.Connect(f)
+		metatada, err := ic.metadataHarvester.Harvest()
+		if err != nil {
+			logger.WithError(err).Error("metadata harvest failed")
+			time.Sleep(1 * time.Second)
+
+			continue
+		}
+
+		ids, retry, err := ic.client.Connect(f, metatada)
 
 		if !ids.ID.IsEmpty() {
 			logger.
@@ -106,10 +118,15 @@ func (ic *identityConnectService) ConnectUpdate(agentIdn entity.Identity) (entit
 	// fingerprint changed, so let's store it for the next round
 	ic.lastFingerprint = f
 
+	metatada, err := ic.metadataHarvester.Harvest()
+	if err != nil {
+		return agentIdn, fmt.Errorf("failed to harvest metadata: %w", err)
+	}
+
 	var retryBO *backoff.Backoff
 	for {
 		logger.WithField(config.TracesFieldName, config.FeatureTrace).Tracef("connect update request with fingerprint: %+v", f)
-		retry, entityIdn, err := ic.client.ConnectUpdate(agentIdn, f)
+		retry, entityIdn, err := ic.client.ConnectUpdate(agentIdn, f, metatada)
 		if retry.After > 0 {
 			logger.WithField("retryAfter", retry.After).Debug("Connect update retry requested.")
 			retryBO = nil

internal/agent/connect_service_test.go

@@ -1,10 +1,18 @@
 // Copyright 2020 New Relic Corporation. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
+
+//nolint:exhaustruct
 package agent
 
 import (
+	"errors"
+	"regexp"
 	"testing"
 
+	"github.com/newrelic/infrastructure-agent/pkg/log" //nolint:depguard
+	logHelper "github.com/newrelic/infrastructure-agent/test/log"
+	"github.com/sirupsen/logrus"
+
 	"github.com/newrelic/infrastructure-agent/pkg/backend/identityapi"
 
 	"github.com/stretchr/testify/assert"
@@ -14,45 +22,74 @@ import (
 	"github.com/newrelic/infrastructure-agent/pkg/helpers/fingerprint"
 )
 
-var (
-	testEntityId = entity.Identity{ID: 999666333}
-)
+// nolint:gochecknoglobals
+var testEntityID = entity.Identity{ID: 999666333}
 
-type MockIdentityConnectClient struct {
-}
+type MockIdentityConnectClient struct{}
 
-func (icc *MockIdentityConnectClient) Connect(fp fingerprint.Fingerprint) (entity.Identity, backendhttp.RetryPolicy, error) {
+func (icc *MockIdentityConnectClient) Connect(_ fingerprint.Fingerprint, _ identityapi.Metadata) (entity.Identity, backendhttp.RetryPolicy, error) {
 	var retry backendhttp.RetryPolicy
-	return testEntityId, retry, nil
+
+	return testEntityID, retry, nil
 }
 
-func (icc *MockIdentityConnectClient) ConnectUpdate(entityID entity.Identity, fp fingerprint.Fingerprint) (backendhttp.RetryPolicy, entity.Identity, error) {
+func (icc *MockIdentityConnectClient) ConnectUpdate(_ entity.Identity, _ fingerprint.Fingerprint, _ identityapi.Metadata) (backendhttp.RetryPolicy, entity.Identity, error) {
 	var retry backendhttp.RetryPolicy
-	return retry, testEntityId, nil
+
+	return retry, testEntityID, nil
 }
 
-func (icc *MockIdentityConnectClient) Disconnect(entityID entity.ID, state identityapi.DisconnectReason) error {
+func (icc *MockIdentityConnectClient) Disconnect(_ entity.ID, _ identityapi.DisconnectReason) error {
 	return nil
 }
 
 func TestConnect(t *testing.T) {
-	service := NewIdentityConnectService(&MockIdentityConnectClient{}, &fingerprint.MockHarvestor{})
+	metadataHarvester := &identityapi.MetadataHarvesterMock{}
+	defer metadataHarvester.AssertExpectations(t)
+	metadataHarvester.ShouldHarvest(identityapi.Metadata{})
+
+	service := NewIdentityConnectService(&MockIdentityConnectClient{}, &fingerprint.MockHarvestor{}, metadataHarvester)
+
+	assert.Equal(t, testEntityID, service.Connect())
+}
+
+func TestConnectOnMetadataError(t *testing.T) {
+	t.Parallel()
+
+	metadataHarvester := &identityapi.MetadataHarvesterMock{}
+	defer metadataHarvester.AssertExpectations(t)
+	//nolint:goerr113
+	metadataHarvester.ShouldNotHarvest(errors.New("some error"))
+	metadataHarvester.ShouldHarvest(identityapi.Metadata{})
+
+	// WHEN we add a hook to the log to capture the "error" and "fatal" levels
+	hook := logHelper.NewInMemoryEntriesHook([]logrus.Level{logrus.ErrorLevel})
+	log.AddHook(hook)
 
-	assert.Equal(t, testEntityId, service.Connect())
+	service := NewIdentityConnectService(&MockIdentityConnectClient{}, &fingerprint.MockHarvestor{}, metadataHarvester)
+
+	assert.Equal(t, testEntityID, service.Connect())
+	assert.True(t, hook.EntryWithMessageExists(regexp.MustCompile(`metadata harvest failed`)))
 }
 
 func TestConnectUpdate(t *testing.T) {
-	service := NewIdentityConnectService(&MockIdentityConnectClient{}, &fingerprint.MockHarvestor{})
+	metadataHarvester := &identityapi.MetadataHarvesterMock{}
+	defer metadataHarvester.AssertExpectations(t)
+	metadataHarvester.ShouldHarvest(identityapi.Metadata{})
+
+	service := NewIdentityConnectService(&MockIdentityConnectClient{}, &fingerprint.MockHarvestor{}, metadataHarvester)
 	entityIdn, err := service.ConnectUpdate(entity.Identity{ID: 1})
 	assert.NoError(t, err)
-	assert.Equal(t, testEntityId, entityIdn)
+	assert.Equal(t, testEntityID, entityIdn)
 }
 
 func Test_ConnectUpdate_ReturnsSameIDForSameFingerprint(t *testing.T) {
+	metadataHarvester := identityapi.MetadataHarvesterMock{}
+	defer metadataHarvester.AssertExpectations(t)
 	harvester := &fingerprint.MockHarvestor{}
 	mockFingerprint, _ := harvester.Harvest()
 	// explicitly setting null client to make sure we're not calling it IF we have the same fingerprint
-	service := NewIdentityConnectService(nil, harvester)
+	service := NewIdentityConnectService(nil, harvester, &metadataHarvester)
 	service.lastFingerprint = mockFingerprint
 
 	agentIdn := entity.Identity{ID: 1}
@@ -67,13 +104,18 @@ func Test_ConnectUpdate_ReturnsSameDifferentIDForDifferentFingerprint(t *testing
 	mockFingerprint, _ := harvester.Harvest()
 	mockFingerprint.Hostname = "someHostName"
 
-	service := NewIdentityConnectService(&MockIdentityConnectClient{}, harvester)
+	metadataHarvester := identityapi.MetadataHarvesterMock{}
+	defer metadataHarvester.AssertExpectations(t)
+
+	metadataHarvester.ShouldHarvest(identityapi.Metadata{})
+
+	service := NewIdentityConnectService(&MockIdentityConnectClient{}, harvester, &metadataHarvester)
 	service.lastFingerprint = mockFingerprint
 
 	agentIdn := entity.Identity{ID: 1}
 	entityID, err := service.ConnectUpdate(agentIdn)
 
 	assert.NoError(t, err)
-	assert.Equal(t, testEntityId, entityID)
-	assert.NotEqual(t, testEntityId, agentIdn)
+	assert.Equal(t, testEntityID, entityID)
+	assert.NotEqual(t, testEntityID, agentIdn)
 }

pkg/backend/identityapi/connect_client.go

@@ -33,8 +33,8 @@ var (
 var ilog = log.WithComponent("IdentityConnectClient")
 
 type IdentityConnectClient interface {
-	Connect(fingerprint fingerprint.Fingerprint) (entity.Identity, backendhttp.RetryPolicy, error)
-	ConnectUpdate(entity.Identity, fingerprint.Fingerprint) (backendhttp.RetryPolicy, entity.Identity, error)
+	Connect(fingerprint fingerprint.Fingerprint, metadata Metadata) (entity.Identity, backendhttp.RetryPolicy, error)
+	ConnectUpdate(entityID entity.Identity, fingerprint fingerprint.Fingerprint, metadata Metadata) (backendhttp.RetryPolicy, entity.Identity, error)
 	Disconnect(entityID entity.ID, reason DisconnectReason) error
 }
 
@@ -49,6 +49,7 @@ type identityClient struct {
 
 type postConnectBody struct {
 	Fingerprint fingerprint.Fingerprint `json:"fingerprint"`
+	Metadata    Metadata                `json:"metadata"`
 	Type        string                  `json:"type"`
 	Protocol    string                  `json:"protocol"`
 	EntityID    entity.ID               `json:"entityId,omitempty"`
@@ -97,13 +98,15 @@ func NewIdentityConnectClient(
 
 // Perform the Connect step. The Agent must supply a fingerprint for the host. Backend should reply
 // with a unique Entity ID across NR.
-func (ic *identityClient) Connect(fingerprint fingerprint.Fingerprint) (ids entity.Identity, retry backendhttp.RetryPolicy, err error) {
+//
+//nolint:cyclop
+func (ic *identityClient) Connect(fingerprint fingerprint.Fingerprint, metadata Metadata) (ids entity.Identity, retry backendhttp.RetryPolicy, err error) {
 	buf, err := ic.marshal(postConnectBody{
 		Fingerprint: fingerprint,
+		Metadata:    metadata,
 		Type:        ic.agentType(),
 		Protocol:    "v1",
 	})
-
 	if err != nil {
 		return
 	}
@@ -161,9 +164,11 @@ func (ic *identityClient) Connect(fingerprint fingerprint.Fingerprint) (ids enti
 }
 
 // ConnectUpdate is used to update the host fingerprint of the entityID to the backend.
-func (ic *identityClient) ConnectUpdate(entityIdn entity.Identity, fingerprint fingerprint.Fingerprint) (retry backendhttp.RetryPolicy, ids entity.Identity, err error) {
+// nolint:cyclop
+func (ic *identityClient) ConnectUpdate(entityIdn entity.Identity, fingerprint fingerprint.Fingerprint, metadata Metadata) (retry backendhttp.RetryPolicy, ids entity.Identity, err error) {
 	buf, err := ic.marshal(postConnectBody{
 		Fingerprint: fingerprint,
+		Metadata:    metadata,
 		Type:        ic.agentType(),
 		Protocol:    "v1",
 		EntityID:    entityIdn.ID,