diff --git a/comp/core/tagger/common/entity_id_builder.go b/comp/core/tagger/common/entity_id_builder.go index 93774664bc7bf6..b1e71cc2800869 100644 --- a/comp/core/tagger/common/entity_id_builder.go +++ b/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 } diff --git a/comp/core/tagger/global.go b/comp/core/tagger/global.go index 060245282f9e09..c7f999a1d0ae07 100644 --- a/comp/core/tagger/global.go +++ b/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) } diff --git a/comp/core/tagger/taggerimpl/generic_store/composite_store.go b/comp/core/tagger/taggerimpl/generic_store/composite_store.go index 1428079ea286ca..1a1b7306265c77 100644 --- a/comp/core/tagger/taggerimpl/generic_store/composite_store.go +++ b/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() diff --git a/comp/core/tagger/taggerimpl/generic_store/store_test.go b/comp/core/tagger/taggerimpl/generic_store/store_test.go index c2f9073021a71e..4a8a809a368128 100644 --- a/comp/core/tagger/taggerimpl/generic_store/store_test.go +++ b/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{}{}) } diff --git a/comp/core/tagger/types/entity_id.go b/comp/core/tagger/types/entity_id.go index 4db94a61c55c6a..5604d060c9621f 100644 --- a/comp/core/tagger/types/entity_id.go +++ b/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" diff --git a/comp/core/tagger/types/entity_id_test.go b/comp/core/tagger/types/entity_id_test.go deleted file mode 100644 index a235e4273ad95c..00000000000000 --- a/comp/core/tagger/types/entity_id_test.go +++ /dev/null @@ -1,22 +0,0 @@ -// Unless explicitly stated otherwise all files in this repository are licensed -// under the Apache License Version 2.0. -// This product includes software developed at Datadog (https://www.datadoghq.com/). -// Copyright 2016-present Datadog, Inc. - -// Package types defines types used by the Tagger component. -package types - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestNewDefaultEntityIDFromStr(t *testing.T) { - str := "container_id://1234" - entityID, err := NewEntityIDFromString(str) - require.NoError(t, err) - assert.Equal(t, ContainerID, entityID.GetPrefix()) - assert.Equal(t, "1234", entityID.GetID()) -} diff --git a/comp/core/tagger/types/types.go b/comp/core/tagger/types/types.go index a66749bae5c285..e45f9e6daa0842 100644 --- a/comp/core/tagger/types/types.go +++ b/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 diff --git a/comp/dogstatsd/replay/impl/writer.go b/comp/dogstatsd/replay/impl/writer.go index b46eaf456bbf5a..77f7ea7575351a 100644 --- a/comp/dogstatsd/replay/impl/writer.go +++ b/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) diff --git a/pkg/collector/corechecks/containers/kubelet/provider/cadvisor/provider_test.go b/pkg/collector/corechecks/containers/kubelet/provider/cadvisor/provider_test.go index 136aa7a67203b4..dbc8a8fa43eebc 100644 --- a/pkg/collector/corechecks/containers/kubelet/provider/cadvisor/provider_test.go +++ b/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) } diff --git a/pkg/collector/corechecks/containers/kubelet/provider/kubelet/provider_test.go b/pkg/collector/corechecks/containers/kubelet/provider/kubelet/provider_test.go index 2830612208a5bf..096b63b8981407 100644 --- a/pkg/collector/corechecks/containers/kubelet/provider/kubelet/provider_test.go +++ b/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) } diff --git a/pkg/collector/corechecks/containers/kubelet/provider/pod/provider_test.go b/pkg/collector/corechecks/containers/kubelet/provider/pod/provider_test.go index e484a641901327..50ac558c64c7f9 100644 --- a/pkg/collector/corechecks/containers/kubelet/provider/pod/provider_test.go +++ b/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) } diff --git a/pkg/collector/corechecks/containers/kubelet/provider/probe/provider_test.go b/pkg/collector/corechecks/containers/kubelet/provider/probe/provider_test.go index 56043b80add61e..2b21551c072019 100644 --- a/pkg/collector/corechecks/containers/kubelet/provider/probe/provider_test.go +++ b/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) } diff --git a/pkg/collector/corechecks/containers/kubelet/provider/summary/provider_test.go b/pkg/collector/corechecks/containers/kubelet/provider/summary/provider_test.go index 766d4c8c0acdbe..0661462956d71b 100644 --- a/pkg/collector/corechecks/containers/kubelet/provider/summary/provider_test.go +++ b/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) diff --git a/pkg/collector/corechecks/ebpf/oomkill/oom_kill.go b/pkg/collector/corechecks/ebpf/oomkill/oom_kill.go index fa511a4e66db5a..33b9393ac9af26 100644 --- a/pkg/collector/corechecks/ebpf/oomkill/oom_kill.go +++ b/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) diff --git a/pkg/collector/corechecks/ebpf/tcpqueuelength/tcp_queue_length.go b/pkg/collector/corechecks/ebpf/tcpqueuelength/tcp_queue_length.go index 8a8b8c9cd5bb7a..4a0fecf278bc6c 100644 --- a/pkg/collector/corechecks/ebpf/tcpqueuelength/tcp_queue_length.go +++ b/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)