[processor/k8sattributesprocessor] Include Container Ports and Container CPU Request in Metadata#38019
[processor/k8sattributesprocessor] Include Container Ports and Container CPU Request in Metadata#38019spurplewang wants to merge 12 commits into
Conversation
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Please resolve conflicts ; please make the addition of this new data opt-in via a boolean flag in metric configuration or a feature gate. Please address the original issue comment from @TylerHelmuth regarding semantic conventions associated with these attributes. |
@spurplewang can you fix the CI check errors? |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
|
Can I have this reopened? Otherwise I can open a new one if that's preferred. |
container cpu request.
container cpu request.
|
The new data are opt-in and default off with controls in metrics config. |
|
Please take a look at the conflicts. |
Conflicts resolved. |
|
Great enhancement @spurplewang Can we expose container memory requests? I am happy to help on contributing on it, I can create a separate issue/pr Thanks |
ChrsMark
left a comment
There was a problem hiding this comment.
I would like to raise that we need better coordination here before continuing with such additions. I'm requesting changes for now to ensure we don't prematurely merge this without having the proper discussions and alignment first.
We are adding container cpu/memory requests and limits as metrics at open-telemetry/semantic-conventions#2178. k8sclusterreceiver already reports those as metrics.
In general we are on a big effort to define all k8s related metrics the Collector uses today as Semantic Conventions: open-telemetry/semantic-conventions#1485
I see the value of having those as resource/entity attributes as well but we would need to decide on this first. Since this is a discussion needs to take place in Semantic Conventions project I would like to ask for this to happen first before continuing with the implementation.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Thank you for flagging this—I agree that getting alignment here is the right next step. We have some immediate needs for this metadata, so I'm keen to find a path forward. Please let me know what works best to proceed.
I did notice the pull request and used the name
Thanks for the pointer. To ensure I contribute effectively to that discussion, could you advise on the best protocol? Is it better to comment on the main issue (#1485), or should I open a new, more focused issue in that repository that links back to it? |
Thanks @antonjim-te ! Please feel free to create a separate issue/pr for memoey request, it would be a great addition. We might also need to wait for the alignment on semantic convention side as @ChrsMark mentioned. |
|
@spurplewang I think raising an issue/PR against https://github.com/open-telemetry/semantic-conventions to suggest the attributes would be the next step. |
Proposed open-telemetry/semantic-conventions#2357 and open-telemetry/semantic-conventions#2358. @ChrsMark , please feel free to take a look! |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Description
Include container ports and CPU request in metadata for resource attribute enrichment.
Link to tracking issue
Closes #37862
Testing
All test files were updated, followed by successful end-to-end testing.
Documentation
Updated README.md and documentation.md