Skip to content

Revert "[CONTP-698] fix bug in EntityID.Empty method (#35793)" #36413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 23, 2025
Merged

Revert "[CONTP-698] fix bug in EntityID.Empty method (#35793)" #36413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 15 additions & 17 deletions comp/core/tagger/generic_store/store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
func TestObjectStore_GetSet(t *testing.T) {
store := NewObjectStore[any]()

id := types.NewEntityID(types.ContainerID, "id")
id := types.NewEntityID("prefix", "id")
// getting a non existent item
obj, found := store.Get(id)
assert.Nil(t, obj)
Expand Down Expand Up @@ -46,7 +46,7 @@ func TestObjectStore_Size(t *testing.T) {
assert.Equalf(t, store.Size(), 0, "store should be empty")

// add item to store
id := types.NewEntityID(types.ContainerID, "id")
id := types.NewEntityID("prefix", "id")
store.Set(id, struct{}{})

// store size should be 1
Expand All @@ -64,7 +64,7 @@ func TestObjectStore_ListObjects(t *testing.T) {

// build some filter
fb := types.NewFilterBuilder()
fb.Include(types.EntityIDPrefix(types.ContainerID), types.EntityIDPrefix(types.KubernetesDeployment))
fb.Include(types.EntityIDPrefix("prefix1"), types.EntityIDPrefix("prefix2"))
filter := fb.Build(types.HighCardinality)

// list should return empty
Expand All @@ -73,10 +73,10 @@ func TestObjectStore_ListObjects(t *testing.T) {

// add some items
ids := []types.EntityID{
types.NewEntityID(types.EntityIDPrefix(types.ContainerID), "id1"),
types.NewEntityID(types.EntityIDPrefix(types.KubernetesDeployment), "id2"),
types.NewEntityID(types.EntityIDPrefix(types.KubernetesPodUID), "id3"),
types.NewEntityID(types.EntityIDPrefix(types.ECSTask), "id4"),
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 {
Expand All @@ -85,8 +85,8 @@ func TestObjectStore_ListObjects(t *testing.T) {

list = store.ListObjects(filter)
expectedListing := []types.EntityID{
types.NewEntityID(types.EntityIDPrefix(types.ContainerID), "id1"),
types.NewEntityID(types.EntityIDPrefix(types.KubernetesDeployment), "id2"),
types.NewEntityID(types.EntityIDPrefix("prefix1"), "id1"),
types.NewEntityID(types.EntityIDPrefix("prefix2"), "id2"),
}
assert.ElementsMatch(t, expectedListing, list)
}
Expand All @@ -95,13 +95,11 @@ func TestObjectStore_ForEach(t *testing.T) {
store := NewObjectStore[any]()

// add some items
entityID1 := types.NewEntityID(types.EntityIDPrefix(types.ContainerID), "id1")
entityID2 := types.NewEntityID(types.EntityIDPrefix(types.KubernetesDeployment), "id2")
ids := []types.EntityID{
entityID1,
entityID2,
types.NewEntityID(types.EntityIDPrefix(types.KubernetesPodUID), "id3"),
types.NewEntityID(types.EntityIDPrefix(types.ECSTask), "id4"),
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 {
Expand All @@ -112,10 +110,10 @@ func TestObjectStore_ForEach(t *testing.T) {

// build some filter
fb := types.NewFilterBuilder()
fb.Include(types.EntityIDPrefix(types.ContainerID), types.EntityIDPrefix(types.KubernetesDeployment))
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()) })
assert.ElementsMatch(t, accumulator, []string{entityID1.String(), entityID2.String()})
assert.ElementsMatch(t, accumulator, []string{"prefix1://id1", "prefix2://id2"})
}
4 changes: 2 additions & 2 deletions comp/core/tagger/impl-remote/tagstore_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import (
)

var (
entityID types.EntityID = types.NewEntityID(types.ContainerID, "bar")
anotherEntityID types.EntityID = types.NewEntityID(types.ContainerID, "quux")
entityID types.EntityID = types.NewEntityID("foo", "bar")
anotherEntityID types.EntityID = types.NewEntityID("foo", "quux")
)

func TestProcessEvent_AddAndModify(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion comp/core/tagger/impl/tagger_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func setupFakeMetricsProvider(mockMetricsProvider metrics.Provider) func() {
}

func TestAccumulateTagsFor(t *testing.T) {
entityID := types.NewEntityID(types.ContainerID, "entity_name")
entityID := types.NewEntityID("", "entity_name")

mockReq := MockRequires{
Config: configmock.New(t),
Expand Down
17 changes: 8 additions & 9 deletions comp/core/tagger/subscriber/subscription_manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (

func TestSubscriptionManager(t *testing.T) {

entityID := types.NewEntityID(types.ContainerID, "bar")
entityID := types.NewEntityID("foo", "bar")

events := map[string]types.EntityEvent{
"added": {
Expand Down Expand Up @@ -55,8 +55,7 @@ func TestSubscriptionManager(t *testing.T) {
"added-with-unmatched-prefix": {
EventType: types.EventTypeAdded,
Entity: types.Entity{
// KubernetesDeployment prefix doesn't match any of the subscription filters used below
ID: types.NewEntityID(types.KubernetesDeployment, "goo"),
ID: types.NewEntityID("gee", "goo"),
},
},
}
Expand All @@ -66,7 +65,7 @@ func TestSubscriptionManager(t *testing.T) {

// Low Cardinality Subscriber
lowCardSubID := "low-card-sub"
lowCardSubscription, err := sm.Subscribe(lowCardSubID, types.NewFilterBuilder().Include(types.ContainerID).Build(types.LowCardinality), nil)
lowCardSubscription, err := sm.Subscribe(lowCardSubID, types.NewFilterBuilder().Include(types.EntityIDPrefix("foo")).Build(types.LowCardinality), nil)
require.NoError(t, err)

sm.Notify([]types.EntityEvent{
Expand All @@ -81,7 +80,7 @@ func TestSubscriptionManager(t *testing.T) {

// Orchestrator Cardinality Subscriber
orchCardSubID := "orch-card-sub"
orchCardSubscription, err := sm.Subscribe(orchCardSubID, types.NewFilterBuilder().Include(types.ContainerID).Build(types.OrchestratorCardinality), nil)
orchCardSubscription, err := sm.Subscribe(orchCardSubID, types.NewFilterBuilder().Include(types.EntityIDPrefix("foo")).Build(types.OrchestratorCardinality), nil)
require.NoError(t, err)

sm.Notify([]types.EntityEvent{
Expand All @@ -96,7 +95,7 @@ func TestSubscriptionManager(t *testing.T) {

// High Cardinality Subscriber
highCardSubID := "high-card-sub"
highCardSubscription, err := sm.Subscribe(highCardSubID, types.NewFilterBuilder().Include(types.ContainerID).Build(types.HighCardinality), []types.EntityEvent{
highCardSubscription, err := sm.Subscribe(highCardSubID, types.NewFilterBuilder().Include(types.EntityIDPrefix("foo")).Build(types.HighCardinality), []types.EntityEvent{
events["added"],
})
require.NoError(t, err)
Expand All @@ -112,7 +111,7 @@ func TestSubscriptionManager(t *testing.T) {

// None Cardinality Subscriber
noneCardSubID := "none-card-sub"
noneCardSubscription, err := sm.Subscribe(noneCardSubID, types.NewFilterBuilder().Include(types.ContainerID).Build(types.NoneCardinality), nil)
noneCardSubscription, err := sm.Subscribe(noneCardSubID, types.NewFilterBuilder().Include(types.EntityIDPrefix("foo")).Build(types.NoneCardinality), nil)
require.NoError(t, err)

sm.Notify([]types.EntityEvent{
Expand Down Expand Up @@ -249,9 +248,9 @@ func TestSubscriptionManagerDuplicateSubscriberID(t *testing.T) {

// Low Cardinality Subscriber
lowCardSubID := "low-card-sub"
_, err := sm.Subscribe(lowCardSubID, types.NewFilterBuilder().Include(types.ContainerID).Build(types.LowCardinality), nil)
_, err := sm.Subscribe(lowCardSubID, types.NewFilterBuilder().Include(types.EntityIDPrefix("foo")).Build(types.LowCardinality), nil)
require.NoError(t, err)

_, err = sm.Subscribe(lowCardSubID, types.NewFilterBuilder().Include(types.ContainerID).Build(types.LowCardinality), nil)
_, err = sm.Subscribe(lowCardSubID, types.NewFilterBuilder().Include(types.EntityIDPrefix("foo")).Build(types.LowCardinality), nil)
require.Error(t, err)
}
2 changes: 1 addition & 1 deletion comp/core/tagger/tagstore/entity_tags_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const (
invalidSource = "invalidSource"
)

var testEntityID = types.NewEntityID(types.ContainerID, "EntityID")
var testEntityID = types.NewEntityID("test", "EntityID")

func TestToEntity(t *testing.T) {
entityTags := newEntityTagsWithSingleSource(testEntityID, testSource)
Expand Down
2 changes: 1 addition & 1 deletion comp/core/tagger/tagstore/tagstore_bench_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func generateRandomTagInfo() *types.TagInfo {
id := ids[rand.Intn(nEntities)]
source := sources[rand.Intn(nSources)]
return &types.TagInfo{
EntityID: types.NewEntityID(types.ContainerID, id),
EntityID: types.NewEntityID("", id),
Source: source,
LowCardTags: generateRandomTags(),
OrchestratorCardTags: generateRandomTags(),
Expand Down
2 changes: 1 addition & 1 deletion comp/core/tagger/tagstore/tagstore_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func (s *StoreTestSuite) TestLookupStandard() {
assert.Contains(s.T(), standard, "env:dev")
assert.Contains(s.T(), standard, "service:foo")

_, err = s.tagstore.LookupStandard(types.NewEntityID(types.ContainerID, "not found"))
_, err = s.tagstore.LookupStandard(types.NewEntityID("not", "found"))
assert.NotNil(s.T(), err)
}

Expand Down
12 changes: 2 additions & 10 deletions comp/core/tagger/types/entity_id.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ type EntityID struct {
id string
}

// Empty returns true if either the prefix or id empty strings
// Empty returns true if prefix and id are both empty strings
func (eid EntityID) Empty() bool {
return eid.prefix == "" || eid.id == ""
return eid.prefix == "" && eid.id == ""
}

// GetPrefix returns the entityID prefix
Expand All @@ -43,20 +43,12 @@ func (eid EntityID) GetID() string {
}

// String returns the entityID in the format `{prefix}://{id}`
// Returns an empty string if `id` is empty
func (eid EntityID) String() string {
return eid.prefix.ToUID(eid.id)
}

var supportedPrefixes = AllPrefixesSet()

// NewEntityID builds and returns an EntityID object
// A panic will occur if an unsupported prefix is used
func NewEntityID(prefix EntityIDPrefix, id string) EntityID {
if _, found := supportedPrefixes[prefix]; !found {
// prefix is expected to be set based on the prefix enum defined below
panic(fmt.Sprintf("unsupported tagger entity prefix: %q", prefix))
}
return EntityID{prefix, id}
}

Expand Down
30 changes: 0 additions & 30 deletions comp/core/tagger/types/entity_id_test.go
View file
Open in desktop

This file was deleted.