From c91db31afcde96dd5467c3ac38c56310ab010c45 Mon Sep 17 00:00:00 2001 From: adel121 Date: Mon, 14 Oct 2024 16:13:16 +0200 Subject: [PATCH 1/7] use entityID struct in tagger Tag method --- comp/core/tagger/component.go | 2 +- comp/core/tagger/global.go | 19 ++++++++++++++++++- pkg/collector/python/tagger.go | 2 +- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/comp/core/tagger/component.go b/comp/core/tagger/component.go index 621225821b58a..00c4fab6d4729 100644 --- a/comp/core/tagger/component.go +++ b/comp/core/tagger/component.go @@ -38,7 +38,7 @@ type Component interface { Stop() error ReplayTagger() ReplayTagger GetTaggerTelemetryStore() *telemetry.Store - Tag(entityID string, cardinality types.TagCardinality) ([]string, error) + Tag(entityID types.EntityID, cardinality types.TagCardinality) ([]string, error) AccumulateTagsFor(entityID types.EntityID, cardinality types.TagCardinality, tb tagset.TagsAccumulator) error Standard(entityID types.EntityID) ([]string, error) List() types.TaggerListResponse diff --git a/comp/core/tagger/global.go b/comp/core/tagger/global.go index dd90944afd8f9..1e59077251139 100644 --- a/comp/core/tagger/global.go +++ b/comp/core/tagger/global.go @@ -44,8 +44,25 @@ func GetEntity(entityID types.EntityID) (*types.Entity, error) { return globalTagger.GetEntity(entityID) } +// TagDeprecated is an interface function that queries taggerclient singleton +// This function is not to be used by golang components +// This function exists in order not to break backward compatibility with rtloader and python +// integrations using the tagger +func TagDeprecated(entity string, cardinality types.TagCardinality) ([]string, error) { + if globalTagger == nil { + return nil, fmt.Errorf("a global tagger must be set before calling Tag") + } + + entityID, err := types.NewEntityIDFromString(entity) + if err != nil { + return []string{}, err + } + + return globalTagger.Tag(entityID, cardinality) +} + // Tag is an interface function that queries taggerclient singleton -func Tag(entity string, cardinality types.TagCardinality) ([]string, error) { +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") } diff --git a/pkg/collector/python/tagger.go b/pkg/collector/python/tagger.go index 531c1636edf85..e572e7474a935 100644 --- a/pkg/collector/python/tagger.go +++ b/pkg/collector/python/tagger.go @@ -26,7 +26,7 @@ import "C" // for testing purposes var ( - tagsFunc = tagger.Tag + tagsFunc = tagger.TagDeprecated ) // Tags bridges towards tagger.Tag to retrieve container tags From 288c08e6ceb33b601c4c7b5b28c34df0e9b2e299 Mon Sep 17 00:00:00 2001 From: adel121 Date: Tue, 15 Oct 2024 14:12:08 +0200 Subject: [PATCH 2/7] update tagger to use entityid struct to return tags --- comp/core/tagger/global.go | 6 +++--- comp/core/tagger/noopimpl/tagger.go | 2 +- comp/core/tagger/taggerimpl/local/fake_tagger.go | 11 +++++------ comp/core/tagger/taggerimpl/local/tagger.go | 6 ++---- comp/core/tagger/taggerimpl/local/tagger_test.go | 7 +++---- comp/core/tagger/taggerimpl/remote/tagger.go | 7 +++---- comp/core/tagger/taggerimpl/replay/tagger.go | 5 ++--- comp/core/tagger/taggerimpl/server/server.go | 2 +- comp/core/tagger/taggerimpl/tagger.go | 10 +++++----- 9 files changed, 25 insertions(+), 31 deletions(-) diff --git a/comp/core/tagger/global.go b/comp/core/tagger/global.go index 1e59077251139..f4bcc2966e10c 100644 --- a/comp/core/tagger/global.go +++ b/comp/core/tagger/global.go @@ -44,11 +44,11 @@ func GetEntity(entityID types.EntityID) (*types.Entity, error) { return globalTagger.GetEntity(entityID) } -// TagDeprecated is an interface function that queries taggerclient singleton -// This function is not to be used by golang components +// LegacyTag is an interface function that queries taggerclient singleton +// If possible, avoid using this function, and use the Tag interface function instead. // This function exists in order not to break backward compatibility with rtloader and python // integrations using the tagger -func TagDeprecated(entity string, cardinality types.TagCardinality) ([]string, error) { +func LegacyTag(entity string, cardinality types.TagCardinality) ([]string, error) { if globalTagger == nil { return nil, fmt.Errorf("a global tagger must be set before calling Tag") } diff --git a/comp/core/tagger/noopimpl/tagger.go b/comp/core/tagger/noopimpl/tagger.go index 3ccb8e7d731ce..3b07469f975e7 100644 --- a/comp/core/tagger/noopimpl/tagger.go +++ b/comp/core/tagger/noopimpl/tagger.go @@ -54,7 +54,7 @@ func (n *noopTagger) GetTaggerTelemetryStore() *telemetry.Store { return nil } -func (n *noopTagger) Tag(string, types.TagCardinality) ([]string, error) { +func (n *noopTagger) Tag(types.EntityID, types.TagCardinality) ([]string, error) { return nil, nil } diff --git a/comp/core/tagger/taggerimpl/local/fake_tagger.go b/comp/core/tagger/taggerimpl/local/fake_tagger.go index 81e40a0c6a84d..74bdbe26b642c 100644 --- a/comp/core/tagger/taggerimpl/local/fake_tagger.go +++ b/comp/core/tagger/taggerimpl/local/fake_tagger.go @@ -97,11 +97,10 @@ func (f *FakeTagger) GetTaggerTelemetryStore() *telemetry.Store { } // Tag fake implementation -func (f *FakeTagger) Tag(entityID string, cardinality types.TagCardinality) ([]string, error) { - id, _ := types.NewEntityIDFromString(entityID) - tags := f.store.Lookup(id, cardinality) +func (f *FakeTagger) Tag(entityID types.EntityID, cardinality types.TagCardinality) ([]string, error) { + tags := f.store.Lookup(entityID, cardinality) - key := f.getKey(id, cardinality) + key := f.getKey(entityID, cardinality) if err := f.errors[key]; err != nil { return nil, err } @@ -111,12 +110,12 @@ func (f *FakeTagger) Tag(entityID string, cardinality types.TagCardinality) ([]s // GlobalTags fake implementation func (f *FakeTagger) GlobalTags(cardinality types.TagCardinality) ([]string, error) { - return f.Tag(common.GetGlobalEntityIDString(), cardinality) + return f.Tag(common.GetGlobalEntityID(), cardinality) } // AccumulateTagsFor fake implementation func (f *FakeTagger) AccumulateTagsFor(entityID types.EntityID, cardinality types.TagCardinality, tb tagset.TagsAccumulator) error { - tags, err := f.Tag(entityID.String(), cardinality) + tags, err := f.Tag(entityID, cardinality) if err != nil { return err } diff --git a/comp/core/tagger/taggerimpl/local/tagger.go b/comp/core/tagger/taggerimpl/local/tagger.go index ea49d601930e1..7fcce587c2937 100644 --- a/comp/core/tagger/taggerimpl/local/tagger.go +++ b/comp/core/tagger/taggerimpl/local/tagger.go @@ -97,10 +97,8 @@ func (t *Tagger) AccumulateTagsFor(entityID types.EntityID, cardinality types.Ta } // Tag returns a copy of the tags for a given entity -func (t *Tagger) Tag(entityID string, cardinality types.TagCardinality) ([]string, error) { - compositeEntityID, _ := types.NewEntityIDFromString(entityID) - - tags, err := t.getTags(compositeEntityID, cardinality) +func (t *Tagger) Tag(entityID types.EntityID, cardinality types.TagCardinality) ([]string, error) { + tags, err := t.getTags(entityID, cardinality) if err != nil { return nil, err } diff --git a/comp/core/tagger/taggerimpl/local/tagger_test.go b/comp/core/tagger/taggerimpl/local/tagger_test.go index dc20106a88c20..99fddf0879859 100644 --- a/comp/core/tagger/taggerimpl/local/tagger_test.go +++ b/comp/core/tagger/taggerimpl/local/tagger_test.go @@ -67,7 +67,6 @@ func TestAccumulateTagsFor(t *testing.T) { func TestTag(t *testing.T) { entityID := types.NewEntityID(types.ContainerID, "123") - entityIDStr := entityID.String() store := fxutil.Test[workloadmetamock.Mock](t, fx.Options( fx.Supply(config.Params{}), @@ -99,15 +98,15 @@ func TestTag(t *testing.T) { }, }) - lowCardTags, err := tagger.Tag(entityIDStr, types.LowCardinality) + lowCardTags, err := tagger.Tag(entityID, types.LowCardinality) assert.NoError(t, err) assert.ElementsMatch(t, []string{"low1", "low2"}, lowCardTags) - orchestratorCardTags, err := tagger.Tag(entityIDStr, types.OrchestratorCardinality) + orchestratorCardTags, err := tagger.Tag(entityID, types.OrchestratorCardinality) assert.NoError(t, err) assert.ElementsMatch(t, []string{"low1", "low2", "orchestrator1", "orchestrator2"}, orchestratorCardTags) - highCardTags, err := tagger.Tag(entityIDStr, types.HighCardinality) + highCardTags, err := tagger.Tag(entityID, types.HighCardinality) assert.NoError(t, err) assert.ElementsMatch(t, []string{"low1", "low2", "orchestrator1", "orchestrator2", "high1", "high2"}, highCardTags) } diff --git a/comp/core/tagger/taggerimpl/remote/tagger.go b/comp/core/tagger/taggerimpl/remote/tagger.go index b69e9ed9508b7..6c32a179ed600 100644 --- a/comp/core/tagger/taggerimpl/remote/tagger.go +++ b/comp/core/tagger/taggerimpl/remote/tagger.go @@ -200,9 +200,8 @@ func (t *Tagger) GetTaggerTelemetryStore() *telemetry.Store { } // Tag returns tags for a given entity at the desired cardinality. -func (t *Tagger) Tag(entityID string, cardinality types.TagCardinality) ([]string, error) { - id, _ := types.NewEntityIDFromString(entityID) - entity := t.store.getEntity(id) +func (t *Tagger) Tag(entityID types.EntityID, cardinality types.TagCardinality) ([]string, error) { + entity := t.store.getEntity(entityID) if entity != nil { t.telemetryStore.QueriesByCardinality(cardinality).Success.Inc() return entity.GetTags(cardinality), nil @@ -215,7 +214,7 @@ func (t *Tagger) Tag(entityID string, cardinality types.TagCardinality) ([]strin // AccumulateTagsFor returns tags for a given entity at the desired cardinality. func (t *Tagger) AccumulateTagsFor(entityID types.EntityID, cardinality types.TagCardinality, tb tagset.TagsAccumulator) error { - tags, err := t.Tag(entityID.String(), cardinality) + tags, err := t.Tag(entityID, cardinality) if err != nil { return err } diff --git a/comp/core/tagger/taggerimpl/replay/tagger.go b/comp/core/tagger/taggerimpl/replay/tagger.go index 84069439d3b04..16d36c5a022ec 100644 --- a/comp/core/tagger/taggerimpl/replay/tagger.go +++ b/comp/core/tagger/taggerimpl/replay/tagger.go @@ -64,9 +64,8 @@ func (t *Tagger) Stop() error { } // Tag returns tags for a given entity at the desired cardinality. -func (t *Tagger) Tag(entityID string, cardinality types.TagCardinality) ([]string, error) { - id, _ := types.NewEntityIDFromString(entityID) - tags := t.store.Lookup(id, cardinality) +func (t *Tagger) Tag(entityID types.EntityID, cardinality types.TagCardinality) ([]string, error) { + tags := t.store.Lookup(entityID, cardinality) return tags, nil } diff --git a/comp/core/tagger/taggerimpl/server/server.go b/comp/core/tagger/taggerimpl/server/server.go index 225e558e7070f..ca51e24c4fc36 100644 --- a/comp/core/tagger/taggerimpl/server/server.go +++ b/comp/core/tagger/taggerimpl/server/server.go @@ -139,7 +139,7 @@ func (s *Server) TaggerFetchEntity(_ context.Context, in *pb.FetchEntityRequest) return nil, status.Errorf(codes.InvalidArgument, `missing "id" parameter`) } - entityID := types.EntityIDPrefix(in.Id.Prefix).ToUID(in.Id.Uid) + entityID := types.NewEntityID(types.EntityIDPrefix(in.Id.Prefix), in.Id.Uid) cardinality, err := proto.Pb2TaggerCardinality(in.GetCardinality()) if err != nil { return nil, err diff --git a/comp/core/tagger/taggerimpl/tagger.go b/comp/core/tagger/taggerimpl/tagger.go index 388fcca3ddea9..3e9e5f2c6904b 100644 --- a/comp/core/tagger/taggerimpl/tagger.go +++ b/comp/core/tagger/taggerimpl/tagger.go @@ -259,7 +259,7 @@ func (t *TaggerClient) GetEntity(entityID types.EntityID) (*types.Entity, error) // Tag queries the captureTagger (for replay scenarios) or the defaultTagger. // It can return tags at high cardinality (with tags about individual containers), // or at orchestrator cardinality (pod/task level). -func (t *TaggerClient) Tag(entityID string, cardinality types.TagCardinality) ([]string, error) { +func (t *TaggerClient) Tag(entityID types.EntityID, cardinality types.TagCardinality) ([]string, error) { // TODO: defer unlock once performance overhead of defer is negligible t.mux.RLock() if t.captureTagger != nil { @@ -294,7 +294,7 @@ func (t *TaggerClient) AccumulateTagsFor(entityID types.EntityID, cardinality ty // GetEntityHash returns the hash for the tags associated with the given entity // Returns an empty string if the tags lookup fails func (t *TaggerClient) GetEntityHash(entityID types.EntityID, cardinality types.TagCardinality) string { - tags, err := t.Tag(entityID.String(), cardinality) + tags, err := t.Tag(entityID, cardinality) if err != nil { return "" } @@ -329,7 +329,7 @@ func (t *TaggerClient) AgentTags(cardinality types.TagCardinality) ([]string, er return nil, nil } - entityID := types.NewEntityID(types.ContainerID, ctrID).String() + entityID := types.NewEntityID(types.ContainerID, ctrID) return t.Tag(entityID, cardinality) } @@ -338,14 +338,14 @@ func (t *TaggerClient) AgentTags(cardinality types.TagCardinality) ([]string, er func (t *TaggerClient) GlobalTags(cardinality types.TagCardinality) ([]string, error) { t.mux.RLock() if t.captureTagger != nil { - tags, err := t.captureTagger.Tag(taggercommon.GetGlobalEntityIDString(), cardinality) + tags, err := t.captureTagger.Tag(taggercommon.GetGlobalEntityID(), cardinality) if err == nil && len(tags) > 0 { t.mux.RUnlock() return tags, nil } } t.mux.RUnlock() - return t.defaultTagger.Tag(taggercommon.GetGlobalEntityIDString(), cardinality) + return t.defaultTagger.Tag(taggercommon.GetGlobalEntityID(), cardinality) } // globalTagBuilder queries global tags that should apply to all data coming From 0aedc350e5ef979e82438b1b021b35c08c17fb07 Mon Sep 17 00:00:00 2001 From: adel121 Date: Tue, 15 Oct 2024 14:13:16 +0200 Subject: [PATCH 3/7] update calls to the tagger --- comp/core/autodiscovery/listeners/service.go | 2 +- comp/otelcol/otlp/collector.go | 2 +- .../infraattributesprocessor/helperclients.go | 2 +- .../infraattributesprocessor/helperclients_test.go | 4 ++-- .../processor/infraattributesprocessor/logs.go | 2 +- .../processor/infraattributesprocessor/metrics.go | 2 +- .../processor/infraattributesprocessor/traces.go | 2 +- comp/trace/config/setup.go | 2 +- .../k8s/pod_tag_provider/node_provider.go | 2 +- .../cluster/orchestrator/transformers/ecs/task.go | 2 +- .../corechecks/containerimage/processor.go | 2 +- .../corechecks/containers/containerd/events.go | 2 +- .../corechecks/containers/docker/check.go | 2 +- .../corechecks/containers/docker/eventbundle.go | 2 +- .../corechecks/containers/docker/events.go | 2 +- .../containers/docker/unbundled_events.go | 2 +- .../corechecks/containers/docker/utils.go | 2 +- .../corechecks/containers/generic/processor.go | 2 +- .../containers/generic/processor_network.go | 2 +- .../containers/generic/processor_network_test.go | 14 +++++++------- .../corechecks/containers/kubelet/common/pod.go | 6 ++---- .../kubelet/provider/cadvisor/provider.go | 8 ++++---- .../kubelet/provider/kubelet/provider.go | 2 +- .../containers/kubelet/provider/pod/provider.go | 8 ++++---- .../containers/kubelet/provider/probe/provider.go | 2 +- .../kubelet/provider/summary/provider.go | 4 ++-- pkg/collector/corechecks/ebpf/oomkill/oom_kill.go | 4 ++-- .../ebpf/tcpqueuelength/tcp_queue_length.go | 4 ++-- pkg/collector/corechecks/sbom/processor.go | 2 +- pkg/collector/python/tagger.go | 2 +- pkg/collector/python/test_tagger.go | 6 +++--- pkg/logs/internal/tag/provider.go | 8 ++++---- pkg/logs/internal/tag/provider_benchmark_test.go | 12 ++++++------ pkg/logs/internal/tag/provider_test.go | 3 ++- pkg/logs/tailers/docker/tailer.go | 2 +- pkg/logs/tailers/file/tailer.go | 2 +- pkg/logs/tailers/journald/docker.go | 2 +- pkg/process/util/containers/containers.go | 2 +- pkg/security/resolvers/tags/resolver.go | 8 ++++---- pkg/util/kubernetes/kubelet/kubelet_common.go | 14 +++++++------- pkg/util/kubernetes/kubelet/kubelet_common_test.go | 6 +++--- 41 files changed, 80 insertions(+), 81 deletions(-) diff --git a/comp/core/autodiscovery/listeners/service.go b/comp/core/autodiscovery/listeners/service.go index 92c8d85f6ea58..d2d47eb8d1add 100644 --- a/comp/core/autodiscovery/listeners/service.go +++ b/comp/core/autodiscovery/listeners/service.go @@ -90,7 +90,7 @@ func (s *service) GetPorts(_ context.Context) ([]ContainerPort, error) { // GetTags returns the tags associated with the service. func (s *service) GetTags() ([]string, error) { - return tagger.Tag(taggercommon.BuildTaggerEntityID(s.entity.GetID()).String(), tagger.ChecksCardinality()) + return tagger.Tag(taggercommon.BuildTaggerEntityID(s.entity.GetID()), tagger.ChecksCardinality()) } // GetTagsWithCardinality returns the tags with given cardinality. diff --git a/comp/otelcol/otlp/collector.go b/comp/otelcol/otlp/collector.go index 2ce66bf173972..6207e329a100a 100644 --- a/comp/otelcol/otlp/collector.go +++ b/comp/otelcol/otlp/collector.go @@ -70,7 +70,7 @@ func (t *tagEnricher) Enrich(_ context.Context, extraTags []string, dimensions * enrichedTags = append(enrichedTags, extraTags...) enrichedTags = append(enrichedTags, dimensions.Tags()...) - entityTags, err := tagger.Tag(dimensions.OriginID(), t.cardinality) + entityTags, err := tagger.LegacyTag(dimensions.OriginID(), t.cardinality) if err != nil { log.Tracef("Cannot get tags for entity %s: %s", dimensions.OriginID(), err) } else { diff --git a/comp/otelcol/otlp/components/processor/infraattributesprocessor/helperclients.go b/comp/otelcol/otlp/components/processor/infraattributesprocessor/helperclients.go index 297e4f724f3b3..cd6e1cb7de8a9 100644 --- a/comp/otelcol/otlp/components/processor/infraattributesprocessor/helperclients.go +++ b/comp/otelcol/otlp/components/processor/infraattributesprocessor/helperclients.go @@ -11,7 +11,7 @@ import "github.com/DataDog/datadog-agent/comp/core/tagger/types" // see comp/core/tagger for tagger functions; client for tagger interface type taggerClient interface { // Tag is an interface function that queries taggerclient singleton - Tag(entity string, cardinality types.TagCardinality) ([]string, error) + Tag(entity types.EntityID, cardinality types.TagCardinality) ([]string, error) // GlobalTags is an interface function that queries taggerclient singleton GlobalTags(cardinality types.TagCardinality) ([]string, error) } diff --git a/comp/otelcol/otlp/components/processor/infraattributesprocessor/helperclients_test.go b/comp/otelcol/otlp/components/processor/infraattributesprocessor/helperclients_test.go index 989623475184a..3069eec0acd56 100644 --- a/comp/otelcol/otlp/components/processor/infraattributesprocessor/helperclients_test.go +++ b/comp/otelcol/otlp/components/processor/infraattributesprocessor/helperclients_test.go @@ -24,8 +24,8 @@ func newTestTaggerClient() *testTaggerClient { } // Tag mocks taggerimpl.Tag functionality for the purpose of testing, removing dependency on Taggerimpl -func (t *testTaggerClient) Tag(entityID string, _ types.TagCardinality) ([]string, error) { - return t.tagMap[entityID], nil +func (t *testTaggerClient) Tag(entityID types.EntityID, _ types.TagCardinality) ([]string, error) { + return t.tagMap[entityID.String()], nil } // GlobalTags mocks taggerimpl.GlobalTags functionality for purpose of testing, removing dependency on Taggerimpl diff --git a/comp/otelcol/otlp/components/processor/infraattributesprocessor/logs.go b/comp/otelcol/otlp/components/processor/infraattributesprocessor/logs.go index 8d63b39c3754b..0ff72e0b56605 100644 --- a/comp/otelcol/otlp/components/processor/infraattributesprocessor/logs.go +++ b/comp/otelcol/otlp/components/processor/infraattributesprocessor/logs.go @@ -43,7 +43,7 @@ func (ialp *infraAttributesLogProcessor) processLogs(_ context.Context, ld plog. // Get all unique tags from resource attributes and global tags for _, entityID := range entityIDs { - entityTags, err := ialp.tagger.Tag(entityID.String(), ialp.cardinality) + entityTags, err := ialp.tagger.Tag(entityID, ialp.cardinality) if err != nil { ialp.logger.Error("Cannot get tags for entity", zap.String("entityID", entityID.String()), zap.Error(err)) continue diff --git a/comp/otelcol/otlp/components/processor/infraattributesprocessor/metrics.go b/comp/otelcol/otlp/components/processor/infraattributesprocessor/metrics.go index e7a8cb3d7a700..8b7f5f14d8e49 100644 --- a/comp/otelcol/otlp/components/processor/infraattributesprocessor/metrics.go +++ b/comp/otelcol/otlp/components/processor/infraattributesprocessor/metrics.go @@ -97,7 +97,7 @@ func (iamp *infraAttributesMetricProcessor) processMetrics(_ context.Context, md // Get all unique tags from resource attributes and global tags for _, entityID := range entityIDs { - entityTags, err := iamp.tagger.Tag(entityID.String(), iamp.cardinality) + entityTags, err := iamp.tagger.Tag(entityID, iamp.cardinality) if err != nil { iamp.logger.Error("Cannot get tags for entity", zap.String("entityID", entityID.String()), zap.Error(err)) continue diff --git a/comp/otelcol/otlp/components/processor/infraattributesprocessor/traces.go b/comp/otelcol/otlp/components/processor/infraattributesprocessor/traces.go index ce6fe02674e95..eca0363e4ab9d 100644 --- a/comp/otelcol/otlp/components/processor/infraattributesprocessor/traces.go +++ b/comp/otelcol/otlp/components/processor/infraattributesprocessor/traces.go @@ -42,7 +42,7 @@ func (iasp *infraAttributesSpanProcessor) processTraces(_ context.Context, td pt // Get all unique tags from resource attributes and global tags for _, entityID := range entityIDs { - entityTags, err := iasp.tagger.Tag(entityID.String(), iasp.cardinality) + entityTags, err := iasp.tagger.Tag(entityID, iasp.cardinality) if err != nil { iasp.logger.Error("Cannot get tags for entity", zap.String("entityID", entityID.String()), zap.Error(err)) continue diff --git a/comp/trace/config/setup.go b/comp/trace/config/setup.go index ab9361127c633..6bd3be6278696 100644 --- a/comp/trace/config/setup.go +++ b/comp/trace/config/setup.go @@ -123,7 +123,7 @@ func prepareConfig(c corecompcfg.Component) (*config.AgentConfig, error) { } func containerTagsFunc(cid string) ([]string, error) { - return tagger.Tag(types.NewEntityID(types.ContainerID, cid).String(), types.HighCardinality) + return tagger.Tag(types.NewEntityID(types.ContainerID, cid), types.HighCardinality) } // appendEndpoints appends any endpoint configuration found at the given cfgKey. diff --git a/pkg/collector/corechecks/cluster/orchestrator/processors/k8s/pod_tag_provider/node_provider.go b/pkg/collector/corechecks/cluster/orchestrator/processors/k8s/pod_tag_provider/node_provider.go index 484fc20efb093..9235a7e02de58 100644 --- a/pkg/collector/corechecks/cluster/orchestrator/processors/k8s/pod_tag_provider/node_provider.go +++ b/pkg/collector/corechecks/cluster/orchestrator/processors/k8s/pod_tag_provider/node_provider.go @@ -20,5 +20,5 @@ func newNodePodTagProvider() PodTagProvider { return &nodePodTagProvider{} } // GetTags implements PodTagProvider#GetTags func (p *nodePodTagProvider) GetTags(pod *corev1.Pod, cardinality taggertypes.TagCardinality) ([]string, error) { - return tagger.Tag(taggertypes.NewEntityID(taggertypes.KubernetesPodUID, string(pod.UID)).String(), cardinality) + return tagger.Tag(taggertypes.NewEntityID(taggertypes.KubernetesPodUID, string(pod.UID)), cardinality) } diff --git a/pkg/collector/corechecks/cluster/orchestrator/transformers/ecs/task.go b/pkg/collector/corechecks/cluster/orchestrator/transformers/ecs/task.go index 0526ab2d659b1..4f01d045b3514 100644 --- a/pkg/collector/corechecks/cluster/orchestrator/transformers/ecs/task.go +++ b/pkg/collector/corechecks/cluster/orchestrator/transformers/ecs/task.go @@ -58,7 +58,7 @@ func ExtractECSTask(task TaskWithContainers) *model.ECSTask { } entityID := types.NewEntityID(types.ECSTask, task.Task.EntityID.ID) - tags, err := tagger.Tag(entityID.String(), types.HighCardinality) + tags, err := tagger.Tag(entityID, types.HighCardinality) if err != nil { log.Debugf("Could not retrieve tags for task: %s", err.Error()) } diff --git a/pkg/collector/corechecks/containerimage/processor.go b/pkg/collector/corechecks/containerimage/processor.go index 668a84fd93bfa..c9dcea2cefaeb 100644 --- a/pkg/collector/corechecks/containerimage/processor.go +++ b/pkg/collector/corechecks/containerimage/processor.go @@ -75,7 +75,7 @@ func (p *processor) processRefresh(allImages []*workloadmeta.ContainerImageMetad func (p *processor) processImage(img *workloadmeta.ContainerImageMetadata) { entityID := types.NewEntityID(types.ContainerImageMetadata, img.ID) - ddTags, err := tagger.Tag(entityID.String(), types.HighCardinality) + ddTags, err := tagger.Tag(entityID, types.HighCardinality) if err != nil { log.Errorf("Could not retrieve tags for container image %s: %v", img.ID, err) } diff --git a/pkg/collector/corechecks/containers/containerd/events.go b/pkg/collector/corechecks/containers/containerd/events.go index 71c14aa585ac5..91c30213b93f4 100644 --- a/pkg/collector/corechecks/containers/containerd/events.go +++ b/pkg/collector/corechecks/containers/containerd/events.go @@ -52,7 +52,7 @@ func computeEvents(events []containerdEvent, sender sender.Sender, fil *containe alertType := event.AlertTypeInfo if split[1] == "containers" || split[1] == "tasks" { // For task events, we use the container ID in order to query the Tagger's API - t, err := tagger.Tag(types.NewEntityID(types.ContainerID, e.ID).String(), types.HighCardinality) + t, err := tagger.Tag(types.NewEntityID(types.ContainerID, e.ID), types.HighCardinality) if err != nil { // If there is an error retrieving tags from the Tagger, we can still submit the event as is. log.Errorf("Could not retrieve tags for the container %s: %v", e.ID, err) diff --git a/pkg/collector/corechecks/containers/docker/check.go b/pkg/collector/corechecks/containers/docker/check.go index 1fea8ba33f979..036e3a4e748e5 100644 --- a/pkg/collector/corechecks/containers/docker/check.go +++ b/pkg/collector/corechecks/containers/docker/check.go @@ -239,7 +239,7 @@ func (d *DockerCheck) runDockerCustom(sender sender.Sender, du docker.Client, ra isContainerExcluded := d.containerFilter.IsExcluded(annotations, containerName, resolvedImageName, rawContainer.Labels[kubernetes.CriContainerNamespaceLabel]) isContainerRunning := rawContainer.State == string(workloadmeta.ContainerStatusRunning) - taggerEntityID := types.NewEntityID(types.ContainerID, rawContainer.ID).String() + taggerEntityID := types.NewEntityID(types.ContainerID, rawContainer.ID) tags, err := getImageTagsFromContainer(taggerEntityID, resolvedImageName, isContainerExcluded || !isContainerRunning) if err != nil { log.Debugf("Unable to fetch tags for image: %s, err: %v", rawContainer.ImageID, err) diff --git a/pkg/collector/corechecks/containers/docker/eventbundle.go b/pkg/collector/corechecks/containers/docker/eventbundle.go index 6f67d03d7a29a..4dfeac8f769af 100644 --- a/pkg/collector/corechecks/containers/docker/eventbundle.go +++ b/pkg/collector/corechecks/containers/docker/eventbundle.go @@ -93,7 +93,7 @@ func (b *dockerEventBundle) toDatadogEvent(hostname string) (event.Event, error) for cid := range seenContainers { - tags, err := tagger.Tag(types.NewEntityID(types.ContainerID, cid).String(), types.HighCardinality) + tags, err := tagger.Tag(types.NewEntityID(types.ContainerID, cid), types.HighCardinality) if err != nil { log.Debugf("no tags for %s: %s", cid, err) } else { diff --git a/pkg/collector/corechecks/containers/docker/events.go b/pkg/collector/corechecks/containers/docker/events.go index b492305983408..a1dbc0337df93 100644 --- a/pkg/collector/corechecks/containers/docker/events.go +++ b/pkg/collector/corechecks/containers/docker/events.go @@ -83,7 +83,7 @@ func (d *DockerCheck) reportExitCodes(events []*docker.ContainerEvent, sender se status = servicecheck.ServiceCheckCritical } - tags, err := tagger.Tag(types.NewEntityID(types.ContainerID, ev.ContainerID).String(), types.HighCardinality) + tags, err := tagger.Tag(types.NewEntityID(types.ContainerID, ev.ContainerID), types.HighCardinality) if err != nil { log.Debugf("no tags for %s: %s", ev.ContainerID, err) tags = []string{} diff --git a/pkg/collector/corechecks/containers/docker/unbundled_events.go b/pkg/collector/corechecks/containers/docker/unbundled_events.go index ddb763e97f77f..19c2273ebb9c7 100644 --- a/pkg/collector/corechecks/containers/docker/unbundled_events.go +++ b/pkg/collector/corechecks/containers/docker/unbundled_events.go @@ -57,7 +57,7 @@ func (t *unbundledTransformer) Transform(events []*docker.ContainerEvent) ([]eve emittedEvents.Inc(string(alertType)) tags, err := tagger.Tag( - types.NewEntityID(types.ContainerID, ev.ContainerID).String(), + types.NewEntityID(types.ContainerID, ev.ContainerID), types.HighCardinality, ) if err != nil { diff --git a/pkg/collector/corechecks/containers/docker/utils.go b/pkg/collector/corechecks/containers/docker/utils.go index 500cf6cdde2d7..3b3990cbfdf8f 100644 --- a/pkg/collector/corechecks/containers/docker/utils.go +++ b/pkg/collector/corechecks/containers/docker/utils.go @@ -30,7 +30,7 @@ func getProcessorFilter(legacyFilter *containers.Filter, store workloadmeta.Comp } } -func getImageTagsFromContainer(taggerEntityID string, resolvedImageName string, isContainerExcluded bool) ([]string, error) { +func getImageTagsFromContainer(taggerEntityID types.EntityID, resolvedImageName string, isContainerExcluded bool) ([]string, error) { if isContainerExcluded { return getImageTags(resolvedImageName) } diff --git a/pkg/collector/corechecks/containers/generic/processor.go b/pkg/collector/corechecks/containers/generic/processor.go index a40fe1512f4e3..44612d6839c52 100644 --- a/pkg/collector/corechecks/containers/generic/processor.go +++ b/pkg/collector/corechecks/containers/generic/processor.go @@ -73,7 +73,7 @@ func (p *Processor) Run(sender sender.Sender, cacheValidity time.Duration) error entityID := types.NewEntityID(types.ContainerID, container.ID) - tags, err := tagger.Tag(entityID.String(), types.HighCardinality) + tags, err := tagger.Tag(entityID, types.HighCardinality) if err != nil { log.Errorf("Could not collect tags for container %q, err: %v", container.ID[:12], err) continue diff --git a/pkg/collector/corechecks/containers/generic/processor_network.go b/pkg/collector/corechecks/containers/generic/processor_network.go index d83fd0f215043..52b992dee3937 100644 --- a/pkg/collector/corechecks/containers/generic/processor_network.go +++ b/pkg/collector/corechecks/containers/generic/processor_network.go @@ -98,7 +98,7 @@ func (pn *ProcessorNetwork) processGroupedContainerNetwork() { pn.generateNetworkMetrics(containerNetworks.tags, containerNetworks.stats) } else if containerNetworks.owner != nil && containerNetworks.owner.Kind == workloadmeta.KindKubernetesPod { podEntityID := types.NewEntityID(types.KubernetesPodUID, containerNetworks.owner.ID) - orchTags, err := tagger.Tag(podEntityID.String(), types.HighCardinality) + orchTags, err := tagger.Tag(podEntityID, types.HighCardinality) if err != nil { log.Debugf("Unable to get orchestrator tags for pod: %s", containerNetworks.owner.ID) continue diff --git a/pkg/collector/corechecks/containers/generic/processor_network_test.go b/pkg/collector/corechecks/containers/generic/processor_network_test.go index fee5d7c42cf67..a6aa31341e2b5 100644 --- a/pkg/collector/corechecks/containers/generic/processor_network_test.go +++ b/pkg/collector/corechecks/containers/generic/processor_network_test.go @@ -192,19 +192,19 @@ func TestNetworkProcessorExtension(t *testing.T) { // Running them through the ProcessorExtension networkProcessor.PreProcess(MockSendMetric, mockSender) - container1Tags, _ := fakeTagger.Tag("container_id://1", types.HighCardinality) + container1Tags, _ := fakeTagger.Tag(types.NewEntityID(types.ContainerID, "1"), types.HighCardinality) networkProcessor.Process(container1Tags, container1, mockCollector, 0) - container2Tags, _ := fakeTagger.Tag("container_id://2", types.HighCardinality) + container2Tags, _ := fakeTagger.Tag(types.NewEntityID(types.ContainerID, "2"), types.HighCardinality) networkProcessor.Process(container2Tags, container2, mockCollector, 0) - container3Tags, _ := fakeTagger.Tag("container_id://3", types.HighCardinality) + container3Tags, _ := fakeTagger.Tag(types.NewEntityID(types.ContainerID, "3"), types.HighCardinality) networkProcessor.Process(container3Tags, container3, mockCollector, 0) - container4Tags, _ := fakeTagger.Tag("container_id://4", types.HighCardinality) + container4Tags, _ := fakeTagger.Tag(types.NewEntityID(types.ContainerID, "4"), types.HighCardinality) networkProcessor.Process(container4Tags, container4, mockCollector, 0) - container5Tags, _ := fakeTagger.Tag("container_id://5", types.HighCardinality) + container5Tags, _ := fakeTagger.Tag(types.NewEntityID(types.ContainerID, "5"), types.HighCardinality) networkProcessor.Process(container5Tags, container5, mockCollector, 0) - container6Tags, _ := fakeTagger.Tag("container_id://6", types.HighCardinality) + container6Tags, _ := fakeTagger.Tag(types.NewEntityID(types.ContainerID, "6"), types.HighCardinality) networkProcessor.Process(container6Tags, container6, mockCollector, 0) - container7Tags, _ := fakeTagger.Tag("container_id://7", types.HighCardinality) + container7Tags, _ := fakeTagger.Tag(types.NewEntityID(types.ContainerID, "7"), types.HighCardinality) networkProcessor.Process(container7Tags, container7, mockCollector, 0) networkProcessor.PostProcess() diff --git a/pkg/collector/corechecks/containers/kubelet/common/pod.go b/pkg/collector/corechecks/containers/kubelet/common/pod.go index 7949d89e5f8a0..54a105c90bab2 100644 --- a/pkg/collector/corechecks/containers/kubelet/common/pod.go +++ b/pkg/collector/corechecks/containers/kubelet/common/pod.go @@ -85,7 +85,7 @@ func (p *PodUtils) PopulateForPod(pod *kubelet.Pod) { // computePodTagsByPVC stores the tags for a given pod in a global caching layer, indexed by pod namespace and persistent // volume name. func (p *PodUtils) computePodTagsByPVC(pod *kubelet.Pod) { - podUID := types.NewEntityID(types.KubernetesPodUID, pod.Metadata.UID).String() + podUID := types.NewEntityID(types.KubernetesPodUID, pod.Metadata.UID) tags, _ := tagger.Tag(podUID, types.OrchestratorCardinality) if len(tags) == 0 { return @@ -192,7 +192,5 @@ func GetContainerID(store workloadmeta.Component, metric model.Metric, filter *c return "", ErrContainerExcluded } - cID := types.NewEntityID(types.ContainerID, container.ID).String() - - return cID, nil + return container.ID, nil } diff --git a/pkg/collector/corechecks/containers/kubelet/provider/cadvisor/provider.go b/pkg/collector/corechecks/containers/kubelet/provider/cadvisor/provider.go index d32cd95ab2027..7d2a65ad04f71 100644 --- a/pkg/collector/corechecks/containers/kubelet/provider/cadvisor/provider.go +++ b/pkg/collector/corechecks/containers/kubelet/provider/cadvisor/provider.go @@ -152,7 +152,7 @@ func (p *Provider) processContainerMetric(metricType, metricName string, metricF // for static pods, see https://github.com/kubernetes/kubernetes/pull/59948 pod := p.getPodByMetricLabel(sample.Metric) if pod != nil && p.podUtils.IsStaticPendingPod(pod.ID) { - podTags, _ := tagger.Tag(taggercommon.BuildTaggerEntityID(pod.GetID()).String(), types.HighCardinality) + podTags, _ := tagger.Tag(taggercommon.BuildTaggerEntityID(pod.GetID()), types.HighCardinality) if len(podTags) == 0 { continue } @@ -208,7 +208,7 @@ func (p *Provider) processPodRate(metricName string, metricFam *prom.MetricFamil continue } entityID := taggercommon.BuildTaggerEntityID(pod.GetID()) - tags, _ := tagger.Tag(entityID.String(), types.HighCardinality) + tags, _ := tagger.Tag(entityID, types.HighCardinality) if len(tags) == 0 { continue } @@ -250,7 +250,7 @@ func (p *Provider) processUsageMetric(metricName string, metricFam *prom.MetricF pod := p.getPodByMetricLabel(sample.Metric) if pod != nil && p.podUtils.IsStaticPendingPod(pod.ID) { entityID := taggercommon.BuildTaggerEntityID(pod.EntityID) - podTags, _ := tagger.Tag(entityID.String(), types.HighCardinality) + podTags, _ := tagger.Tag(entityID, types.HighCardinality) if len(podTags) == 0 { continue } @@ -352,7 +352,7 @@ func (p *Provider) getEntityIDIfContainerMetric(labels model.Metric) string { return p.getPodUID(labels) } cID, _ := common.GetContainerID(p.store, labels, p.filter) - return cID + return types.NewEntityID(types.ContainerID, cID).String() } return "" } diff --git a/pkg/collector/corechecks/containers/kubelet/provider/kubelet/provider.go b/pkg/collector/corechecks/containers/kubelet/provider/kubelet/provider.go index ea3d40af9c990..2307903d85cd0 100644 --- a/pkg/collector/corechecks/containers/kubelet/provider/kubelet/provider.go +++ b/pkg/collector/corechecks/containers/kubelet/provider/kubelet/provider.go @@ -185,7 +185,7 @@ func (p *Provider) kubeletContainerLogFilesystemUsedBytes(metricFam *prom.Metric continue } - tags, _ := tagger.Tag(cID, types.HighCardinality) + tags, _ := tagger.Tag(types.NewEntityID(types.ContainerID, cID), types.HighCardinality) if len(tags) == 0 { log.Debugf("Tags not found for container: %s/%s/%s:%s", metric.Metric["namespace"], metric.Metric["pod"], metric.Metric["container"], cID) } diff --git a/pkg/collector/corechecks/containers/kubelet/provider/pod/provider.go b/pkg/collector/corechecks/containers/kubelet/provider/pod/provider.go index aa2a19af797c5..38ee589c389cf 100644 --- a/pkg/collector/corechecks/containers/kubelet/provider/pod/provider.go +++ b/pkg/collector/corechecks/containers/kubelet/provider/pod/provider.go @@ -116,7 +116,7 @@ func (p *Provider) Provide(kc kubelet.KubeUtilInterface, sender sender.Sender) e return nil } -func (p *Provider) generateContainerSpecMetrics(sender sender.Sender, pod *kubelet.Pod, container *kubelet.ContainerSpec, cStatus *kubelet.ContainerStatus, containerID string) { +func (p *Provider) generateContainerSpecMetrics(sender sender.Sender, pod *kubelet.Pod, container *kubelet.ContainerSpec, cStatus *kubelet.ContainerStatus, containerID types.EntityID) { if pod.Status.Phase != "Running" && pod.Status.Phase != "Pending" { return } @@ -140,7 +140,7 @@ func (p *Provider) generateContainerSpecMetrics(sender sender.Sender, pod *kubel } } -func (p *Provider) generateContainerStatusMetrics(sender sender.Sender, pod *kubelet.Pod, _ *kubelet.ContainerSpec, cStatus *kubelet.ContainerStatus, containerID string) { +func (p *Provider) generateContainerStatusMetrics(sender sender.Sender, pod *kubelet.Pod, _ *kubelet.ContainerSpec, cStatus *kubelet.ContainerStatus, containerID types.EntityID) { if pod.Metadata.UID == "" || pod.Metadata.Name == "" { return } @@ -184,7 +184,7 @@ func newRunningAggregator() *runningAggregator { } } -func (r *runningAggregator) recordContainer(p *Provider, pod *kubelet.Pod, cStatus *kubelet.ContainerStatus, containerID string) { +func (r *runningAggregator) recordContainer(p *Provider, pod *kubelet.Pod, cStatus *kubelet.ContainerStatus, containerID types.EntityID) { if cStatus.State.Running == nil || time.Time.IsZero(cStatus.State.Running.StartedAt) { return } @@ -210,7 +210,7 @@ func (r *runningAggregator) recordPod(p *Provider, pod *kubelet.Pod) { log.Debug("skipping pod with no uid") return } - entityID := types.NewEntityID(types.KubernetesPodUID, podID).String() + entityID := types.NewEntityID(types.KubernetesPodUID, podID) tagList, _ := tagger.Tag(entityID, types.LowCardinality) if len(tagList) == 0 { return diff --git a/pkg/collector/corechecks/containers/kubelet/provider/probe/provider.go b/pkg/collector/corechecks/containers/kubelet/provider/probe/provider.go index f6f1f2ae62255..ca8d8369e3009 100644 --- a/pkg/collector/corechecks/containers/kubelet/provider/probe/provider.go +++ b/pkg/collector/corechecks/containers/kubelet/provider/probe/provider.go @@ -88,7 +88,7 @@ func (p *Provider) proberProbeTotal(metricFam *prom.MetricFamily, sender sender. continue } - tags, _ := tagger.Tag(cID, types.HighCardinality) + tags, _ := tagger.Tag(types.NewEntityID(types.ContainerID, cID), types.HighCardinality) if len(tags) == 0 { continue } diff --git a/pkg/collector/corechecks/containers/kubelet/provider/summary/provider.go b/pkg/collector/corechecks/containers/kubelet/provider/summary/provider.go index 6e2d330d66d7d..37635048ad41d 100644 --- a/pkg/collector/corechecks/containers/kubelet/provider/summary/provider.go +++ b/pkg/collector/corechecks/containers/kubelet/provider/summary/provider.go @@ -150,7 +150,7 @@ func (p *Provider) processPodStats(sender sender.Sender, } entityID := types.NewEntityID(types.KubernetesPodUID, podStats.PodRef.UID) - podTags, _ := tagger.Tag(entityID.String(), + podTags, _ := tagger.Tag(entityID, types.OrchestratorCardinality) if len(podTags) == 0 { @@ -221,7 +221,7 @@ func (p *Provider) processContainerStats(sender sender.Sender, podStats.PodRef.Namespace) { continue } - tags, err := tagger.Tag(types.NewEntityID(types.ContainerID, ctr.ID).String(), types.HighCardinality) + tags, err := tagger.Tag(types.NewEntityID(types.ContainerID, ctr.ID), types.HighCardinality) if err != nil || len(tags) == 0 { log.Debugf("Tags not found for container: %s/%s/%s:%s - no metrics will be sent", podStats.PodRef.Namespace, podStats.PodRef.Name, containerName, ctr.ID) diff --git a/pkg/collector/corechecks/ebpf/oomkill/oom_kill.go b/pkg/collector/corechecks/ebpf/oomkill/oom_kill.go index 0a5b506ce1e7b..fa511a4e66db5 100644 --- a/pkg/collector/corechecks/ebpf/oomkill/oom_kill.go +++ b/pkg/collector/corechecks/ebpf/oomkill/oom_kill.go @@ -115,9 +115,9 @@ func (m *OOMKillCheck) Run() error { log.Debugf("Unable to extract containerID from cgroup name: %s, err: %v", line.CgroupName, err) } - entityID := types.NewEntityID(types.ContainerID, containerID).String() + entityID := types.NewEntityID(types.ContainerID, containerID) var tags []string - if entityID != "" { + 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 126f949876c87..8a8b8c9cd5bb7 100644 --- a/pkg/collector/corechecks/ebpf/tcpqueuelength/tcp_queue_length.go +++ b/pkg/collector/corechecks/ebpf/tcpqueuelength/tcp_queue_length.go @@ -110,9 +110,9 @@ func (t *TCPQueueLengthCheck) Run() error { continue } - entityID := types.NewEntityID(types.ContainerID, containerID).String() + entityID := types.NewEntityID(types.ContainerID, containerID) var tags []string - if entityID != "" { + if entityID.Empty() { tags, err = tagger.Tag(entityID, types.HighCardinality) if err != nil { log.Errorf("Error collecting tags for container %s: %s", k, err) diff --git a/pkg/collector/corechecks/sbom/processor.go b/pkg/collector/corechecks/sbom/processor.go index 91bfeddccf531..b26e2eaebc596 100644 --- a/pkg/collector/corechecks/sbom/processor.go +++ b/pkg/collector/corechecks/sbom/processor.go @@ -242,7 +242,7 @@ func (p *processor) processImageSBOM(img *workloadmeta.ContainerImageMetadata) { return } - entityID := types.NewEntityID(types.ContainerImageMetadata, img.ID).String() + entityID := types.NewEntityID(types.ContainerImageMetadata, img.ID) ddTags, err := tagger.Tag(entityID, types.HighCardinality) if err != nil { log.Errorf("Could not retrieve tags for container image %s: %v", img.ID, err) diff --git a/pkg/collector/python/tagger.go b/pkg/collector/python/tagger.go index e572e7474a935..0638963dea642 100644 --- a/pkg/collector/python/tagger.go +++ b/pkg/collector/python/tagger.go @@ -26,7 +26,7 @@ import "C" // for testing purposes var ( - tagsFunc = tagger.TagDeprecated + tagsFunc = tagger.LegacyTag ) // Tags bridges towards tagger.Tag to retrieve container tags diff --git a/pkg/collector/python/test_tagger.go b/pkg/collector/python/test_tagger.go index f3acbf2e03c32..bbeff9e0dfd7e 100644 --- a/pkg/collector/python/test_tagger.go +++ b/pkg/collector/python/test_tagger.go @@ -49,7 +49,7 @@ func tagsMockEmpty(string, types.TagCardinality) ([]string, error) { func testTags(t *testing.T) { tagsFunc = tagsMock - defer func() { tagsFunc = tagger.Tag }() + defer func() { tagsFunc = tagger.LegacyTag }() id := C.CString("test") defer C.free(unsafe.Pointer(id)) @@ -68,7 +68,7 @@ func testTags(t *testing.T) { func testTagsNull(t *testing.T) { tagsFunc = tagsMockNull - defer func() { tagsFunc = tagger.Tag }() + defer func() { tagsFunc = tagger.LegacyTag }() id := C.CString("test") defer C.free(unsafe.Pointer(id)) @@ -79,7 +79,7 @@ func testTagsNull(t *testing.T) { func testTagsEmpty(t *testing.T) { tagsFunc = tagsMockEmpty - defer func() { tagsFunc = tagger.Tag }() + defer func() { tagsFunc = tagger.LegacyTag }() id := C.CString("test") defer C.free(unsafe.Pointer(id)) diff --git a/pkg/logs/internal/tag/provider.go b/pkg/logs/internal/tag/provider.go index 7196bacb02646..1a78e2f2d11e9 100644 --- a/pkg/logs/internal/tag/provider.go +++ b/pkg/logs/internal/tag/provider.go @@ -24,12 +24,12 @@ type Provider interface { // EntityTagAdder returns the associated tag for an entity and their cardinality type EntityTagAdder interface { - Tag(entity string, cardinality types.TagCardinality) ([]string, error) + Tag(entity types.EntityID, cardinality types.TagCardinality) ([]string, error) } // provider provides a list of up-to-date tags for a given entity by calling the tagger. type provider struct { - entityID string + entityID types.EntityID taggerWarmupDuration time.Duration localTagProvider Provider clock clock.Clock @@ -38,12 +38,12 @@ type provider struct { } // NewProvider returns a new Provider. -func NewProvider(entityID string, tagAdder EntityTagAdder) Provider { +func NewProvider(entityID types.EntityID, tagAdder EntityTagAdder) Provider { return newProviderWithClock(entityID, clock.New(), tagAdder) } // newProviderWithClock returns a new provider using the given clock. -func newProviderWithClock(entityID string, clock clock.Clock, tagAdder EntityTagAdder) Provider { +func newProviderWithClock(entityID types.EntityID, clock clock.Clock, tagAdder EntityTagAdder) Provider { p := &provider{ entityID: entityID, taggerWarmupDuration: config.TaggerWarmupDuration(pkgconfigsetup.Datadog()), diff --git a/pkg/logs/internal/tag/provider_benchmark_test.go b/pkg/logs/internal/tag/provider_benchmark_test.go index 8b2ece36e2d83..ce47807725883 100644 --- a/pkg/logs/internal/tag/provider_benchmark_test.go +++ b/pkg/logs/internal/tag/provider_benchmark_test.go @@ -28,7 +28,7 @@ func setupConfig(t testing.TB, tags []string) (model.Config, time.Time) { type dummyTagAdder struct{} -func (dummyTagAdder) Tag(string, types.TagCardinality) ([]string, error) { +func (dummyTagAdder) Tag(types.EntityID, types.TagCardinality) ([]string, error) { return nil, nil } @@ -46,7 +46,7 @@ func BenchmarkProviderExpectedTags(b *testing.B) { m.SetWithoutSource("logs_config.expected_tags_duration", "1m") defer m.SetWithoutSource("logs_config.expected_tags_duration", 0) - p := NewProvider("foo", dummyTagAdder{}) + p := NewProvider(types.NewEntityID(types.ContainerID, "foo"), dummyTagAdder{}) for i := 0; i < b.N; i++ { p.GetTags() @@ -69,7 +69,7 @@ func BenchmarkProviderExpectedTagsEmptySlice(b *testing.B) { m.SetWithoutSource("logs_config.expected_tags_duration", "1m") defer m.SetWithoutSource("logs_config.expected_tags_duration", 0) - p := NewProvider("foo", dummyTagAdder{}) + p := NewProvider(types.NewEntityID(types.ContainerID, "foo"), dummyTagAdder{}) for i := 0; i < b.N; i++ { p.GetTags() @@ -92,7 +92,7 @@ func BenchmarkProviderExpectedTagsNil(b *testing.B) { m.SetWithoutSource("logs_config.expected_tags_duration", "1m") defer m.SetWithoutSource("logs_config.expected_tags_duration", 0) - p := NewProvider("foo", dummyTagAdder{}) + p := NewProvider(types.NewEntityID(types.ContainerID, "foo"), dummyTagAdder{}) for i := 0; i < b.N; i++ { p.GetTags() @@ -112,7 +112,7 @@ func BenchmarkProviderNoExpectedTags(b *testing.B) { // Setting a test-friendly value for the deadline (test should not take 1m) m.SetWithoutSource("logs_config.expected_tags_duration", "0") - p := NewProvider("foo", dummyTagAdder{}) + p := NewProvider(types.NewEntityID(types.ContainerID, "foo"), dummyTagAdder{}) for i := 0; i < b.N; i++ { p.GetTags() @@ -132,7 +132,7 @@ func BenchmarkProviderNoExpectedTagsNil(b *testing.B) { // Setting a test-friendly value for the deadline (test should not take 1m) m.SetWithoutSource("logs_config.expected_tags_duration", "0") - p := NewProvider("foo", dummyTagAdder{}) + p := NewProvider(types.NewEntityID(types.ContainerID, "foo"), dummyTagAdder{}) for i := 0; i < b.N; i++ { p.GetTags() diff --git a/pkg/logs/internal/tag/provider_test.go b/pkg/logs/internal/tag/provider_test.go index b238d9c58b542..3b7b361ec550d 100644 --- a/pkg/logs/internal/tag/provider_test.go +++ b/pkg/logs/internal/tag/provider_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/require" "github.com/DataDog/datadog-agent/comp/core/tagger/taggerimpl" + "github.com/DataDog/datadog-agent/comp/core/tagger/types" configmock "github.com/DataDog/datadog-agent/pkg/config/mock" pkgconfigsetup "github.com/DataDog/datadog-agent/pkg/config/setup" @@ -41,7 +42,7 @@ func TestProviderExpectedTags(t *testing.T) { m.SetWithoutSource("logs_config.expected_tags_duration", "5s") defer m.SetWithoutSource("logs_config.expected_tags_duration", 0) - p := newProviderWithClock("foo", clock, fakeTagger) + p := newProviderWithClock(types.NewEntityID(types.ContainerID, "foo"), clock, fakeTagger) pp := p.(*provider) var tt []string diff --git a/pkg/logs/tailers/docker/tailer.go b/pkg/logs/tailers/docker/tailer.go index be25cdc72b521..7eb2b19b6ee64 100644 --- a/pkg/logs/tailers/docker/tailer.go +++ b/pkg/logs/tailers/docker/tailer.go @@ -86,7 +86,7 @@ func NewTailer(cli *dockerutil.DockerUtil, containerID string, source *sources.L outputChan: outputChan, decoder: decoder.NewDecoderWithFraming(sources.NewReplaceableSource(source), dockerstream.New(containerID), framer.DockerStream, nil, status.NewInfoRegistry()), Source: source, - tagProvider: tag.NewProvider(types.NewEntityID(types.ContainerID, containerID).String(), tagger), + tagProvider: tag.NewProvider(types.NewEntityID(types.ContainerID, containerID), tagger), dockerutil: cli, readTimeout: readTimeout, sleepDuration: defaultSleepDuration, diff --git a/pkg/logs/tailers/file/tailer.go b/pkg/logs/tailers/file/tailer.go index 4cd584b6e509e..b9d96b982fcf8 100644 --- a/pkg/logs/tailers/file/tailer.go +++ b/pkg/logs/tailers/file/tailer.go @@ -144,7 +144,7 @@ type TailerOptions struct { func NewTailer(opts *TailerOptions) *Tailer { var tagProvider tag.Provider if opts.File.Source.Config().Identifier != "" { - tagProvider = tag.NewProvider(types.NewEntityID(types.ContainerID, opts.File.Source.Config().Identifier).String(), opts.TagAdder) + tagProvider = tag.NewProvider(types.NewEntityID(types.ContainerID, opts.File.Source.Config().Identifier), opts.TagAdder) } else { tagProvider = tag.NewLocalProvider([]string{}) } diff --git a/pkg/logs/tailers/journald/docker.go b/pkg/logs/tailers/journald/docker.go index 0c80df85cf435..bcc835d341373 100644 --- a/pkg/logs/tailers/journald/docker.go +++ b/pkg/logs/tailers/journald/docker.go @@ -34,7 +34,7 @@ func (t *Tailer) getContainerID(entry *sdjournal.JournalEntry) string { // getContainerTags returns all the tags of a given container. func (t *Tailer) getContainerTags(containerID string) []string { - tags, err := tagger.Tag(types.NewEntityID(types.ContainerID, containerID).String(), types.HighCardinality) + tags, err := tagger.Tag(types.NewEntityID(types.ContainerID, containerID), types.HighCardinality) if err != nil { log.Warn(err) } diff --git a/pkg/process/util/containers/containers.go b/pkg/process/util/containers/containers.go index fd66a93ae0af4..6ab0b27c1e32c 100644 --- a/pkg/process/util/containers/containers.go +++ b/pkg/process/util/containers/containers.go @@ -119,7 +119,7 @@ func (p *containerProvider) GetContainers(cacheValidity time.Duration, previousC continue } - entityID := types.NewEntityID(types.ContainerID, container.ID).String() + entityID := types.NewEntityID(types.ContainerID, container.ID) tags, err := tagger.Tag(entityID, types.HighCardinality) if err != nil { log.Debugf("Could not collect tags for container %q, err: %v", container.ID[:12], err) diff --git a/pkg/security/resolvers/tags/resolver.go b/pkg/security/resolvers/tags/resolver.go index e62736f1e5afa..c8674a0bfa3f0 100644 --- a/pkg/security/resolvers/tags/resolver.go +++ b/pkg/security/resolvers/tags/resolver.go @@ -24,7 +24,7 @@ import ( type Tagger interface { Start(ctx context.Context) error Stop() error - Tag(entity string, cardinality types.TagCardinality) ([]string, error) + Tag(entity types.EntityID, cardinality types.TagCardinality) ([]string, error) } type nullTagger struct{} @@ -37,7 +37,7 @@ func (n *nullTagger) Stop() error { return nil } -func (n *nullTagger) Tag(_ string, _ types.TagCardinality) ([]string, error) { +func (n *nullTagger) Tag(_ types.EntityID, _ types.TagCardinality) ([]string, error) { return nil, nil } @@ -80,14 +80,14 @@ func (t *DefaultResolver) Resolve(id string) []string { } entityID := types.NewEntityID(types.ContainerID, id) - tags, _ := t.tagger.Tag(entityID.String(), types.OrchestratorCardinality) + tags, _ := t.tagger.Tag(entityID, types.OrchestratorCardinality) return tags } // ResolveWithErr returns the tags for the given id func (t *DefaultResolver) ResolveWithErr(id string) ([]string, error) { entityID := types.NewEntityID(types.ContainerID, id) - return t.tagger.Tag(entityID.String(), types.OrchestratorCardinality) + return t.tagger.Tag(entityID, types.OrchestratorCardinality) } // GetValue return the tag value for the given id and tag name diff --git a/pkg/util/kubernetes/kubelet/kubelet_common.go b/pkg/util/kubernetes/kubelet/kubelet_common.go index e8fe1e94fafec..cb27bcdb8dfac 100644 --- a/pkg/util/kubernetes/kubelet/kubelet_common.go +++ b/pkg/util/kubernetes/kubelet/kubelet_common.go @@ -60,27 +60,27 @@ func ParseMetricFromRaw(raw []byte, metric string) (string, error) { // KubeContainerIDToTaggerEntityID builds an entity ID from a container ID coming from // the pod status (i.e. including the :// prefix). -func KubeContainerIDToTaggerEntityID(ctrID string) (string, error) { +func KubeContainerIDToTaggerEntityID(ctrID string) (types.EntityID, error) { sep := strings.LastIndex(ctrID, containers.EntitySeparator) if sep != -1 && len(ctrID) > sep+len(containers.EntitySeparator) { - return types.NewEntityID(types.ContainerID, ctrID[sep+len(containers.EntitySeparator):]).String(), nil + return types.NewEntityID(types.ContainerID, ctrID[sep+len(containers.EntitySeparator):]), nil } - return "", fmt.Errorf("can't extract an entity ID from container ID %s", ctrID) + return types.EntityID{}, fmt.Errorf("can't extract an entity ID from container ID %s", ctrID) } // KubePodUIDToTaggerEntityID builds an entity ID from a pod UID coming from // the pod status (i.e. including the :// prefix). -func KubePodUIDToTaggerEntityID(podUID string) (string, error) { +func KubePodUIDToTaggerEntityID(podUID string) (types.EntityID, error) { sep := strings.LastIndex(podUID, containers.EntitySeparator) if sep != -1 && len(podUID) > sep+len(containers.EntitySeparator) { - return types.NewEntityID(types.KubernetesPodUID, podUID[sep+len(containers.EntitySeparator):]).String(), nil + return types.NewEntityID(types.KubernetesPodUID, podUID[sep+len(containers.EntitySeparator):]), nil } - return "", fmt.Errorf("can't extract an entity ID from pod UID %s", podUID) + return types.EntityID{}, fmt.Errorf("can't extract an entity ID from pod UID %s", podUID) } // KubeIDToTaggerEntityID builds a tagger entity ID from an entity ID belonging to // a container or pod. -func KubeIDToTaggerEntityID(entityName string) (string, error) { +func KubeIDToTaggerEntityID(entityName string) (types.EntityID, error) { prefix, _ := containers.SplitEntityName(entityName) if prefix == KubePodEntityName { diff --git a/pkg/util/kubernetes/kubelet/kubelet_common_test.go b/pkg/util/kubernetes/kubelet/kubelet_common_test.go index 1119161dc6c19..1d105dd978703 100644 --- a/pkg/util/kubernetes/kubelet/kubelet_common_test.go +++ b/pkg/util/kubernetes/kubelet/kubelet_common_test.go @@ -72,7 +72,7 @@ func TestKubeContainerIDToTaggerEntityID(t *testing.T) { } { t.Run(fmt.Sprintf("case: %s", in), func(t *testing.T) { res, _ := KubeContainerIDToTaggerEntityID(in) - assert.Equal(t, out, res) + assert.Equal(t, out, res.String()) }) } } @@ -89,7 +89,7 @@ func TestKubePodUIDToTaggerEntityID(t *testing.T) { } { t.Run(fmt.Sprintf("case: %s", in), func(t *testing.T) { res, _ := KubePodUIDToTaggerEntityID(in) - assert.Equal(t, out, res) + assert.Equal(t, out, res.String()) }) } } @@ -106,7 +106,7 @@ func TestKubeIDToTaggerEntityID(t *testing.T) { } { t.Run(fmt.Sprintf("case: %s", in), func(t *testing.T) { res, _ := KubeIDToTaggerEntityID(in) - assert.Equal(t, out, res) + assert.Equal(t, out, res.String()) }) } } From 52302c6fac5b26dcf8f86f6f1c20e5c529152501 Mon Sep 17 00:00:00 2001 From: adel121 Date: Wed, 16 Oct 2024 16:42:40 +0200 Subject: [PATCH 4/7] address PR comments - mainly remove all calls to NewEntityIDFromString --- comp/core/tagger/common/entity_id_builder.go | 15 +++-- comp/core/tagger/global.go | 18 +++--- .../generic_store/composite_store.go | 10 ---- .../taggerimpl/generic_store/store_test.go | 60 +++++++------------ comp/core/tagger/types/entity_id.go | 16 ----- comp/core/tagger/types/entity_id_test.go | 22 ------- comp/core/tagger/types/types.go | 7 --- comp/dogstatsd/replay/impl/writer.go | 7 ++- .../provider/cadvisor/provider_test.go | 4 +- .../kubelet/provider/kubelet/provider_test.go | 4 +- .../kubelet/provider/pod/provider_test.go | 4 +- .../kubelet/provider/probe/provider_test.go | 4 +- .../kubelet/provider/summary/provider_test.go | 4 +- .../corechecks/ebpf/oomkill/oom_kill.go | 2 +- .../ebpf/tcpqueuelength/tcp_queue_length.go | 2 +- 15 files changed, 65 insertions(+), 114 deletions(-) delete mode 100644 comp/core/tagger/types/entity_id_test.go diff --git a/comp/core/tagger/common/entity_id_builder.go b/comp/core/tagger/common/entity_id_builder.go index 798faa8df3fc0..f296090b588cd 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" @@ -37,14 +40,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 f4bcc2966e10c..626cc604434aa 100644 --- a/comp/core/tagger/global.go +++ b/comp/core/tagger/global.go @@ -6,9 +6,10 @@ package tagger import ( - "fmt" + "errors" "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" @@ -39,7 +40,7 @@ func UnlockGlobalTaggerClient() { // GetEntity returns the hash for the provided entity id. func GetEntity(entityID types.EntityID) (*types.Entity, error) { if globalTagger == nil { - return nil, fmt.Errorf("a global tagger must be set before calling GetEntity") + return nil, errors.New("a global tagger must be set before calling GetEntity") } return globalTagger.GetEntity(entityID) } @@ -50,21 +51,22 @@ func GetEntity(entityID types.EntityID) (*types.Entity, error) { // integrations using the tagger func LegacyTag(entity string, 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") } - 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) } @@ -80,7 +82,7 @@ func GetEntityHash(entityID types.EntityID, cardinality types.TagCardinality) st // AgentTags is an interface function that queries taggerclient singleton func AgentTags(cardinality types.TagCardinality) ([]string, error) { if globalTagger == nil { - return nil, fmt.Errorf("a global tagger must be set before calling AgentTags") + return nil, errors.New("a global tagger must be set before calling AgentTags") } return globalTagger.AgentTags(cardinality) } @@ -88,7 +90,7 @@ func AgentTags(cardinality types.TagCardinality) ([]string, error) { // GlobalTags is an interface function that queries taggerclient singleton func GlobalTags(cardinality types.TagCardinality) ([]string, error) { if globalTagger == nil { - return nil, fmt.Errorf("a global tagger must be set before calling GlobalTags") + return nil, errors.New("a global tagger must be set before calling GlobalTags") } return globalTagger.GlobalTags(cardinality) } diff --git a/comp/core/tagger/taggerimpl/generic_store/composite_store.go b/comp/core/tagger/taggerimpl/generic_store/composite_store.go index 1428079ea286c..1a1b7306265c7 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 c2f9073021a71..c1f4678ad6a0a 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,15 +72,22 @@ 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 list = store.ListObjects(filter) - expectedListing := []any{"prefix1://id1", "prefix2://id2"} + expectedListing := []types.EntityID{ + types.NewEntityID(types.EntityIDPrefix("prefix1"), "id1"), + types.NewEntityID(types.EntityIDPrefix("prefix2"), "id2"), + } assert.ElementsMatch(t, expectedListing, list) } @@ -115,9 +95,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{}{}) } @@ -128,8 +113,7 @@ func TestObjectStore_ForEach(t *testing.T) { fb.Include(types.EntityIDPrefix("prefix1"), types.EntityIDPrefix("prefix2")) filter := fb.Build(types.HighCardinality) + // only elements matching the filter should be included in the accumulator store.ForEach(filter, func(id types.EntityID, _ any) { accumulator = append(accumulator, id.String()) }) - - // list should return empty assert.ElementsMatch(t, accumulator, []string{"prefix1://id1", "prefix2://id2"}) } diff --git a/comp/core/tagger/types/entity_id.go b/comp/core/tagger/types/entity_id.go index 4db94a61c55c6..5604d060c9621 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 a235e4273ad95..0000000000000 --- 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 a66749bae5c28..e45f9e6daa084 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 b46eaf456bbf5..77f7ea7575351 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 136aa7a67203b..dbc8a8fa43eeb 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 2830612208a5b..096b63b898140 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 e484a64190132..50ac558c64c7f 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 56043b80add61..2b21551c07201 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 766d4c8c0acdb..0661462956d71 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 fa511a4e66db5..33b9393ac9af2 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 8a8b8c9cd5bb7..4a0fecf278bc6 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) From ab9d0163bfec9f84c6e8f80c4f24fa457e3f3b0f Mon Sep 17 00:00:00 2001 From: adel121 Date: Mon, 21 Oct 2024 16:53:51 +0200 Subject: [PATCH 5/7] pr review --- comp/otelcol/otlp/collector.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/comp/otelcol/otlp/collector.go b/comp/otelcol/otlp/collector.go index 6207e329a100a..3fc7b89a11c65 100644 --- a/comp/otelcol/otlp/collector.go +++ b/comp/otelcol/otlp/collector.go @@ -32,6 +32,7 @@ import ( "github.com/DataDog/datadog-agent/comp/core/config" "github.com/DataDog/datadog-agent/comp/core/tagger" + "github.com/DataDog/datadog-agent/comp/core/tagger/common" "github.com/DataDog/datadog-agent/comp/core/tagger/types" "github.com/DataDog/datadog-agent/comp/core/workloadmeta/collectors/util" "github.com/DataDog/datadog-agent/comp/otelcol/otlp/components/exporter/logsagentexporter" @@ -69,12 +70,17 @@ func (t *tagEnricher) Enrich(_ context.Context, extraTags []string, dimensions * enrichedTags := make([]string, 0, len(extraTags)+len(dimensions.Tags())) enrichedTags = append(enrichedTags, extraTags...) enrichedTags = append(enrichedTags, dimensions.Tags()...) - - entityTags, err := tagger.LegacyTag(dimensions.OriginID(), t.cardinality) + prefix, id, err := common.ExtractPrefixAndID(dimensions.OriginID()) if err != nil { - log.Tracef("Cannot get tags for entity %s: %s", dimensions.OriginID(), err) + entityID := types.NewEntityID(prefix, id) + entityTags, err := tagger.Tag(entityID, t.cardinality) + if err != nil { + log.Tracef("Cannot get tags for entity %s: %s", dimensions.OriginID(), err) + } else { + enrichedTags = append(enrichedTags, entityTags...) + } } else { - enrichedTags = append(enrichedTags, entityTags...) + log.Tracef("Cannot get tags for entity %s: %s", dimensions.OriginID(), err) } globalTags, err := tagger.GlobalTags(t.cardinality) From 50cc5a2f43362ad726f8896b4e482190896309d0 Mon Sep 17 00:00:00 2001 From: adel121 Date: Mon, 21 Oct 2024 18:15:03 +0200 Subject: [PATCH 6/7] run inv -e go-deps.generate --- cmd/serverless/dependencies_linux_amd64.txt | 1 + cmd/serverless/dependencies_linux_arm64.txt | 1 + 2 files changed, 2 insertions(+) diff --git a/cmd/serverless/dependencies_linux_amd64.txt b/cmd/serverless/dependencies_linux_amd64.txt index 9937caed286ee..c41e8063d6c92 100644 --- a/cmd/serverless/dependencies_linux_amd64.txt +++ b/cmd/serverless/dependencies_linux_amd64.txt @@ -76,6 +76,7 @@ github.com/DataDog/datadog-agent/comp/core/log/impl github.com/DataDog/datadog-agent/comp/core/secrets github.com/DataDog/datadog-agent/comp/core/status github.com/DataDog/datadog-agent/comp/core/tagger +github.com/DataDog/datadog-agent/comp/core/tagger/common github.com/DataDog/datadog-agent/comp/core/tagger/noopimpl github.com/DataDog/datadog-agent/comp/core/tagger/telemetry github.com/DataDog/datadog-agent/comp/core/tagger/types diff --git a/cmd/serverless/dependencies_linux_arm64.txt b/cmd/serverless/dependencies_linux_arm64.txt index 7433a1de5a3a2..4463ec06b1b7d 100644 --- a/cmd/serverless/dependencies_linux_arm64.txt +++ b/cmd/serverless/dependencies_linux_arm64.txt @@ -76,6 +76,7 @@ github.com/DataDog/datadog-agent/comp/core/log/impl github.com/DataDog/datadog-agent/comp/core/secrets github.com/DataDog/datadog-agent/comp/core/status github.com/DataDog/datadog-agent/comp/core/tagger +github.com/DataDog/datadog-agent/comp/core/tagger/common github.com/DataDog/datadog-agent/comp/core/tagger/noopimpl github.com/DataDog/datadog-agent/comp/core/tagger/telemetry github.com/DataDog/datadog-agent/comp/core/tagger/types From 5b8925c58005b0abcb2a1497790d3fe3ec679a15 Mon Sep 17 00:00:00 2001 From: adel121 Date: Wed, 23 Oct 2024 16:50:08 +0200 Subject: [PATCH 7/7] fix lint --- comp/core/autodiscovery/listeners/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/comp/core/autodiscovery/listeners/service.go b/comp/core/autodiscovery/listeners/service.go index d2d47eb8d1add..372b079766324 100644 --- a/comp/core/autodiscovery/listeners/service.go +++ b/comp/core/autodiscovery/listeners/service.go @@ -97,7 +97,7 @@ func (s *service) GetTags() ([]string, error) { func (s *service) GetTagsWithCardinality(cardinality string) ([]string, error) { checkCard, err := types.StringToTagCardinality(cardinality) if err == nil { - return tagger.Tag(taggercommon.BuildTaggerEntityID(s.entity.GetID()).String(), checkCard) + return tagger.Tag(taggercommon.BuildTaggerEntityID(s.entity.GetID()), checkCard) } log.Warnf("error converting cardinality %s to TagCardinality: %v", cardinality, err) return s.GetTags()