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

[workloadmeta] componentize logic #19707

Merged
merged 95 commits into from Nov 20, 2023
Merged

[workloadmeta] componentize logic #19707

merged 95 commits into from Nov 20, 2023

Conversation

truthbk
Copy link
Member

@truthbk truthbk commented Sep 25, 2023

What does this PR do?

Moves pkg/workloadmeta to comp/core/workloadmeta.

Motivation

Componentize workloadmeta as a pre-requisite for the tagger and auto-discovery components.

Additional Notes

Still some global state that should be eventually addressed.

Possible Drawbacks / Trade-offs

Describe how to test/QA your changes

Reviewer's Checklist

  • If known, an appropriate milestone has been selected; otherwise the Triage milestone is set.
  • Use the major_change label if your change either has a major impact on the code base, is impacting multiple teams or is changing important well-established internals of the Agent. This label will be use during QA to make sure each team pay extra attention to the changed behavior. For any customer facing change use a releasenote.
  • A release note has been added or the changelog/no-changelog label has been applied.
  • Changed code has automated tests for its functionality.
  • Adequate QA/testing plan information is provided if the qa/skip-qa label is not applied.
  • At least one team/.. label has been applied, indicating the team(s) that should QA this change.
  • If applicable, docs team has been notified or an issue has been opened on the documentation repo.
  • If applicable, the need-change/operator and need-change/helm labels have been applied.
  • If applicable, the k8s/<min-version> label, indicating the lowest Kubernetes version compatible with this feature.
  • If applicable, the config template has been updated.

@truthbk truthbk force-pushed the jaime/workloadmeta branch 2 times, most recently from 56f225f to f8bbcef Compare September 28, 2023 12:30
@kacper-murzyn kacper-murzyn modified the milestones: 7.49.0, 7.50.0 Sep 30, 2023
@truthbk truthbk force-pushed the jaime/workloadmeta branch 4 times, most recently from 9f22c9a to cfad53f Compare October 20, 2023 15:20
@truthbk truthbk force-pushed the jaime/workloadmeta branch 11 times, most recently from 4d838a3 to 50c450c Compare October 25, 2023 10:04
@truthbk truthbk marked this pull request as ready for review October 25, 2023 12:57
@truthbk truthbk requested review from a team as code owners October 25, 2023 12:58
Copy link
Contributor

@ogaca-dd ogaca-dd left a comment

Choose a reason for hiding this comment

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

LGTM

} else {
catalog = workloadmeta.NodeAgent
}
catalog := workloadmeta.NodeAgent
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it expected catalog is never set to workloadmeta.ClusterAgent?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, for dogstatsd I think this is expected.

components := &DogstatsdComponents{
DogstatsdServer: server,
WorkloadMeta: wmeta,
WorkloadMeta: w,
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally WorkloadMeta type could be optional.Option[workloadmeta.Component].

Copy link
Member Author

Choose a reason for hiding this comment

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

To avoid running the CI again and hold back the train after the freeze, I'll do this in a follow-up. But that makes sense, I can make that change.

@kacper-murzyn kacper-murzyn merged commit 191e90c into main Nov 20, 2023
147 checks passed
@kacper-murzyn kacper-murzyn deleted the jaime/workloadmeta branch November 20, 2023 11:18
vickenty added a commit that referenced this pull request Dec 6, 2023
vickenty added a commit that referenced this pull request Dec 6, 2023
vickenty added a commit that referenced this pull request Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants