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

Change run_id log resource attribute to k8s.container.restart_count #5572

Merged
merged 2 commits into from
Oct 6, 2021

Conversation

dmitryax
Copy link
Member

@dmitryax dmitryax commented Oct 3, 2021

Change name of the attribute according to agreement in open-telemetry/opentelemetry-specification#1945. It's better if the change can be merged before the next release to avoid breaking changes in k8sattributes processor after #5467

@dmitryax dmitryax requested a review from owais as a code owner October 3, 2021 05:02
@dmitryax dmitryax requested a review from a team October 3, 2021 05:02
@dmitryax dmitryax force-pushed the run_id-restart_count branch from b2be13a to 55d256e Compare October 3, 2021 05:07
Change name of the attribute according to agreement in open-telemetry/opentelemetry-specification#1945. It's better if the actual change  can be merged before the next release to avoid breaking changes in k8sattributes processor.
@dmitryax dmitryax force-pushed the run_id-restart_count branch from 55d256e to 6080277 Compare October 3, 2021 06:14
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

LGTM, I would feel more comfortable if someone who approved the spec PR approved this one too.

Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

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

LGTM. Let's wait for spec PR to be merged.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Just one question regarding the comment.

processor/k8sattributesprocessor/doc.go Outdated Show resolved Hide resolved
@dmitryax
Copy link
Member Author

dmitryax commented Oct 4, 2021

LGTM. Let's wait for spec PR to be merged.

@tigrannajaryan I believe merging spec might take some time, but, if this PR goes after this week's release, it'll be another "breaking" change -- not so critical tho

@tigrannajaryan
Copy link
Member

LGTM. Let's wait for spec PR to be merged.

@tigrannajaryan I believe merging spec might take some time, but, if this PR goes after this week's release, it'll be another "breaking" change -- not so critical tho

I am only worried that the spec PR may be changed again forcing us to break Collector twice.

@tigrannajaryan tigrannajaryan merged commit 4c03611 into open-telemetry:main Oct 6, 2021
@dmitryax dmitryax deleted the run_id-restart_count branch October 22, 2021 03:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants