address PR comments - mainly remove all calls to NewEntityIDFromString

comp/core/tagger/common/entity_id_builder.go

@@ -7,6 +7,9 @@
 package common
 
 import (
+	"fmt"
+	"strings"
+
 	"github.com/DataDog/datadog-agent/comp/core/tagger/types"
 	workloadmeta "github.com/DataDog/datadog-agent/comp/core/workloadmeta/def"
 	"github.com/DataDog/datadog-agent/pkg/util/log"
@@ -39,14 +42,18 @@ func BuildTaggerEntityID(entityID workloadmeta.EntityID) types.EntityID {
 }
 
 var globalEntityID = types.NewEntityID("internal", "global-entity-id")
-var globalEntityIDString = globalEntityID.String()
 
 // GetGlobalEntityID returns the entity ID that holds global tags
 func GetGlobalEntityID() types.EntityID {
 	return globalEntityID
 }
 
-// GetGlobalEntityIDString returns, in a plain string format, the entity ID that holds global tags
-func GetGlobalEntityIDString() string {
-	return globalEntityIDString
+// ExtractPrefixAndID extracts prefix and id from tagger entity id and returns an error if the received entityID is not valid
+func ExtractPrefixAndID(entityID string) (prefix types.EntityIDPrefix, id string, err error) {
+	extractedPrefix, extractedID, found := strings.Cut(entityID, "://")
+	if !found {
+		return "", "", fmt.Errorf("unsupported tagger entity id format %q, correct format is `{prefix}://{id}`", entityID)
+	}
+
+	return types.EntityIDPrefix(extractedPrefix), extractedID, nil
 }

comp/core/tagger/global.go

@@ -6,9 +6,11 @@
 package tagger
 
 import (
+	"errors"
 	"fmt"
 	"sync"
 
+	"github.com/DataDog/datadog-agent/comp/core/tagger/common"
 	"github.com/DataDog/datadog-agent/comp/core/tagger/types"
 	taggertypes "github.com/DataDog/datadog-agent/pkg/tagger/types"
 	"github.com/DataDog/datadog-agent/pkg/tagset"
@@ -53,18 +55,19 @@ func LegacyTag(entity string, cardinality types.TagCardinality) ([]string, error
 		return nil, fmt.Errorf("a global tagger must be set before calling Tag")
 	}
 
-	entityID, err := types.NewEntityIDFromString(entity)
+	prefix, id, err := common.ExtractPrefixAndID(entity)
 	if err != nil {
-		return []string{}, err
+		return nil, err
 	}
 
+	entityID := types.NewEntityID(prefix, id)
 	return globalTagger.Tag(entityID, cardinality)
 }
 
 // Tag is an interface function that queries taggerclient singleton
 func Tag(entity types.EntityID, cardinality types.TagCardinality) ([]string, error) {
 	if globalTagger == nil {
-		return nil, fmt.Errorf("a global tagger must be set before calling Tag")
+		return nil, errors.New("a global tagger must be set before calling Tag")
 	}
 	return globalTagger.Tag(entity, cardinality)
 }

comp/core/tagger/taggerimpl/generic_store/composite_store.go

@@ -32,16 +32,6 @@ func (os *compositeObjectStore[T]) Get(entityID types.EntityID) (object T, found
 	return
 }
 
-// GetWithEntityIDStr implements ObjectStore#GetWithEntityIDStr
-func (os *compositeObjectStore[T]) GetWithEntityIDStr(id string) (object T, found bool) {
-	entityID, err := types.NewEntityIDFromString(id)
-	if err != nil {
-		return
-	}
-
-	return os.Get(entityID)
-}
-
 // Set implements ObjectStore#Set
 func (os *compositeObjectStore[T]) Set(entityID types.EntityID, object T) {
 	prefix := entityID.GetPrefix()

comp/core/tagger/taggerimpl/generic_store/store_test.go

@@ -39,33 +39,6 @@ func TestObjectStore_GetSet(t *testing.T) {
 	assert.Falsef(t, found, "item should not be found in store")
 }
 
-func TestObjectStore_GetWithEntityIDStr(t *testing.T) {
-	store := NewObjectStore[any]()
-
-	id := types.NewEntityID("prefix", "id")
-	idStr := id.String()
-	// getting a non-existent item
-	obj, found := store.GetWithEntityIDStr(idStr)
-	assert.Nil(t, obj)
-	assert.Falsef(t, found, "item should not be found in store")
-
-	// set item
-	store.Set(id, struct{}{})
-
-	// getting item
-	obj, found = store.GetWithEntityIDStr(idStr)
-	assert.NotNil(t, obj)
-	assert.Truef(t, found, "item should be found in store")
-
-	// unsetting item
-	store.Unset(id)
-
-	// getting a non-existent item
-	obj, found = store.GetWithEntityIDStr(idStr)
-	assert.Nil(t, obj)
-	assert.Falsef(t, found, "item should not be found in store")
-}
-
 func TestObjectStore_Size(t *testing.T) {
 	store := NewObjectStore[any]()
 
@@ -99,10 +72,15 @@ func TestObjectStore_ListObjects(t *testing.T) {
 	assert.Equalf(t, len(list), 0, "ListObjects should return an empty list")
 
 	// add some items
-	ids := []string{"prefix1://id1", "prefix2://id2", "prefix3://id3", "prefix4://id4"}
-	for _, id := range ids {
-		entityID, _ := types.NewEntityIDFromString(id)
-		store.Set(entityID, id)
+	ids := []types.EntityID{
+		types.NewEntityID(types.EntityIDPrefix("prefix1"), "id1"),
+		types.NewEntityID(types.EntityIDPrefix("prefix2"), "id2"),
+		types.NewEntityID(types.EntityIDPrefix("prefix3"), "id3"),
+		types.NewEntityID(types.EntityIDPrefix("prefix4"), "id4"),
+	}
+
+	for _, entityID := range ids {
+		store.Set(entityID, entityID)
 	}
 
 	// list should return empty
@@ -115,9 +93,14 @@ func TestObjectStore_ForEach(t *testing.T) {
 	store := NewObjectStore[any]()
 
 	// add some items
-	ids := []string{"prefix1://id1", "prefix2://id2", "prefix3://id3", "prefix4://id4"}
-	for _, id := range ids {
-		entityID, _ := types.NewEntityIDFromString(id)
+	ids := []types.EntityID{
+		types.NewEntityID(types.EntityIDPrefix("prefix1"), "id1"),
+		types.NewEntityID(types.EntityIDPrefix("prefix2"), "id2"),
+		types.NewEntityID(types.EntityIDPrefix("prefix3"), "id3"),
+		types.NewEntityID(types.EntityIDPrefix("prefix4"), "id4"),
+	}
+
+	for _, entityID := range ids {
 		store.Set(entityID, struct{}{})
 	}
 

comp/core/tagger/types/entity_id.go

@@ -6,11 +6,6 @@
 // Package types defines types used by the Tagger component.
 package types
 
-import (
-	"fmt"
-	"strings"
-)
-
 const separator = "://"
 const separatorLength = len(separator)
 
@@ -50,17 +45,6 @@ func NewEntityID(prefix EntityIDPrefix, id string) EntityID {
 	return EntityID{prefix, id}
 }
 
-// NewEntityIDFromString constructs EntityID from a plain string id
-func NewEntityIDFromString(plainStringID string) (EntityID, error) {
-	prefix, id, found := strings.Cut(plainStringID, separator)
-
-	if !found {
-		return EntityID{}, fmt.Errorf("unsupported tagger entity id format %q, correct format is `{prefix}://{id}`", plainStringID)
-	}
-
-	return EntityID{EntityIDPrefix(prefix), id}, nil
-}
-
 const (
 	// ContainerID is the prefix `container_id`
 	ContainerID EntityIDPrefix = "container_id"

comp/core/tagger/types/types.go

@@ -22,13 +22,6 @@ type ApplyFunc[V any] func(EntityID, V)
 type ObjectStore[V any] interface {
 	// Get returns an object with the specified entity ID if it exists in the store
 	Get(EntityID) (V, bool)
-	// GetWithEntityIDStr returns an object with the specified entity ID if it
-	// exists in the store.
-	// This function is needed only for performance reasons. It functions like
-	// Get, but accepts a string instead of an EntityID, creating the EntityID
-	// internally. This reduces the allocations that occur when an EntityID is
-	// passed as a parameter.
-	GetWithEntityIDStr(string) (V, bool)
 	// Set sets a given entityID to a given object in the store
 	Set(EntityID, V)
 	// Unset unsets a given entityID in the store

comp/dogstatsd/replay/impl/writer.go

@@ -23,6 +23,7 @@ import (
 	"github.com/golang/protobuf/proto"
 
 	"github.com/DataDog/datadog-agent/comp/core/tagger"
+	"github.com/DataDog/datadog-agent/comp/core/tagger/common"
 	taggerproto "github.com/DataDog/datadog-agent/comp/core/tagger/proto"
 	"github.com/DataDog/datadog-agent/comp/core/tagger/types"
 	"github.com/DataDog/datadog-agent/comp/dogstatsd/packets"
@@ -313,12 +314,14 @@ func (tc *TrafficCaptureWriter) writeState() (int, error) {
 	}
 
 	// iterate entities
-	for _, id := range tc.taggerState {
-		entityID, err := types.NewEntityIDFromString(id)
+	for _, entityIDStr := range tc.taggerState {
+		prefix, id, err := common.ExtractPrefixAndID(entityIDStr)
 		if err != nil {
 			log.Warnf("Invalid entity id: %q", id)
 			continue
 		}
+
+		entityID := types.NewEntityID(prefix, id)
 		entity, err := tagger.GetEntity(entityID)
 		if err != nil {
 			log.Warnf("There was no entity for container id: %v present in the tagger", entity)

pkg/collector/corechecks/containers/kubelet/provider/cadvisor/provider_test.go

@@ -18,6 +18,7 @@ import (
 
 	"github.com/DataDog/datadog-agent/comp/core"
 	"github.com/DataDog/datadog-agent/comp/core/autodiscovery/common/types"
+	taggercommon "github.com/DataDog/datadog-agent/comp/core/tagger/common"
 	"github.com/DataDog/datadog-agent/comp/core/tagger/taggerimpl"
 	taggertypes "github.com/DataDog/datadog-agent/comp/core/tagger/types"
 	workloadmeta "github.com/DataDog/datadog-agent/comp/core/workloadmeta/def"
@@ -108,7 +109,8 @@ func (suite *ProviderTestSuite) SetupTest() {
 	fakeTagger := taggerimpl.SetupFakeTagger(suite.T())
 	defer fakeTagger.ResetTagger()
 	for entity, tags := range commontesting.CommonTags {
-		entityID, _ := taggertypes.NewEntityIDFromString(entity)
+		prefix, id, _ := taggercommon.ExtractPrefixAndID(entity)
+		entityID := taggertypes.NewEntityID(prefix, id)
 		fakeTagger.SetTags(entityID, "foo", tags, nil, nil, nil)
 	}
 

pkg/collector/corechecks/containers/kubelet/provider/kubelet/provider_test.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/DataDog/datadog-agent/comp/core"
 	"github.com/DataDog/datadog-agent/comp/core/autodiscovery/common/types"
+	taggercommon "github.com/DataDog/datadog-agent/comp/core/tagger/common"
 	"github.com/DataDog/datadog-agent/comp/core/tagger/taggerimpl"
 	taggertypes "github.com/DataDog/datadog-agent/comp/core/tagger/types"
 	workloadmeta "github.com/DataDog/datadog-agent/comp/core/workloadmeta/def"
@@ -139,7 +140,8 @@ func (suite *ProviderTestSuite) SetupTest() {
 	fakeTagger := taggerimpl.SetupFakeTagger(suite.T())
 	defer fakeTagger.ResetTagger()
 	for entity, tags := range commontesting.CommonTags {
-		entityID, _ := taggertypes.NewEntityIDFromString(entity)
+		prefix, id, _ := taggercommon.ExtractPrefixAndID(entity)
+		entityID := taggertypes.NewEntityID(prefix, id)
 		fakeTagger.SetTags(entityID, "foo", tags, nil, nil, nil)
 	}
 

pkg/collector/corechecks/containers/kubelet/provider/pod/provider_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/stretchr/testify/suite"
 
 	"github.com/DataDog/datadog-agent/comp/core/autodiscovery/common/types"
+	taggercommon "github.com/DataDog/datadog-agent/comp/core/tagger/common"
 	"github.com/DataDog/datadog-agent/comp/core/tagger/taggerimpl"
 	taggertypes "github.com/DataDog/datadog-agent/comp/core/tagger/types"
 	"github.com/DataDog/datadog-agent/pkg/aggregator/mocksender"
@@ -116,7 +117,8 @@ func (suite *ProviderTestSuite) SetupTest() {
 	fakeTagger := taggerimpl.SetupFakeTagger(suite.T())
 	defer fakeTagger.ResetTagger()
 	for entity, tags := range commontesting.CommonTags {
-		entityID, _ := taggertypes.NewEntityIDFromString(entity)
+		prefix, id, _ := taggercommon.ExtractPrefixAndID(entity)
+		entityID := taggertypes.NewEntityID(prefix, id)
 		fakeTagger.SetTags(entityID, "foo", tags, nil, nil, nil)
 	}
 

pkg/collector/corechecks/containers/kubelet/provider/probe/provider_test.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/DataDog/datadog-agent/comp/core"
 	"github.com/DataDog/datadog-agent/comp/core/autodiscovery/common/types"
+	taggercommon "github.com/DataDog/datadog-agent/comp/core/tagger/common"
 	"github.com/DataDog/datadog-agent/comp/core/tagger/taggerimpl"
 	taggertypes "github.com/DataDog/datadog-agent/comp/core/tagger/types"
 	workloadmeta "github.com/DataDog/datadog-agent/comp/core/workloadmeta/def"
@@ -275,7 +276,8 @@ func TestProvider_Provide(t *testing.T) {
 			fakeTagger := taggerimpl.SetupFakeTagger(t)
 			defer fakeTagger.ResetTagger()
 			for entity, tags := range probeTags {
-				entityID, _ := taggertypes.NewEntityIDFromString(entity)
+				prefix, id, _ := taggercommon.ExtractPrefixAndID(entity)
+				entityID := taggertypes.NewEntityID(prefix, id)
 				fakeTagger.SetTags(entityID, "foo", tags, nil, nil, nil)
 			}
 

pkg/collector/corechecks/containers/kubelet/provider/summary/provider_test.go

@@ -21,6 +21,7 @@ import (
 	configcomp "github.com/DataDog/datadog-agent/comp/core/config"
 	log "github.com/DataDog/datadog-agent/comp/core/log/def"
 	logmock "github.com/DataDog/datadog-agent/comp/core/log/mock"
+	taggercommon "github.com/DataDog/datadog-agent/comp/core/tagger/common"
 	"github.com/DataDog/datadog-agent/comp/core/tagger/taggerimpl"
 	taggertypes "github.com/DataDog/datadog-agent/comp/core/tagger/types"
 	workloadmeta "github.com/DataDog/datadog-agent/comp/core/workloadmeta/def"
@@ -337,7 +338,8 @@ func TestProvider_Provide(t *testing.T) {
 			fakeTagger := taggerimpl.SetupFakeTagger(t)
 			defer fakeTagger.ResetTagger()
 			for entity, tags := range entityTags {
-				entityID, _ := taggertypes.NewEntityIDFromString(entity)
+				prefix, id, _ := taggercommon.ExtractPrefixAndID(entity)
+				entityID := taggertypes.NewEntityID(prefix, id)
 				fakeTagger.SetTags(entityID, "foo", tags, nil, nil, nil)
 			}
 			store := creatFakeStore(t)

pkg/collector/corechecks/ebpf/oomkill/oom_kill.go

@@ -117,7 +117,7 @@ func (m *OOMKillCheck) Run() error {
 
 		entityID := types.NewEntityID(types.ContainerID, containerID)
 		var tags []string
-		if entityID.Empty() {
+		if !entityID.Empty() {
 			tags, err = tagger.Tag(entityID, tagger.ChecksCardinality())
 			if err != nil {
 				log.Errorf("Error collecting tags for container %s: %s", containerID, err)

pkg/collector/corechecks/ebpf/tcpqueuelength/tcp_queue_length.go

@@ -112,7 +112,7 @@ func (t *TCPQueueLengthCheck) Run() error {
 
 		entityID := types.NewEntityID(types.ContainerID, containerID)
 		var tags []string
-		if entityID.Empty() {
+		if !entityID.Empty() {
 			tags, err = tagger.Tag(entityID, types.HighCardinality)
 			if err != nil {
 				log.Errorf("Error collecting tags for container %s: %s", k, err)