-
Notifications
You must be signed in to change notification settings - Fork 3.1k
[processor/k8sattributes] Add feature to extract deployment name from replicaset name #42534
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
[processor/k8sattributes] Add feature to extract deployment name from replicaset name #42534
Conversation
|
Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib. Important reminders:
A maintainer will review your pull request soon. Thank you for helping make OpenTelemetry better! |
0d3927d to
d360653
Compare
|
Hi @atoulme the PR needs a workflows approval to run the test. Thanks! |
|
Another gentle ping: @ChrsMark @dmitryax @TylerHelmuth @fatsheep9146 |
|
I think we definitely should add this (see #42530 (comment)). Since this is a heuristic, it is possible (although unlikely in my experience) that it could break someone. If that happens when the feature gate goes beta, I think we can add a config option to revert back to the previous behavior. But i've seen this heuristic used widely without any issues. |
6137c53 to
6fad22a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I support this feature! Left a comment about making this configurable.
079892a to
904412f
Compare
904412f to
ee555d1
Compare
|
Hi @ChrsMark do I need repo owner to merge the change? Thanks! |
Typically few code-owners of the component need to approve and then a maintainer can merge :). |
48822dd to
be317da
Compare
|
Please address conflicts and mark ready for review again. |
1f346f2 to
478984f
Compare
|
atoulme Done. |
478984f to
05792de
Compare
05792de to
98f8fcb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine adding this if other code-owners agree.
@TylerHelmuth @dmitryax @fatsheep9146 PTAL
|
Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib. Important reminders:
A maintainer will review your pull request soon. Thank you for helping make OpenTelemetry better! |
… replicaset name Fixes open-telemetry#42530
98f8fcb to
b9ba568
Compare
|
@TylerHelmuth A gentle ping for review. |
Co-authored-by: Jina Jain <jjain@splunk.com>
|
Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib. Important reminders:
A maintainer will review your pull request soon. Thank you for helping make OpenTelemetry better! |
|
Thank you for your contribution @JeffLuoo! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey. If you are getting started contributing, you can also join the CNCF Slack channel #opentelemetry-new-contributors to ask for guidance and get help. |
Fixes #42530
Description
Extract deployment name from replicaset name in the pod owner reference. Doesn't need to start the informer for the replicaset, reduce traffic to api server.
Link to tracking issue
Fixes #42530
Testing
Unit test, and test in kind cluster that trace still has container name attribute.
Documentation