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.

Sign up for GitHub

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

Jump to bottom

Adds support for rules screen in react-ui #6503

Merged
merged 13 commits into from Jan 27, 2020
Merged

Adds support for rules screen in react-ui #6503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
98a95f4
base
Harkishen-Singh Dec 21, 2019
332e32c
base of rules page
Harkishen-Singh Dec 21, 2019
846bfe0
initial version
Harkishen-Singh Dec 26, 2019
e04f866
removed unused function
Harkishen-Singh Dec 26, 2019
e32ffca
version 1
Harkishen-Singh Dec 27, 2019
bcb85b1
implemented suggestions
Harkishen-Singh Jan 14, 2020
41a334a
fixed conflicts
Harkishen-Singh Jan 14, 2020
fc9dbfd
Merge https://github.com/prometheus/prometheus into react-rules-ui
Harkishen-Singh Jan 17, 2020
40987c2
implemented suggestions
Harkishen-Singh Jan 17, 2020
012ec3a
Merge https://github.com/prometheus/prometheus into react-rules-ui
Harkishen-Singh Jan 24, 2020
ff367c4
implemented suggestions.
Harkishen-Singh Jan 24, 2020
441c285
new fetching pattern
Harkishen-Singh Jan 25, 2020
8ed6406
implemented suggestions
Harkishen-Singh Jan 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
84 changes: 48 additions & 36 deletions web/api/v1/api.go
View file
Open in desktop
Expand Up @@ -897,33 +897,39 @@ type RuleGroup struct {
// In order to preserve rule ordering, while exposing type (alerting or recording)
// specific properties, both alerting and recording rules are exposed in the
// same array.
Rules []rule `json:"rules"`
Interval float64 `json:"interval"`
Rules []rule `json:"rules"`
Interval float64 `json:"interval"`
EvaluationTime float64 `json:"evaluationTime"`
LastEvaluation time.Time `json:"lastEvaluation"`
}

type rule interface{}

type alertingRule struct {
// State can be "pending", "firing", "inactive".
State string `json:"state"`
Name string `json:"name"`
Query string `json:"query"`
Duration float64 `json:"duration"`
Labels labels.Labels `json:"labels"`
Annotations labels.Labels `json:"annotations"`
Alerts []*Alert `json:"alerts"`
Health rules.RuleHealth `json:"health"`
LastError string `json:"lastError,omitempty"`
State string `json:"state"`
Name string `json:"name"`
Query string `json:"query"`
Duration float64 `json:"duration"`
Labels labels.Labels `json:"labels"`
Annotations labels.Labels `json:"annotations"`
Alerts []*Alert `json:"alerts"`
Health rules.RuleHealth `json:"health"`
LastError string `json:"lastError,omitempty"`
EvaluationTime float64 `json:"evaluationTime"`
LastEvaluation time.Time `json:"lastEvaluation"`
// Type of an alertingRule is always "alerting".
Type string `json:"type"`
}
Harkishen-Singh marked this conversation as resolved.
Show resolved Hide resolved

