-
Notifications
You must be signed in to change notification settings - Fork 644
Monitoring: Add metric graphs to Alert and Rule details pages #1237
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
Monitoring: Add metric graphs to Alert and Rule details pages #1237
Conversation
Rebase master-next
Merge master into master-next
Merge master into master-next
@kyoto can you add an image on how this looks like? |
@sichvoge Sorry, I hit the create button before adding the screenshots. Now updated. |
/test console-e2e |
FYI @cshinn |
Will need #1226 to fix the test failure |
#1240 fast-forwards master-next for the CI fix |
Merge master into master-next
@sichvoge Good point. There's no legend as part of this PR, but that will need to be added in a future PR. |
e0b3e84
to
a550b15
Compare
Rebased |
Adds a `QueryBrowser` component, which plots a PromQL query as a line graph along with controls for changing the graph to display the query for any time range. Adds the `QueryBrowser` to both the alert details page and the rule details page. The alert graph shows the single metric for that alert and the rule graph shows all metrics data for that rule. Upgrade Plotly to version 1.44.4. Move the "View in Prometheus UI" link from the top of the page to the graph area. Make some changes to the graph Base class. - Make the `title` property optional - Add the ability to invoke `fetch()` without enabling polling - Set a lower limit of 5 seconds on the graph refresh interval to prevent too frequent polling when viewing narrow time ranges. - Add optional `numSamples` property that defines how many data points will be plotted for each metric
a550b15
to
cb4fd3e
Compare
Force pushed a couple of fixes. |
/retest |
2 similar comments
/retest |
/retest |
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.
Nice work on this
/lgtm
const units = {w, d, h, m, s}; | ||
|
||
// Formats a duration in milliseconds like "1h 10m" | ||
export const formatPrometheusDuration = (ms: number) => { |
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.
Wondering if we should update formatDuration
to be this and use it everywhere. Is the primary difference to support longer durations like days and weeks?
const spans = ['5m', '15m', '30m', '1h', '2h', '6h', '12h', '1d', '2d', '1w', '2w']; | ||
const dropdownItems = _.zipObject(spans, spans); | ||
|
||
class QueryBrowser_ extends Line_ { |
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.
General comment that we've been trying to get away from inheritance in React components, but I'm fine with it here since it's how the other charts work.
/retest |
1 similar comment
/retest |
Adds a
QueryBrowser
component, which plots a PromQL query as a linegraph along with controls for changing the graph to display the query
for any time range.
Adds the
QueryBrowser
to both the alert details page and the ruledetails page. The alert graph shows the single metric for that alert and
the rule graph shows all metrics data for that rule.
Upgrade Plotly to version 1.44.4.
Move the "View in Prometheus UI" link from the top of the page to the
graph area.
Make some changes to the graph Base class.
title
property optionalfetch()
without enabling pollingprevent too frequent polling when viewing narrow time ranges.
numSamples
property that defines how many data pointswill be plotted for each metric
Work left for future PRs includes