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

container: add CSI attributes #1337

Merged
merged 21 commits into from
Sep 20, 2024
Merged

container: add CSI attributes #1337

merged 21 commits into from
Sep 20, 2024

Conversation

gdvalle
Copy link
Contributor

@gdvalle gdvalle commented Aug 16, 2024

Fixes #1119

Changes

Add CSI (Container Storage Interface) attributes: container.csi.plugin.name and container.csi.volume.id.

Merge requirement checklist


xref open-telemetry/opentelemetry-collector-contrib#32055 (comment)

model/registry/k8s.yaml Outdated Show resolved Hide resolved
model/registry/k8s.yaml Outdated Show resolved Hide resolved
gdvalle and others added 2 commits August 21, 2024 14:57
Co-authored-by: Joshua MacDonald <[email protected]>
Co-authored-by: Joshua MacDonald <[email protected]>
@gdvalle gdvalle requested a review from jmacd August 21, 2024 19:58
docs/attributes-registry/k8s.md Outdated Show resolved Hide resolved
@gdvalle gdvalle changed the title k8s: add CSI driver attributes csi: new component Aug 30, 2024
Copy link
Member

@joaopgrassi joaopgrassi left a comment

Choose a reason for hiding this comment

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

Make sure to update things via the make targets:

make fix
make generate-gh-issue-templates

model/registry/k8s.yaml Outdated Show resolved Hide resolved
docs/attributes-registry/csi.md Outdated Show resolved Hide resolved
@gdvalle
Copy link
Contributor Author

gdvalle commented Sep 4, 2024

I ran into a couple little snags:

@lmolkova
Copy link
Contributor

lmolkova commented Sep 5, 2024

/cc @open-telemetry/semconv-container-approvers @open-telemetry/semconv-k8s-approvers

@gdvalle gdvalle requested review from a team as code owners September 18, 2024 20:47
@gdvalle gdvalle changed the title csi: new component container: add CSI attributes Sep 18, 2024
Copy link
Contributor

@lmolkova lmolkova left a comment

Choose a reason for hiding this comment

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

LGTM!

@lmolkova
Copy link
Contributor

@jmacd could you please take another look? I believe your feedback is addressed.

model/container/registry.yaml Outdated Show resolved Hide resolved
docs/attributes-registry/csi.md Outdated Show resolved Hide resolved
@lmolkova lmolkova merged commit beaeab8 into open-telemetry:main Sep 20, 2024
13 of 14 checks passed
@gdvalle gdvalle deleted the gtd.csi branch September 20, 2024 18:00
drewby pushed a commit to drewby/otel-semantic-conventions that referenced this pull request Oct 1, 2024
Co-authored-by: Joshua MacDonald <[email protected]>
Co-authored-by: Joao Grassi <[email protected]>
Co-authored-by: Christos Markou <[email protected]>
@ChrsMark
Copy link
Member

ChrsMark commented Oct 18, 2024

Hey @gdvalle, is there a reason why those were only added in the registry and not as Resource Attributes as well?
In https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/32055/files#diff-3343de7bfda986546ce7cb166e641ae88c0b0aecadd016cb253cd5a0463ff464R72-R79 I see they are added as Resource Attributes but from the SemConv perspective that's not clear yet, right? So I wonder if we can actually use them in the implementations right now.

@gdvalle
Copy link
Contributor Author

gdvalle commented Oct 21, 2024

Hey @gdvalle, is there a reason why those were only added in the registry and not as Resource Attributes as well? In https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/32055/files#diff-3343de7bfda986546ce7cb166e641ae88c0b0aecadd016cb253cd5a0463ff464R72-R79 I see they are added as Resource Attributes but from the SemConv perspective that's not clear yet, right? So I wonder if we can actually use them in the implementations right now.

No, just my oversight. See #1499 for adding to resource attributes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

k8s: new attributes: CSI driver and volume handle
6 participants