[receiver/kubeletstatsreciever] add volume attrs for CSI-backed PVCs #32055
Conversation
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Bump, not stale, just awaiting review. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Bump |
|
@gdvalle can you fix the merge conflicts and make CI pass? Thanks |
|
@mx-psi CI is green now! |
|
@dmitryax @TylerHelmuth PTAL! |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
There was a problem hiding this comment.
We've started being stricter with how we add new attributes in k8s components, specifically looking for approval from the semantic convention SIG on the proposed semantic conventions.
Please open a PR at https://github.com/open-telemetry/semantic-conventions, similar to open-telemetry/semantic-conventions#997, to propose these new attributes.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
bump |
| | csi.driver | CSI driver | Any Str | true | | ||
| | csi.volume.handle | CSI volume handle | Any Str | true | |
There was a problem hiding this comment.
csi is kubernetes specific concern only, right? In that case, it's better to prepend it with k8s. namespace.
There was a problem hiding this comment.
Looks like it's not the case...
There was a problem hiding this comment.
I'm still not 100% sure about the naming here. csi namespace may be too ambiguous. I don't have any suggestions at this point. I'm open to merging the PR as is. Can you please submit a PR for open-telemetry/semantic-conventions#1119? Maybe we will get some more feedback there
There was a problem hiding this comment.
Done open-telemetry/semantic-conventions#1337
The alteration of k8s.volume.type done here (adding a special csiPersistentVolume value) maybe looks less appealing if we want to keep the 1:1 mapping here. It's still technically a persistentVolumeClaim, so perhaps we should leave it at that, and users can query for defined(csi.driver) or something to enumerate CSI-backed PVCs.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #32055 +/- ##
==========================================
+ Coverage 82.62% 83.44% +0.82%
==========================================
Files 1808 2158 +350
Lines 140874 168416 +27542
==========================================
+ Hits 116392 140542 +24150
- Misses 20310 22956 +2646
- Partials 4172 4918 +746 ☔ View full report in Codecov by Sentry. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
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. |
|
@TylerHelmuth or @dmitryax could you please re-open this? The semconv has been established/merged and branch is updated to match. |
A retry of #25170, where it was auto-closed from no maintainer activity.
Description:
Add a couple new attrs to record information about CSI volume type.
These are now part of semconv as of open-telemetry/semantic-conventions#1337
container.csi.plugin.name: The raw CSI.Driver field.container.csi.volume.id: The raw CSI.VolumeHandle field.Also, if we can identify the CSI driver as a GCP-backed one, use the
VolumeHandleto populategce.pd.name. This gives parity with the legacyGCEPersistentDiskin-tree PV kind.Link to tracking Issue: n/a
Testing:
A test is added to validate all labels described above are populated.
Documentation:
A changelog entry is added.