Skip to content

Fix kubernetes grafana dashboard (#4380)#5012

Merged
alpeb merged 1 commit into
linkerd:mainfrom
aimbot31:master
Sep 29, 2020
Merged

Fix kubernetes grafana dashboard (#4380)#5012
alpeb merged 1 commit into
linkerd:mainfrom
aimbot31:master

Conversation

@aimbot31
Copy link
Copy Markdown
Contributor

Prometheus use a relabel rule that changed since 1.16

Use "pod_name" and "pod" to avoid breaking changes.
Alse use "container" and "container_name" for the
same reasons.

Run some tests on the Grafana dashboard

Fixes #4380

Signed-off-by: Florian Davasse florian.davasse@stack-labs.com

Copy link
Copy Markdown
Member

@alpeb alpeb left a comment

Choose a reason for hiding this comment

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

Thanks @aimbot31 , this looks great. I could verify the offending graphs now are non-empty/not-as-empty as they were before 👍

(Adding ref #4393 in case more context is needed for other reviewers).

Copy link
Copy Markdown
Contributor

@Pothulapati Pothulapati left a comment

Choose a reason for hiding this comment

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

Thanks for taking this up! I have a small nit around pod_name

"interval": "10s",
"intervalFactor": 1,
"legendFormat": "{{ pod_name }}",
"legendFormat": "{{ pod_name }}{{ pod }}",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is pod_name still a correct label? because all I can see in the dashboard is it being statically printed.
2020-09-29_14-50

Should we just remove it? as I see {{ pod }} is added everywhere it is present?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As suggested by alpeb in #4393 I've letted pod and pod_name in order to keep things compatible.
Can you tell me what version did you used ? Have you used kind or minikube or ... ?

Thank's for you answers

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@aimbot31 Ahh, That makes sense! I tried this out in kind.

Do these json files follow the same go templating? (They kinda look like it?) If yes? maybe we can have some kind of check to print only the pod when pod_name is empty?

cc: @alpeb

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Pothulapati That's weird since pod_name should just be empty... Although I did try after merging main that brings a new version of Grafana. @aimbot31 could you please merge main to your branch so we can give it another test?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yup i don't understand this because i'm not seeing pod or pod_name, just the only value filled.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@alpeb Done

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome, Works now. and I don't see pod_name anymore.

Prometheus use a relabel rule that changed since 1.16

Use "pod_name" and "pod" to avoid breaking changes.
Alse use "container" and "container_name" for the
same reasons.

Run some tests on the Grafana dashboard

Fixes linkerd#4380

Signed-off-by: Florian Davasse <florian.davasse@stack-labs.com>
Copy link
Copy Markdown
Contributor

@Pothulapati Pothulapati left a comment

Choose a reason for hiding this comment

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

LGTM

@alpeb alpeb merged commit 7c08fff into linkerd:main Sep 29, 2020
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.

Grafana dashboard isn't using the good label for querying prometheus

3 participants