type recordingRule struct {
Name string `json:"name"`
Query string `json:"query"`
Labels labels.Labels `json:"labels,omitempty"`
Health rules.RuleHealth `json:"health"`
LastError string `json:"lastError,omitempty"`
Name string `json:"name"`
Query string `json:"query"`
Labels labels.Labels `json:"labels,omitempty"`
Health rules.RuleHealth `json:"health"`
LastError string `json:"lastError,omitempty"`
EvaluationTime float64 `json:"evaluationTime"`
LastEvaluation time.Time `json:"lastEvaluation"`
// Type of a recordingRule is always "recording".
Type string `json:"type"`
}
Expand All @@ -943,10 +949,12 @@ func (api *API) rules(r *http.Request) apiFuncResult {

for i, grp := range ruleGroups {
apiRuleGroup := &RuleGroup{
Name: grp.Name(),
File: grp.File(),
Interval: grp.Interval().Seconds(),
Rules: []rule{},
Name: grp.Name(),
File: grp.File(),
Interval: grp.Interval().Seconds(),
Rules: []rule{},
EvaluationTime: grp.GetEvaluationDuration().Seconds(),
LastEvaluation: grp.GetEvaluationTimestamp(),
}
for _, r := range grp.Rules() {
var enrichedRule rule
Expand All @@ -961,28 +969,32 @@ func (api *API) rules(r *http.Request) apiFuncResult {
break
}
enrichedRule = alertingRule{
State: rule.State().String(),
Name: rule.Name(),
Query: rule.Query().String(),
Duration: rule.Duration().Seconds(),
Labels: rule.Labels(),
Annotations: rule.Annotations(),
Alerts: rulesAlertsToAPIAlerts(rule.ActiveAlerts()),
Health: rule.Health(),
LastError: lastError,
Type: "alerting",
State: rule.State().String(),
Name: rule.Name(),
Query: rule.Query().String(),
Duration: rule.Duration().Seconds(),
Labels: rule.Labels(),
Annotations: rule.Annotations(),
Alerts: rulesAlertsToAPIAlerts(rule.ActiveAlerts()),
Health: rule.Health(),
LastError: lastError,
EvaluationTime: rule.GetEvaluationDuration().Seconds(),
LastEvaluation: rule.GetEvaluationTimestamp(),
Type: "alerting",
}
case *rules.RecordingRule:
if !returnRecording {
break
}
enrichedRule = recordingRule{
Name: rule.Name(),
Query: rule.Query().String(),
Labels: rule.Labels(),
Health: rule.Health(),
LastError: lastError,
Type: "recording",
Name: rule.Name(),
Query: rule.Query().String(),
Labels: rule.Labels(),
Health: rule.Health(),
LastError: lastError,
EvaluationTime: rule.GetEvaluationDuration().Seconds(),
LastEvaluation: rule.GetEvaluationTimestamp(),
Type: "recording",
}
default:
err := errors.Errorf("failed to assert type of rule '%v'", rule.Name())
Expand Down
11 changes: 11 additions & 0 deletions web/ui/react-app/src/App.css
View file
Open in desktop
Expand Up @@ -237,3 +237,14 @@ button.execute-btn {
margin-right: 5px;
max-height: 20px;
}

.rules-head {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove rulse-head - it's not used anywhere

font-weight: 600;
}

.rule_cell {
white-space: pre-wrap;
background-color: #F5F5F5;
display: block;
font-family: monospace;
}
15 changes: 2 additions & 13 deletions web/ui/react-app/src/pages/alerts/AlertContents.tsx
View file
Open in desktop
Expand Up @@ -3,21 +3,10 @@ import { Badge } from 'reactstrap';
import CollapsibleAlertPanel from './CollapsibleAlertPanel';
import Checkbox from '../../components/Checkbox';
import { isPresent } from '../../utils';
import { Rule } from '../../types/types';

export type RuleState = keyof RuleStatus<any>;

export interface Rule {
alerts: Alert[];
annotations: Record<string, string>;
duration: number;
health: string;
labels: Record<string, string>;
name: string;
query: string;
state: RuleState;
type: string;
}

export interface RuleStatus<T> {
firing: T;
pending: T;
Expand All @@ -29,7 +18,7 @@ export interface AlertsProps {
statsCount: RuleStatus<number>;
}

interface Alert {
export interface Alert {
labels: Record<string, string>;
state: RuleState;
value: string;
Expand Down
8 changes: 3 additions & 5 deletions web/ui/react-app/src/pages/alerts/CollapsibleAlertPanel.tsx
View file
Open in desktop
@@ -1,9 +1,11 @@
import React, { FC, useState, Fragment } from 'react';
import { Link } from '@reach/router';
import { Alert, Collapse, Table, Badge } from 'reactstrap';
import { Rule, RuleStatus } from './AlertContents';
import { RuleStatus } from './AlertContents';
import { Rule } from '../../types/types';
import { faChevronDown, faChevronRight } from '@fortawesome/free-solid-svg-icons';
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
import { createExpressionLink } from '../../utils/index';

interface CollapsibleAlertPanelProps {
rule: Rule;
Expand All @@ -16,10 +18,6 @@ const alertColors: RuleStatus<string> = {
inactive: 'success',
};

const createExpressionLink = (expr: string) => {
return `../graph?g0.expr=${encodeURIComponent(expr)}&g0.tab=1&g0.stacked=0&g0.range_input=1h`;
};

const CollapsibleAlertPanel: FC<CollapsibleAlertPanelProps> = ({ rule, showAnnotations }) => {
const [open, toggle] = useState(false);

Expand Down
21 changes: 11 additions & 10 deletions web/ui/react-app/src/pages/rules/Rules.tsx
View file
Open in desktop
@@ -1,15 +1,16 @@
import React, { FC } from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you align response handling with the pattern used in other pages i.e check how withStatusIndicator HOC is used

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you align response handling with the pattern used in other pages i.e check how withStatusIndicator HOC is used

Better to merge this one first #6629

import { RouteComponentProps } from '@reach/router';
import PathPrefixProps from '../../types/PathPrefixProps';
import { Alert } from 'reactstrap';

const Rules: FC<RouteComponentProps & PathPrefixProps> = ({ pathPrefix }) => (
<>
<h2>Rules</h2>
<Alert color="warning">
This page is still under construction. Please try it in the <a href={`${pathPrefix}/rules`}>Classic UI</a>.
</Alert>
</>
);
import { useFetch } from '../../hooks/useFetch';
import { withStatusIndicator } from '../../components/withStatusIndicator';
import { RulesMap, RulesContent } from './RulesContent';

const RulesWithStatusIndicator = withStatusIndicator(RulesContent);

const Rules: FC<RouteComponentProps & PathPrefixProps> = ({ pathPrefix }) => {
const { response, error, isLoading } = useFetch<RulesMap>(`${pathPrefix}/api/v1/rules`);

return <RulesWithStatusIndicator response={response} error={error} isLoading={isLoading} />;
};

export default Rules;
131 changes: 131 additions & 0 deletions web/ui/react-app/src/pages/rules/RulesContent.tsx
View file
Open in desktop
@@ -0,0 +1,131 @@
import React, { FC } from 'react';
import { RouteComponentProps } from '@reach/router';
import { APIResponse } from '../../hooks/useFetch';
import { Alert, Table, Badge } from 'reactstrap';
import { Link } from '@reach/router';
import { formatRelative, createExpressionLink, humanizeDuration } from '../../utils';
import { Rule } from '../../types/types';
import { now } from 'moment';

interface RulesContentProps {
response: APIResponse<RulesMap>;
}

interface RuleGroup {
name: string;
file: string;
rules: Rule[];
evaluationTime: string;
lastEvaluation: string;
}

export interface RulesMap {
groups: RuleGroup[];
}

const GraphExpressionLink: FC<{ expr: string; title: string }> = props => {
Copy link
Contributor

@boyskila boyskila Jan 25, 2020 •
edited

Choose a reason for hiding this comment

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

This component should look like:

export const GraphExpressionLink: FC<{ expr: string; title: string; name: string  & PathPrefixProps}> = memo(({ name, title, expr, pathPrefix }) => {
  return (
    <div>
      {title}:
      <Link className="ml-4" to={createExpressionLink(expr, pathPrefix)}>
        {name}
      </Link>
    </div>
  );
});

name and expr may have different content, so we can't rely on expr to represent the name
Also, wrapping the Link and title in div will remove the need of <br/>. IMO it's better.
Comparing with the old UI titles here are wrapped in <strong>, maybe you should remove the extra tag

Another, more important thing is to pas the pathPrefix prop down to createExpressionLink. IMO the ../ in createExpressionLink should be passed from outside and the function just to return
${prefix}graph?g0.expr=${encodeURIComponent(expr)}&g0.tab=1&g0.stacked=0&g0.range_input=1h
The usage should looks like createExpressionLink(expr, pathPrefix + '/../')

Suggestion: GraphExpressionLink can be moved to component folder and can replace both expressions here

prometheus/web/ui/react-app/src/pages/alerts/CollapsibleAlertPanel.tsx

Lines 35 to 41 in d996ba2

<div>
name: <Link to={createExpressionLink(`ALERTS{alertname="${rule.name}"}`)}>{rule.name}</Link>
</div>
<div>
expr: <Link to={createExpressionLink(rule.query)}>{rule.query}</Link>
</div>
<div>
as well

return (
<>
<strong>{props.title}:</strong>
<Link className="ml-4" to={createExpressionLink(props.expr)}>
{props.expr}
</Link>
<br />
</>
);
};

export const RulesContent: FC<RouteComponentProps & RulesContentProps> = ({ response }) => {
const getBadgeColor = (state: string) => {
switch (state) {
case 'ok':
return 'success';

case 'err':
return 'danger';

case 'unknown':
return 'warning';
}
};

if (response.data) {
const groups: RuleGroup[] = response.data.groups;
return (
<>
<h2>Rules</h2>
{groups.map((g, i) => {
return (
<Table bordered key={i}>
<thead>
<tr>
<td colSpan={3}>
<a href={'#' + g.name}>
<h2 id={g.name}>{g.name}</h2>
</a>
</td>
<td>
<h2>{formatRelative(g.lastEvaluation, now())} ago</h2>
</td>
<td>
<h2>{humanizeDuration(parseFloat(g.evaluationTime) * 1000)}</h2>
</td>
</tr>
</thead>
<tbody>
<tr className="font-weight-bold">
<td>Rule</td>
<td>State</td>
<td>Error</td>
<td>Last Evaluation</td>
<td>Evaluation Time</td>
</tr>
{g.rules.map((r, i) => {
return (
<tr key={i}>
{r.alerts ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

<td> content can be reduced to

<td className="rule_cell">
      <GraphExpressionLink name={r.name} title={r.alerts ? 'alert' : 'record'} expr={`ALERTS{alertname="${r.name}"}`}/>
      <GraphExpressionLink title="expr" expr={r.query} name={r.query} />
         {r.alerts && (
              <>
                 labels:
                 {Object.entries(r.labels).map(([key, value]) => (
                     <div className="ml-4" key={key}>
                         {key}: {value}
                      </div>
                   ))}
                    annotations:
                    {Object.entries(r.annotations).map(([key, value]) => (
                        <div className="ml-4" key={key}>
                            {key}: {value}
                         </div>
                      ))}
                  </>
             )}
 </td>

Some of the divs are redundant and.
Alse, note that first GraphExpressionLink has as an expression ALERTS{alertname="${r.name}"}. Passing just r.name as expr is not enough and its not working

<td style={{ backgroundColor: '#F5F5F5' }} className="rule_cell">
<GraphExpressionLink title="alert" expr={r.name} />
<GraphExpressionLink title="expr" expr={r.query} />
<div>
<strong>labels:</strong>
{Object.entries(r.labels).map(([key, value]) => (
<div className="ml-4" key={key}>
{key}: {value}
</div>
))}
</div>
<div>
<strong>annotations:</strong>
{Object.entries(r.annotations).map(([key, value]) => (
<div className="ml-4" key={key}>
{key}: {value}
</div>
))}
</div>
</td>
) : (
<td style={{ backgroundColor: '#F5F5F5' }}>
<GraphExpressionLink title="record" expr={r.name} />
<GraphExpressionLink title="expr" expr={r.query} />
</td>
)}
<td>
<Badge color={getBadgeColor(r.health)}>{r.health.toUpperCase()}</Badge>
</td>
<td>{r.lastError ? <Alert color="danger">{r.lastError}</Alert> : null}</td>
<td>{formatRelative(r.lastEvaluation, now())} ago</td>
<td>{humanizeDuration(parseFloat(r.evaluationTime) * 1000)}</td>
</tr>
);
})}
</tbody>
</Table>
);
})}
</>
);
}

return null;
};
17 changes: 17 additions & 0 deletions web/ui/react-app/src/types/types.ts
View file
Open in desktop
@@ -1,3 +1,5 @@
import { Alert, RuleState } from '../pages/alerts/AlertContents';

export interface Metric {
[key: string]: string;
}
Expand All @@ -7,3 +9,18 @@ export interface QueryParams {
endTime: number;
resolution: number;
}

export interface Rule {
alerts: Alert[];
annotations: Record<string, string>;
duration: number;
evaluationTime: string;
health: string;
labels: Record<string, string>;
lastError?: string;
lastEvaluation: string;
name: string;
query: string;
state: RuleState;
type: string;
}
4 changes: 4 additions & 0 deletions web/ui/react-app/src/utils/index.ts
View file
Open in desktop
Expand Up @@ -197,3 +197,7 @@ export const toQueryString = ({ key, options }: PanelMeta) => {
export const encodePanelOptionsToQueryString = (panels: PanelMeta[]) => {
return `?${panels.map(toQueryString).join('&')}`;
};

export const createExpressionLink = (expr: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please pass as second argument the pathPrefix prop and remove ../ - this should be passed from outside as well

return `../graph?g0.expr=${encodeURIComponent(expr)}&g0.tab=1&g0.stacked=0&g0.range_input=1h`;
};