Skip to content
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.

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

[receiver/discovery] Update entity events ID fields #4739

Merged
merged 1 commit into from
Apr 29, 2024

Conversation

dmitryax
Copy link
Contributor

@dmitryax dmitryax commented Apr 26, 2024

Update entity events ID fields according to the latest contract with the inventory service:

  • discovery.endpoint.id is moved from identifying to regular attributes
  • The following attributes now marked as identifying instead:
    • k8s.pod.uid
    • k8s.node.uid
    • container.id
    • source.port
    • host.id

@dmitryax dmitryax requested review from a team as code owners April 26, 2024 22:02
@dmitryax dmitryax force-pushed the update-entity-events-model branch from f6b12bd to 36f5e80 Compare April 26, 2024 22:48
}
return entityEvents, failed, err
}

func entityDeleteEvents(endpoints []observer.Endpoint, ts time.Time) experimentalmetricmetadata.EntityEventsSlice {
func entityDeleteEvents(endpoints []observer.Endpoint, ts time.Time) (ees experimentalmetricmetadata.EntityEventsSlice, failed int, err error) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we need the failed (int) value? Is it a return code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just for logging. We do the same for emitEntityStateEvents, so I kept it the same, but I agree it's not ideal. We can probably move the logging inside the funcs. I can address it separately

Copy link

@samiura samiura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM! Just a question left there to understand.

@dmitryax dmitryax force-pushed the update-entity-events-model branch from 36f5e80 to 80ff86c Compare April 28, 2024 19:49
@dmitryax dmitryax requested a review from a team as a code owner April 28, 2024 19:49
@dmitryax
Copy link
Contributor Author

Test failures are unrelated and fixed in #4740

Update entity events ID fields according to the latest contract with the inventory service:
- `discovery.endpoint.id` is moved from identifying to regular attributes
- The following attributes now marked as identifying instead:
  - `k8s.pod.uid`
  - `k8s.node.uid`
  - `container.id`
  - `source.port`
  - `host.id`
@dmitryax dmitryax force-pushed the update-entity-events-model branch from 80ff86c to 4f0f7fa Compare April 29, 2024 15:34
@dmitryax dmitryax merged commit 8af23ba into main Apr 29, 2024
27 checks passed
@dmitryax dmitryax deleted the update-entity-events-model branch April 29, 2024 16:07
@github-actions github-actions bot locked and limited conversation to collaborators Apr 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants