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

Added base Grafana dashboard and Alert rules #1745

Merged

Conversation

@jpkrohling
Copy link
Member

jpkrohling commented Aug 20, 2019

Signed-off-by: Juraci Paixão Kröhling juraci@kroehling.de

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
@jpkrohling jpkrohling force-pushed the jpkrohling:Added-base-dashboard-alerts branch from 5cba820 to 2efd17d Aug 20, 2019
@codecov

This comment has been minimized.

Copy link

codecov bot commented Aug 20, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@8954c13). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1745   +/-   ##
=========================================
  Coverage          ?   98.31%           
=========================================
  Files             ?      194           
  Lines             ?     9527           
  Branches          ?        0           
=========================================
  Hits              ?     9366           
  Misses            ?      126           
  Partials          ?       35

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8954c13...9d15605. Read the comment docs.

@pavolloffay

This comment has been minimized.

Copy link
Member

pavolloffay commented Aug 20, 2019

I have two comments:

  • Is there a way to verify that these files work with grafana? An automated test would be useful.
  • Do these definitions work for other systems than grafana? If no I would consider renaming subdirectory jaeger-mixins to grafana. It should help with search but also it tells folks not familiar with grafana that we provide something. monitoring/jaeger-mixin/dashboards-jaeger.json
@jpkrohling

This comment has been minimized.

Copy link
Member Author

jpkrohling commented Aug 20, 2019

Is there a way to verify that these files work with grafana? An automated test would be useful.

Not easily: a test would require a client sending spans to Jaeger, which exposes metrics that are scraped by Prometheus and displayed by Grafana.

Do these definitions work for other systems than grafana?

I'm not sure, but I agree that the dashboard file name could be changed to mention Grafana in the name. I'll also update the readme file.

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
@jpkrohling jpkrohling merged commit 976fa47 into jaegertracing:master Aug 20, 2019
5 checks passed
5 checks passed
DCO DCO
Details
WIP Ready for review
Details
codecov/patch Coverage not affected.
Details
codecov/project 98.31% (target 95%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.