-
Notifications
You must be signed in to change notification settings - Fork 659
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 and update scripts for generating grafana dashboards #4382
Fix and update scripts for generating grafana dashboards #4382
Conversation
Signed-off-by: Thomas Newton <[email protected]>
Signed-off-by: Thomas Newton <[email protected]>
Signed-off-by: Thomas Newton <[email protected]>
Signed-off-by: Thomas Newton <[email protected]>
Signed-off-by: Thomas Newton <[email protected]>
Signed-off-by: Thomas Newton <[email protected]>
Signed-off-by: Thomas Newton <[email protected]>
f436ffc
to
de37547
Compare
Signed-off-by: Thomas Newton <[email protected]>
Signed-off-by: Thomas Newton <[email protected]>
bc99145
to
4185eb7
Compare
This is amazing! Thank you! One thing that's still not clear is how these dashboards are published. I'm not sure if we were doing it manually or if there was some automation somewhere (I couldn't find anything in the |
According to this README they are published manually. https://github.com/flyteorg/flyte/blob/master/stats/README.md |
I've just discovered a problem with the flyteadmin dashboard. It seems like latency metrics are not aggregated correctly if you are running flyteadmin with more than one replica. I will try to push a fix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, thank you!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4382 +/- ##
==========================================
+ Coverage 59.81% 60.28% +0.46%
==========================================
Files 632 562 -70
Lines 53644 40314 -13330
==========================================
- Hits 32089 24303 -7786
+ Misses 19029 13657 -5372
+ Partials 2526 2354 -172
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Congrats on merging your first pull request! 🎉 |
Tracking issue
Closes #4346
Describe your changes
generate-dashboard
command works correctly. Previously we were gettinggrafanalib._gen.DefinitionError: Definition /tmp/flyte/stats/flytepropeller_dashboard.py does not define a variable '/tmp/flyte/stats/flytepropeller_dashboard'
json
sflyte-core
deployment. The metric names might be difference onflyte-binary
Check all the applicable boxes
make stats
works but probably it will be difficult to check that all the metric names are correctScreenshots
Note to reviewers
I went through all the graphs in the dashboards to confirm either they have data or there is a valid reason for them not to show anything and that the metrics they use are exposed by flyte.