Bug 1735489 - Show other alerts when opening graph view (#7296)

* Bug 1735489 - Fetch other alerts from backend based on framework id * Bug 1735489 - Add highlight other alerts toggle, add vertical lines to mark other alerts * Bug 1735489 - Add unit test, clean code * Bug 1735489 - Address pr requests * Bug 1735489 - Fix failing test
2021-11-05 11:22:08 +02:00 · 2021-11-05 11:22:08 +02:00 · 3b4504dd0f
--- a/tests/ui/mock/alert_summaries_common.json
+++ b/tests/ui/mock/alert_summaries_common.json
@ -0,0 +1,124 @@
+[
+  [
+    {
+      "id": 32205,
+      "push_id": 980228,
+      "prev_push_id": 980227,
+      "created": "2021-11-01T11:36:53.407841",
+      "repository": "autoland",
+      "framework": 1,
+      "alerts": [
+        {
+          "id": 137494,
+          "status": 0,
+          "series_signature": {
+            "id": 3184996,
+            "framework_id": 1,
+            "signature_hash": "ecac3c7a967df71884a23d925a49e8e96fd21c21",
+            "machine_platform": "macosx1015-64-shippable-qr",
+            "suite": "perf_reftest_singletons",
+            "test": "window-named-property-get.html",
+            "lower_is_better": true,
+            "has_subtests": false,
+            "option_collection_hash": "102210fe594ee9b33d82058545b1ed14f4c8206e",
+            "tags": [""],
+            "extra_options": ["e10s", "stylo", "webrender"],
+            "measurement_unit": "ms",
+            "suite_public_name": null,
+            "test_public_name": null
+          },
+          "is_regression": false,
+          "prev_value": 723.69,
+          "new_value": 649.57,
+          "t_value": 7.01,
+          "amount_abs": -74.12,
+          "amount_pct": 10.24,
+          "summary_id": 32205,
+          "related_summary_id": null,
+          "manually_created": false,
+          "classifier": null,
+          "starred": false,
+          "classifier_email": null,
+          "backfill_record": {
+            "context": "[{\"perf_datum_id\": 1437239345, \"value\": 703.62, \"job_id\": 356253496, \"push_id\": 980182, \"push_timestamp\": \"2021-10-27T19:02:20\", \"push__revision\": \"c24a073833415c904a810f0bc14de21ed8f04def\"}, {\"perf_datum_id\": 1438848232, \"value\": 752.3, \"job_id\": 356594861, \"push_id\": 980227, \"push_timestamp\": \"2021-10-27T20:48:35\", \"push__revision\": \"197af0b52aec261be5f484b48c46c26c8f208ae2\"}, {\"perf_datum_id\": 1438845794, \"value\": 637.2, \"job_id\": 356594352, \"push_id\": 980228, \"push_timestamp\": \"2021-10-27T20:49:39\", \"push__revision\": \"ff38beee092bcdfcc289f4737668c9b5d4ff3d42\"}, {\"perf_datum_id\": 1438591078, \"value\": 700.56, \"job_id\": 356552835, \"push_id\": 980244, \"push_timestamp\": \"2021-10-27T21:17:00\", \"push__revision\": \"dd99452b671854d9f1f3079a051fc4822161a993\"}, {\"perf_datum_id\": 1438585688, \"value\": 635.5699999999999, \"job_id\": 356550571, \"push_id\": 980255, \"push_timestamp\": \"2021-10-27T21:43:08\", \"push__revision\": \"cb160c184b4ac2b4701688e825514f8a3add6a7e\"}]",
+            "status": 0,
+            "total_backfills_triggered": 0
+          },
+          "noise_profile": "N/A"
+        }
+      ],
+      "related_alerts": [],
+      "status": 0,
+      "bug_number": null,
+      "bug_updated": null,
+      "issue_tracker": 1,
+      "notes": null,
+      "revision": "2909b0a1eb06cc34ce0a11544e5e6826aba87c06",
+      "push_timestamp": 1635367779,
+      "prev_push_revision": "197af0b52aec261be5f484b48c46c26c8f208ae2",
+      "assignee_username": null,
+      "assignee_email": null,
+      "performance_tags": []
+    },
+    {
+      "id": 32204,
+      "push_id": 980323,
+      "prev_push_id": 980306,
+      "created": "2021-11-01T11:31:11.031842",
+      "repository": "autoland",
+      "framework": 1,
+      "alerts": [
+        {
+          "id": 137492,
+          "status": 0,
+          "series_signature": {
+            "id": 3869898,
+            "framework_id": 1,
+            "signature_hash": "50fbde7bb038a62d5fe6e00ffa2ea23c31125b7c",
+            "machine_platform": "macosx1015-64-shippable-qr",
+            "suite": "perf_reftest_singletons",
+            "test": "id-getter-2.html",
+            "lower_is_better": true,
+            "has_subtests": false,
+            "option_collection_hash": "102210fe594ee9b33d82058545b1ed14f4c8206e",
+            "tags": [""],
+            "extra_options": ["e10s", "fission", "stylo", "webrender"],
+            "measurement_unit": "ms",
+            "suite_public_name": null,
+            "test_public_name": null
+          },
+          "is_regression": true,
+          "prev_value": 890.04,
+          "new_value": 945.52,
+          "t_value": 9.96,
+          "amount_abs": 55.48,
+          "amount_pct": 6.23,
+          "summary_id": 32204,
+          "related_summary_id": null,
+          "manually_created": false,
+          "classifier": null,
+          "starred": false,
+          "classifier_email": null,
+          "backfill_record": {
+            "context": "[{\"perf_datum_id\": 1438588964, \"value\": 858.7900000000001, \"job_id\": 356551789, \"push_id\": 980300, \"push_timestamp\": \"2021-10-27T22:38:38\", \"push__revision\": \"3d8df6da990da00dae776904d851c50c80aeb16e\"}, {\"perf_datum_id\": 1438591284, \"value\": 900.6400000000001, \"job_id\": 356552898, \"push_id\": 980306, \"push_timestamp\": \"2021-10-27T22:47:19\", \"push__revision\": \"909433910d0d03229dd23cc430d08d6326b44a95\"}, {\"perf_datum_id\": 1437405462, \"value\": 957.5899999999999, \"job_id\": 356285912, \"push_id\": 980323, \"push_timestamp\": \"2021-10-27T23:16:04\", \"push__revision\": \"3d8181bdd6c2b372d65125e9e68b7771d70fce8a\"}, {\"perf_datum_id\": 1437433742, \"value\": 932.48, \"job_id\": 356290338, \"push_id\": 980349, \"push_timestamp\": \"2021-10-28T00:09:34\", \"push__revision\": \"617a98c2fc8fb577df2c6c04e3c08f78dfc65c07\"}, {\"perf_datum_id\": 1438589209, \"value\": 877.1500000000001, \"job_id\": 356552355, \"push_id\": 980360, \"push_timestamp\": \"2021-10-28T00:39:38\", \"push__revision\": \"184d074f07d7d64f5aad4ccb7b683b2e937d3548\"}]",
+            "status": 0,
+            "total_backfills_triggered": 0
+          },
+          "noise_profile": "OK"
+        }
+      ],
+      "related_alerts": [],
+      "status": 0,
+      "bug_number": null,
+      "bug_updated": null,
+      "issue_tracker": 1,
+      "notes": null,
+      "revision": "462d2d4ba0516adc69de20372b16931cef20de9e",
+      "push_timestamp": 1635376564,
+      "prev_push_revision": "909433910d0d03229dd23cc430d08d6326b44a95",
+      "assignee_username": null,
+      "assignee_email": null,
+      "performance_tags": []
+    }
+  ]
+]
--- a/tests/ui/perfherder/graphs-view/graphs_view_test.jsx
+++ b/tests/ui/perfherder/graphs-view/graphs_view_test.jsx
@ -20,6 +20,7 @@ import testData from '../../mock/performance_summary.json';
 import changelogData from '../../mock/infra_changelog.json';
 import seriesData from '../../mock/performance_signature_formatted.json';
 import seriesData2 from '../../mock/performance_signature_formatted2.json';
+import commonAlerts from '../../mock/alert_summaries_common.json';
 import { getProjectUrl } from '../../../../ui/helpers/location';
 import { createGraphData } from '../../../../ui/perfherder/perf-helpers/helpers';
 import {
@ -35,6 +36,7 @@ const graphData = createGraphData(
  [],
  [...graphColors],
  [...graphSymbols],
+  [...commonAlerts],
 );
 const inputPlaceholder = 'filter tests e.g. linux tp5o';
 const frameworks = [
@ -386,3 +388,18 @@ test("'Highlight infra changes' button can be turned off", async () => {

  expect(updateStateParams).toHaveBeenCalledTimes(1);
 });
+
+test("'Highlight other alerts' button can be turned on", async () => {
+  const updateStateParams = jest.fn();
+  const { getByText } = graphsViewControls(graphData, false, updateStateParams);
+
+  const commonAlertsButton = await waitFor(() =>
+    getByText('Highlight common alerts'),
+  );
+
+  expect(commonAlertsButton.classList).not.toContain('active');
+
+  fireEvent.click(commonAlertsButton);
+
+  expect(updateStateParams).toHaveBeenCalledTimes(1);
+});
--- a/tests/ui/perfherder/table_view_test.jsx
+++ b/tests/ui/perfherder/table_view_test.jsx
@ -8,6 +8,7 @@ import {
 } from '../../../ui/perfherder/perf-helpers/constants';
 import repos from '../mock/repositories';
 import testData from '../mock/performance_summary.json';
+import commonAlerts from '../mock/alert_summaries_common.json';
 import { createGraphData } from '../../../ui/perfherder/perf-helpers/helpers';

 const graphData = createGraphData(
@ -15,6 +16,7 @@ const graphData = createGraphData(
  [],
  [...graphColors],
  [...graphSymbols],
+  [...commonAlerts],
 );

 const tableView = (
--- a/ui/perfherder/graphs/GraphTooltip.jsx
+++ b/ui/perfherder/graphs/GraphTooltip.jsx
@ -59,6 +59,7 @@ const GraphTooltip = ({
  const deltaPercent = value / v0 - 1;
  let alert;
  let alertStatus;
+  let isCommonAlert = false;

  if (dataPointDetails.alertSummary && dataPointDetails.alertSummary.alerts) {
    alert = dataPointDetails.alertSummary.alerts.find(
@ -66,6 +67,10 @@ const GraphTooltip = ({
    );
  }

+  if (dataPointDetails.commonAlert) {
+    isCommonAlert = true;
+  }
+
  if (alert) {
    alertStatus =
      alert.status === alertStatusMap.acknowledged && testDetails.alertSummary
@ -199,6 +204,9 @@ const GraphTooltip = ({
                Could be affected by infra changes.
              </p>
            )}
+            {isCommonAlert && !dataPointDetails.alertSummary && (
+              <p className="small text-danger">Common alert</p>
+            )}
          </div>

          <div>
--- a/ui/perfherder/graphs/GraphsContainer.jsx
+++ b/ui/perfherder/graphs/GraphsContainer.jsx
@ -45,6 +45,7 @@ class GraphsContainer extends React.Component {
    const zoomDomain = this.initZoomDomain(scatterPlotData);
    this.state = {
      highlights: [],
+      highlightCommonAlertsData: [],
      scatterPlotData,
      zoomDomain,
      lockTooltip: false,
@ -70,6 +71,7 @@ class GraphsContainer extends React.Component {
  componentDidUpdate(prevProps) {
    const {
      highlightAlerts,
+      highlightCommonAlerts,
      highlightChangelogData,
      highlightedRevisions,
      testData,
@ -81,6 +83,7 @@ class GraphsContainer extends React.Component {

    if (
      prevProps.highlightAlerts !== highlightAlerts ||
+      prevProps.highlightCommonAlerts !== highlightCommonAlerts ||
      prevProps.highlightChangelogData !== highlightChangelogData ||
      prevProps.highlightedRevisions !== highlightedRevisions
    ) {
@ -185,8 +188,14 @@ class GraphsContainer extends React.Component {
  };

  addHighlights = () => {
-    const { testData, highlightAlerts, highlightedRevisions } = this.props;
+    const {
+      testData,
+      highlightAlerts,
+      highlightCommonAlerts,
+      highlightedRevisions,
+    } = this.props;
    let highlights = [];
+    let highlightCommonAlertsData = [];

    for (const series of testData) {
      if (!series.visible) {
@ -198,6 +207,16 @@ class GraphsContainer extends React.Component {
        highlights = [...highlights, ...dataPoints];
      }

+      if (highlightCommonAlerts) {
+        const dataPoints = series.data.filter(
+          (item) => item.commonAlert && !item.alertSummary,
+        );
+        highlightCommonAlertsData = [
+          ...highlightCommonAlertsData,
+          ...dataPoints,
+        ];
+      }
+
      for (const rev of highlightedRevisions) {
        if (!rev) {
          continue;
@ -212,7 +231,7 @@ class GraphsContainer extends React.Component {
        }
      }
    }
-    this.setState({ highlights });
+    this.setState({ highlights, highlightCommonAlertsData });
  };

  getTooltipPosition = (point, yOffset = 15) => ({
@ -364,9 +383,11 @@ class GraphsContainer extends React.Component {
      zoom,
      highlightedRevisions,
      highlightChangelogData,
+      highlightCommonAlerts,
    } = this.props;
    const {
      highlights,
+      highlightCommonAlertsData,
      scatterPlotData,
      zoomDomain,
      lockTooltip,
@ -556,6 +577,22 @@ class GraphsContainer extends React.Component {
                      />
                    ))}

+                  {highlightCommonAlerts &&
+                    highlightCommonAlertsData.length > 0 &&
+                    highlightCommonAlertsData.map((item) => (
+                      <VictoryLine
+                        key={item}
+                        style={{
+                          data: {
+                            stroke: 'gray',
+                            strokeWidth: 1,
+                            strokeDasharray: '5',
+                          },
+                        }}
+                        x={() => item.x}
+                      />
+                    ))}
+
                  {highlightChangelogData && changelogData.length > 0 && (
                    <VictoryBar
                      key="changelog"
--- a/ui/perfherder/graphs/GraphsView.jsx
+++ b/ui/perfherder/graphs/GraphsView.jsx
@ -7,6 +7,8 @@ import queryString from 'query-string';
 import { getData, processResponse, processErrors } from '../../helpers/http';
 import {
  createApiUrl,
+  createQueryParams,
+  getApiUrl,
  parseQueryParams,
  updateQueryParams,
 } from '../../helpers/url';
@ -37,6 +39,7 @@ class GraphsView extends React.Component {
      zoom: {},
      selectedDataPoint: null,
      highlightAlerts: true,
+      highlightCommonAlerts: false,
      highlightChangelogData: true,
      highlightedRevisions: ['', ''],
      testData: [],
@ -87,6 +90,7 @@ class GraphsView extends React.Component {
      zoom,
      selected,
      highlightAlerts,
+      highlightCommonAlerts,
      highlightChangelogData,
      highlightedRevisions,
    } = queryString.parse(this.props.location.search);
@ -103,6 +107,12 @@ class GraphsView extends React.Component {
      updates.highlightAlerts = Boolean(parseInt(highlightAlerts, 10));
    }

+    if (highlightCommonAlerts) {
+      updates.highlightCommonAlerts = Boolean(
+        parseInt(highlightCommonAlerts, 10),
+      );
+    }
+
    if (highlightChangelogData) {
      updates.highlightChangelogData = Boolean(
        parseInt(highlightChangelogData, 10),
@ -197,6 +207,9 @@ class GraphsView extends React.Component {
        this.getAlertSummaries(series.signature_id, series.repository_id),
      ),
    );
+    const commonAlerts = await Promise.all(
+      seriesData.map((series) => this.getCommonAlerts(series.framework_id)),
+    );
    const newColors = [...colors];
    const newSymbols = [...symbols];

@ -205,6 +218,7 @@ class GraphsView extends React.Component {
      alertSummaries.flat(),
      newColors,
      newSymbols,
+      commonAlerts,
    );

    this.setState({ colors: newColors, symbols: newSymbols });
@ -229,6 +243,17 @@ class GraphsView extends React.Component {
    return [];
  };

+  getCommonAlerts = async (frameworkId) => {
+    const params = { framework: frameworkId };
+    const url = getApiUrl(
+      `${endpoints.alertSummary}${createQueryParams(params)}`,
+    );
+    const response = await getData(url);
+    const commonAlerts = [...response.data.results];
+
+    return commonAlerts;
+  };
+
  updateData = async (
    signatureId,
    repositoryName,
@ -292,6 +317,7 @@ class GraphsView extends React.Component {
      selectedDataPoint,
      zoom,
      highlightAlerts,
+      highlightCommonAlerts,
      highlightChangelogData,
      highlightedRevisions,
      timeRange,
@ -304,6 +330,7 @@ class GraphsView extends React.Component {
    const params = {
      series: newSeries,
      highlightAlerts: +highlightAlerts,
+      highlightCommonAlerts: +highlightCommonAlerts,
      highlightChangelogData: +highlightChangelogData,
      timerange: timeRange.value,
      zoom,
@ -339,6 +366,7 @@ class GraphsView extends React.Component {
      timeRange,
      testData,
      highlightAlerts,
+      highlightCommonAlerts,
      highlightChangelogData,
      highlightedRevisions,
      selectedDataPoint,
@ -426,6 +454,7 @@ class GraphsView extends React.Component {
                highlightAlerts={highlightAlerts}
                highlightChangelogData={highlightChangelogData}
                highlightedRevisions={highlightedRevisions}
+                highlightCommonAlerts={highlightCommonAlerts}
                zoom={zoom}
                selectedDataPoint={selectedDataPoint}
                updateStateParams={(state) =>
--- a/ui/perfherder/graphs/GraphsViewControls.jsx
+++ b/ui/perfherder/graphs/GraphsViewControls.jsx
@ -73,6 +73,7 @@ export default class GraphsViewControls extends React.Component {
      highlightAlerts,
      highlightChangelogData,
      highlightedRevisions,
+      highlightCommonAlerts,
      updateTimeRange,
      hasNoData,
      toggle,
@ -182,6 +183,19 @@ export default class GraphsViewControls extends React.Component {
                  >
                    Highlight infra changes
                  </Button>
+                  <Button
+                    className="ml-3"
+                    color="darker-info"
+                    outline
+                    onClick={() =>
+                      updateStateParams({
+                        highlightCommonAlerts: !highlightCommonAlerts,
+                      })
+                    }
+                    active={highlightCommonAlerts}
+                  >
+                    Highlight common alerts
+                  </Button>
                </Col>
              )}
            </Row>
--- a/ui/perfherder/perf-helpers/helpers.js
+++ b/ui/perfherder/perf-helpers/helpers.js
@ -744,7 +744,13 @@ export const retriggerMultipleJobs = async (
  );
 };

-export const createGraphData = (seriesData, alertSummaries, colors, symbols) =>
+export const createGraphData = (
+  seriesData,
+  alertSummaries,
+  colors,
+  symbols,
+  commonAlerts,
+) =>
  seriesData.map((series) => {
    const color = colors.pop();
    const symbol = symbols.pop();
@ -776,6 +782,9 @@ export const createGraphData = (seriesData, alertSummaries, colors, symbols) =>
        alertSummary: alertSummaries.find(
          (item) => item.push_id === dataPoint.push_id,
        ),
+        commonAlert: commonAlerts.some((item) => {
+          return item.find((alert) => alert.push_id === dataPoint.push_id);
+        }),
        signature_id: series.signature_id,
        pushId: dataPoint.push_id,
        jobId: dataPoint.job_id,