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

Rename cadvisor metric labels to match instrumentation guidelines #69099

Merged
merged 1 commit into from Feb 22, 2019

Conversation

ehashman
Copy link
Member

@ehashman ehashman commented Sep 26, 2018

What this PR does / why we need it:

Changes exported cadvisor metric labels to match the Kubernetes instrumentation guidelines.

Which issue(s) this PR fixes:

Fixes #66790.

Special notes for your reviewer:

See also the metrics overhaul KEP-0031.

Release note:

Added cadvisor metric labels `pod` and `container` where `pod_name` and `container_name` are present to match instrumentation guidelines.

Action required: any Prometheus queries that match `pod_name` and `container_name` labels (e.g. cadvisor or kubelet probe metrics) should be updated to use `pod` and `container` instead. `pod_name` and `container_name` labels will be present alongside `pod` and `container` labels for one transitional release and removed in the future.

@k8s-ci-robot k8s-ci-robot added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 26, 2018
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. area/kubelet sig/node Categorizes an issue or PR as relevant to SIG Node. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Sep 26, 2018
@ehashman ehashman changed the title Rename labels to match instrumentation guidelines Rename cadvisor metric labels to match instrumentation guidelines Sep 26, 2018
@ehashman
Copy link
Member Author

/sig instrumentation
/cc @brancz

@k8s-ci-robot k8s-ci-robot added the sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. label Sep 26, 2018
@ehashman
Copy link
Member Author

/check-cla

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Sep 26, 2018
@idealhack
Copy link
Member

/ok-to-test
/kind bug

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Oct 29, 2018
@coffeepac
Copy link
Contributor

As discussed during F2F at KubeCon, please include both current incorrect and new desired form of the labels.

@ehashman
Copy link
Member Author

Updated PR to include both sets of metrics (also updated the release note).

@ehashman ehashman changed the title Rename cadvisor metric labels to match instrumentation guidelines Rename cadvisor and prober metric labels to match instrumentation guidelines Dec 17, 2018
@coffeepac
Copy link
Contributor

/test pull-kubernetes-e2e-kops-aws
/test pull-kubernetes-local-e2e-containerized

@brancz
Copy link
Member

brancz commented Feb 5, 2019

/retest

@brancz
Copy link
Member

brancz commented Feb 5, 2019

I didn't realize that, let's give it another go then :) (I checked the logs on multiple runs, the failures seemed entirely disconnected)

@brancz
Copy link
Member

brancz commented Feb 7, 2019

/retest

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Feb 7, 2019

@ehashman: The following test failed for commit 3539e89, say /retest to rerun them:

Test name Details Rerun command
pull-kubernetes-e2e-kops-aws 3539e89 link

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@brancz
Copy link
Member

brancz commented Feb 7, 2019

/retest

@dashpole
Copy link
Contributor

dashpole commented Feb 7, 2019

/cc

@ehashman
Copy link
Member Author

Now that tests seem fixed is this GTM?

@coffeepac
Copy link
Contributor

@ehashman needs an 'approved' label. @derekwaynecarr is assigned but you can ping other people in the pkg/kubelet/OWNERS file

@brancz
Copy link
Member

brancz commented Feb 22, 2019

/lgtm

1 similar comment
@danielqsj
Copy link
Contributor

/lgtm

@derekwaynecarr
Copy link
Member

thank you for the change.

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekwaynecarr, ehashman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 22, 2019
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@k8s-ci-robot k8s-ci-robot merged commit 2f29457 into kubernetes:master Feb 22, 2019
ehashman added a commit to ehashman/kubernetes that referenced this pull request Jul 19, 2019
These labels were deprecated in 1.14 (kubernetes#69099) and should no longer be
used in metric queries.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/node Categorizes an issue or PR as relevant to SIG Node. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider renaming "pod_name" labels to "pod" and "container_name" labels to "container" for cAdvisor metrics
10 participants