-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Fix grafana datasources #2888
Fix grafana datasources #2888
Conversation
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. |
e2ebd92
to
41a7b94
Compare
41a7b94
to
d1a38c2
Compare
/check-cla |
/assign @caseydavenport |
@vasrem please don't assign PRs manually |
@vasrem please add a comment about the Grafana version (something like v5.2.0 or higher is required) in https://github.com/kubernetes/ingress-nginx/blob/master/deploy/grafana/dashboards/README.md |
d345288
to
0b62a83
Compare
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. |
0b62a83
to
2c883d0
Compare
2c883d0
to
c3d9aa8
Compare
I think its ok now. |
Codecov Report
@@ Coverage Diff @@
## master #2888 +/- ##
==========================================
- Coverage 47.56% 47.54% -0.02%
==========================================
Files 76 76
Lines 5483 5483
==========================================
- Hits 2608 2607 -1
- Misses 2540 2542 +2
+ Partials 335 334 -1
Continue to review full report at Codecov.
|
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aledbf, vasrem 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 |
@vasrem thanks! |
@kerneljack Could you test if this feature works with 5.1.3? I did some tests for several previous versions and I saw that it works for >=5.2.0. I'm not sure which feature is required since I didn't delve into the code. |
@vasrem unfortunately I've already upgraded to 5.2.x now, so I can no longer test it for you, apologies :-( |
@vasrem btw, when you say |
@kerneljack I couldn't reply earlier. My mistake. Regarding the feature, I meant the dashboard yaml that I modified in that PR. |
@vasrem no worries, it's ok, it works for us now anyway in 5.2.x so it's good, thanks for fixing it :-) |
@vasrem btw, do you have a list of upcoming features or panels that you are planning on adding to that dashboard? For example, we prefer to see the error rates on a line graph "per-upstream" instead of the table that you have. We just happen to find that a lot more intuitive so I was wondering if you're going to add anything like that at some point. |
What this PR does / why we need it:
We need to assign the correct datasources in the grafana dashboard.
Special notes for your reviewer:
I have submitted two different commits. The first commit (2a3f03c) solves the problem of using default datasource which doesn't make sense in my opinion since in other parts of the file we use prometheus (like here). This leads to wrong data inputs and in addition to wrong visualization of the nginx metrics.
The second commit d1a38c2 fixes a bug which shows on the right corner of grafana the following message.
FYI: I'm using the following setup:
Grafana v5.2.1
Prometheus v2.3.1