Browse Source

feat(crons): Merge timeline ticks in crons timeline (#51136)

We request data from the backend in "1 pixel" buckets to have an
accurate timeline view. When rendering however, we would ideally like to
merge consecutive identical status counts (ex: merging 100 consecutive
OK runs into one UI element) to avoid situations like this:

<img width="1243" alt="image"
src="https://github.com/getsentry/sentry/assets/9372512/f624c039-865d-4605-a20c-7f47e60044aa">

The above chart is hard to read and also not performant by having to
draw hundreds of dom nodes unnecessarily.

After:
<img width="1250" alt="image"
src="https://github.com/getsentry/sentry/assets/9372512/354517bd-277e-418c-a88a-21c1c683e37e">


The merging strategy is relatively simple
- Merge consecutive tick marks of equivalent status
- But always maintain a minimum tick mark width of 4px

Having to maintain a width of 4px means that an example like `[OK, OK,
OK, MISSED, MISSED, MISSED, MISSED]` would not turn into `[OK * 3,
MISSED * 4]` as the first `OK` tick mark would only be 3 pixels wide.
Instead we are forced to group at least `[OK, OK, OK, MISSED]` into one
yellow tick mark (since we want to surface the most important status).
However this means that we should actually group all 7 job runs into one
long yellow tick mark.

This is what you may see here with this yellow and green band expressed
as one longer yellow band
<img width="135" alt="image"
src="https://github.com/getsentry/sentry/assets/9372512/f59c26d1-78b0-4607-a724-3ca7f5cb465e">
<img width="135" alt="image"
src="https://github.com/getsentry/sentry/assets/9372512/ba3b0050-319c-4520-b25e-b464732f5d54">


A tooltip will delineate the count of each status of check-in (upcoming
PR), and in the future the user will be able to zoom to a deeper level
so each job run is shown without merging into previous
David Wang 1 year ago
parent
commit
95d77172be

+ 40 - 44
static/app/views/monitors/components/checkInTimeline.tsx

@@ -5,11 +5,10 @@ import DateTime from 'sentry/components/dateTime';
 import {Resizeable} from 'sentry/components/replays/resizeable';
 import {Tooltip} from 'sentry/components/tooltip';
 import {space} from 'sentry/styles/space';
-import {
-  MonitorBucketData,
-  MonitorBucketEnvMapping,
-} from 'sentry/views/monitors/components/overviewTimeline/types';
+import {MonitorBucketData} from 'sentry/views/monitors/components/overviewTimeline/types';
 import {CheckInStatus} from 'sentry/views/monitors/types';
+import {getAggregateStatus} from 'sentry/views/monitors/utils/getAggregateStatus';
+import {mergeBuckets} from 'sentry/views/monitors/utils/mergeBuckets';
 
 interface Props {
   bucketedData: MonitorBucketData;
@@ -18,25 +17,6 @@ interface Props {
   width?: number;
 }
 
-function getAggregateStatus(envData: MonitorBucketEnvMapping) {
-  // Orders the status in terms of precedence for showing to the user
-  const statusOrdering = [
-    CheckInStatus.OK,
-    CheckInStatus.MISSED,
-    CheckInStatus.TIMEOUT,
-    CheckInStatus.ERROR,
-  ];
-
-  return Object.values(envData).reduce((currentStatus, value) => {
-    for (const [index, status] of statusOrdering.entries()) {
-      if (value[status] > 0 && index > statusOrdering.indexOf(currentStatus)) {
-        currentStatus = status;
-      }
-    }
-    return currentStatus;
-  }, CheckInStatus.OK);
-}
-
 function getColorFromStatus(status: CheckInStatus, theme: Theme) {
   const statusToColor: Record<CheckInStatus, string> = {
     [CheckInStatus.ERROR]: theme.red200,
@@ -64,27 +44,29 @@ export function CheckInTimeline(props: Props) {
     const timeWindow = end.getTime() - start.getTime();
     const msPerPixel = timeWindow / width;
 
+    const jobTicks = mergeBuckets(bucketedData);
+
     return (
       <TimelineContainer>
-        {bucketedData.map(([timestamp, envData]) => {
-          const timestampMs = timestamp * 1000;
-          if (Object.keys(envData).length === 0) {
-            return null;
+        {jobTicks.map(
+          ({startTs, width: tickWidth, envMapping, roundedLeft, roundedRight}) => {
+            const timestampMs = startTs * 1000;
+            const left = getBucketedCheckInsPosition(timestampMs, start, msPerPixel);
+
+            return (
+              <JobTickContainer style={{left}} key={startTs}>
+                <Tooltip title={<DateTime date={timestampMs} seconds />}>
+                  <JobTick
+                    style={{width: tickWidth}}
+                    status={getAggregateStatus(envMapping)}
+                    roundedLeft={roundedLeft}
+                    roundedRight={roundedRight}
+                  />
+                </Tooltip>
+              </JobTickContainer>
+            );
           }
-
-          const left = getBucketedCheckInsPosition(timestampMs, start, msPerPixel);
-          if (left < 0) {
-            return null;
-          }
-
-          return (
-            <JobTickContainer style={{left}} key={timestamp}>
-              <Tooltip title={<DateTime date={timestampMs} seconds />}>
-                <JobTick status={getAggregateStatus(envData)} />
-              </Tooltip>
-            </JobTickContainer>
-          );
-        })}
+        )}
       </TimelineContainer>
     );
   }
@@ -104,12 +86,26 @@ const TimelineContainer = styled('div')`
 
 const JobTickContainer = styled('div')`
   position: absolute;
-  transform: translateX(-50%);
 `;
 
-const JobTick = styled('div')<{status: CheckInStatus}>`
+const JobTick = styled('div')<{
+  roundedLeft: boolean;
+  roundedRight: boolean;
+  status: CheckInStatus;
+}>`
   background: ${p => getColorFromStatus(p.status, p.theme)};
   width: 4px;
   height: 14px;
-  border-radius: 6px;
+  ${p =>
+    p.roundedLeft &&
+    `
+    border-top-left-radius: 2px;
+    border-bottom-left-radius: 2px;
+  `}
+  ${p =>
+    p.roundedRight &&
+    `
+    border-top-right-radius: 2px;
+    border-bottom-right-radius: 2px;
+  `}
 `;

+ 17 - 1
static/app/views/monitors/components/overviewTimeline/types.tsx

@@ -21,4 +21,20 @@ export type TimeWindowData = Record<TimeWindow, TimeWindowOptions>;
 
 export type MonitorBucketData = [timestamp: number, envData: MonitorBucketEnvMapping][];
 
-export type MonitorBucketEnvMapping = Record<string, Record<CheckInStatus, number>>;
+export interface JobTickData {
+  endTs: number;
+  envMapping: MonitorBucketEnvMapping;
+  roundedLeft: boolean;
+  roundedRight: boolean;
+  startTs: number;
+  width: number;
+}
+
+export type StatsBucket = {
+  [CheckInStatus.OK]: number;
+  [CheckInStatus.MISSED]: number;
+  [CheckInStatus.TIMEOUT]: number;
+  [CheckInStatus.ERROR]: number;
+};
+
+export type MonitorBucketEnvMapping = Record<string, StatsBucket>;

+ 10 - 0
static/app/views/monitors/utils/constants.tsx

@@ -0,0 +1,10 @@
+import {StatsBucket} from 'sentry/views/monitors/components/overviewTimeline/types';
+import {CheckInStatus} from 'sentry/views/monitors/types';
+
+// Orders the status in terms of precedence for showing to the user
+export const CHECKIN_STATUS_PRECEDENT = [
+  CheckInStatus.OK,
+  CheckInStatus.MISSED,
+  CheckInStatus.TIMEOUT,
+  CheckInStatus.ERROR,
+] satisfies Array<keyof StatsBucket>;

+ 21 - 0
static/app/views/monitors/utils/getAggregateStatus.spec.tsx

@@ -0,0 +1,21 @@
+import {CheckInStatus} from 'sentry/views/monitors/types';
+
+import {getAggregateStatus} from './getAggregateStatus';
+
+type StatusCounts = [ok: number, missed: number, timeout: number, error: number];
+
+export function generateEnvMapping(name: string, counts: StatusCounts) {
+  const [ok, missed, timeout, error] = counts;
+  return {
+    [name]: {ok, timeout, error, missed},
+  };
+}
+describe('getAggregateStatus', function () {
+  it('aggregates correctly across multiple envs', function () {
+    const envData = {
+      ...generateEnvMapping('prod', [1, 2, 0, 1]),
+      ...generateEnvMapping('dev', [1, 0, 1, 0]),
+    };
+    expect(getAggregateStatus(envData)).toEqual(CheckInStatus.ERROR);
+  });
+});

+ 14 - 0
static/app/views/monitors/utils/getAggregateStatus.tsx

@@ -0,0 +1,14 @@
+import {MonitorBucketEnvMapping} from 'sentry/views/monitors/components/overviewTimeline/types';
+
+import {CHECKIN_STATUS_PRECEDENT} from './constants';
+
+export function getAggregateStatus(envData: MonitorBucketEnvMapping) {
+  return Object.values(envData).reduce((currentStatus, value) => {
+    for (const [index, status] of CHECKIN_STATUS_PRECEDENT.entries()) {
+      if (value[status] > 0 && index > CHECKIN_STATUS_PRECEDENT.indexOf(currentStatus)) {
+        currentStatus = status;
+      }
+    }
+    return currentStatus;
+  }, CHECKIN_STATUS_PRECEDENT[0]);
+}

+ 23 - 0
static/app/views/monitors/utils/getAggregateStatusFromMultipleBuckets.spec.tsx

@@ -0,0 +1,23 @@
+import {CheckInStatus} from 'sentry/views/monitors/types';
+
+import {getAggregateStatusFromMultipleBuckets} from './getAggregateStatusFromMultipleBuckets';
+
+type StatusCounts = [ok: number, missed: number, timeout: number, error: number];
+
+export function generateEnvMapping(name: string, counts: StatusCounts) {
+  const [ok, missed, timeout, error] = counts;
+  return {
+    [name]: {ok, timeout, error, missed},
+  };
+}
+describe('getAggregateStatusFromMultipleBuckets', function () {
+  it('aggregates correctly across multiple envs', function () {
+    const envData1 = generateEnvMapping('prod', [1, 2, 1, 0]);
+    const envData2 = generateEnvMapping('dev', [2, 0, 0, 0]);
+    const envData3 = generateEnvMapping('prod', [1, 1, 3, 0]);
+
+    const status = getAggregateStatusFromMultipleBuckets([envData1, envData2, envData3]);
+
+    expect(status).toEqual(CheckInStatus.TIMEOUT);
+  });
+});

+ 23 - 0
static/app/views/monitors/utils/getAggregateStatusFromMultipleBuckets.tsx

@@ -0,0 +1,23 @@
+import {MonitorBucketEnvMapping} from 'sentry/views/monitors/components/overviewTimeline/types';
+
+import {CHECKIN_STATUS_PRECEDENT} from './constants';
+import {getAggregateStatus} from './getAggregateStatus';
+
+/**
+ * Given multiple env buckets [{prod: {ok: 1, ...}, {prod: {ok: 0, ...}}]
+ * returns the aggregate status across all buckets
+ */
+export function getAggregateStatusFromMultipleBuckets(
+  envDataArr: MonitorBucketEnvMapping[]
+) {
+  return envDataArr
+    .map(getAggregateStatus)
+    .reduce(
+      (aggregateStatus, currentStatus) =>
+        CHECKIN_STATUS_PRECEDENT.indexOf(currentStatus) >
+        CHECKIN_STATUS_PRECEDENT.indexOf(aggregateStatus)
+          ? currentStatus
+          : aggregateStatus,
+      CHECKIN_STATUS_PRECEDENT[0]
+    );
+}

+ 13 - 0
static/app/views/monitors/utils/isEnvMappingEmpty.spec.tsx

@@ -0,0 +1,13 @@
+import {isEnvMappingEmpty} from './isEnvMappingEmpty';
+
+describe('isEnvMappingEmpty', function () {
+  it('returns true for an empty env', function () {
+    const envMapping = {};
+    expect(isEnvMappingEmpty(envMapping)).toEqual(true);
+  });
+
+  it('returns false for a filled env', function () {
+    const envMapping = {prod: {ok: 1, missed: 0, timeout: 0, error: 0, in_progress: 0}};
+    expect(isEnvMappingEmpty(envMapping)).toEqual(false);
+  });
+});

+ 8 - 0
static/app/views/monitors/utils/isEnvMappingEmpty.tsx

@@ -0,0 +1,8 @@
+import {MonitorBucketEnvMapping} from 'sentry/views/monitors/components/overviewTimeline/types';
+
+/**
+ * Determines if an environment mapping includes any job run data
+ */
+export function isEnvMappingEmpty(envMapping: MonitorBucketEnvMapping) {
+  return Object.keys(envMapping).length === 0;
+}

+ 114 - 0
static/app/views/monitors/utils/mergeBuckets.spec.tsx

@@ -0,0 +1,114 @@
+import {MonitorBucketData} from 'sentry/views/monitors/components/overviewTimeline/types';
+import {CheckInStatus} from 'sentry/views/monitors/types';
+
+import {mergeBuckets} from './mergeBuckets';
+
+type StatusCounts = [ok: number, missed: number, timeout: number, error: number];
+
+function generateEnvMapping(name: string, counts: StatusCounts) {
+  const [ok, missed, timeout, error] = counts;
+  return {
+    [name]: {ok, timeout, error, missed},
+  };
+}
+
+function generateJobRun(envName: string, jobStatus: CheckInStatus) {
+  const sortedStatuses = [
+    CheckInStatus.OK,
+    CheckInStatus.MISSED,
+    CheckInStatus.TIMEOUT,
+    CheckInStatus.ERROR,
+    CheckInStatus.IN_PROGRESS,
+  ];
+  const counts: StatusCounts = [0, 0, 0, 0];
+  counts[sortedStatuses.indexOf(jobStatus)] = 1;
+  return generateEnvMapping(envName, counts);
+}
+
+describe('mergeBuckets', function () {
+  it('does not generate ticks less than 3px width', function () {
+    const bucketData: MonitorBucketData = [
+      [1, generateJobRun('prod', CheckInStatus.OK)],
+      [2, generateJobRun('prod', CheckInStatus.OK)],
+      [3, generateJobRun('prod', CheckInStatus.OK)],
+      [4, {}],
+      [5, generateJobRun('prod', CheckInStatus.OK)],
+      [6, generateJobRun('prod', CheckInStatus.OK)],
+      [7, generateJobRun('prod', CheckInStatus.OK)],
+      [8, generateJobRun('prod', CheckInStatus.OK)],
+    ];
+    const mergedData = mergeBuckets(bucketData);
+    const expectedMerged = [
+      {
+        startTs: 1,
+        endTs: 8,
+        width: 8,
+        roundedLeft: true,
+        roundedRight: true,
+        envMapping: generateEnvMapping('prod', [7, 0, 0, 0]),
+      },
+    ];
+
+    expect(mergedData).toEqual(expectedMerged);
+  });
+
+  it('generates adjacent ticks without border radius', function () {
+    const bucketData: MonitorBucketData = [
+      [1, generateJobRun('prod', CheckInStatus.OK)],
+      [2, generateJobRun('prod', CheckInStatus.OK)],
+      [3, generateJobRun('prod', CheckInStatus.OK)],
+      [4, generateJobRun('prod', CheckInStatus.OK)],
+      [5, generateJobRun('prod', CheckInStatus.MISSED)],
+      [6, generateJobRun('prod', CheckInStatus.TIMEOUT)],
+      [7, generateJobRun('prod', CheckInStatus.MISSED)],
+      [8, generateJobRun('prod', CheckInStatus.MISSED)],
+    ];
+    const mergedData = mergeBuckets(bucketData);
+    const expectedMerged = [
+      {
+        startTs: 1,
+        endTs: 4,
+        width: 4,
+        roundedLeft: true,
+        roundedRight: false,
+        envMapping: generateEnvMapping('prod', [4, 0, 0, 0]),
+      },
+      {
+        startTs: 5,
+        endTs: 8,
+        width: 4,
+        roundedLeft: false,
+        roundedRight: true,
+        envMapping: generateEnvMapping('prod', [0, 3, 1, 0]),
+      },
+    ];
+
+    expect(mergedData).toEqual(expectedMerged);
+  });
+
+  it('does not generate a separate tick if the next generated tick would be the same status', function () {
+    const bucketData: MonitorBucketData = [
+      [1, generateJobRun('prod', CheckInStatus.TIMEOUT)],
+      [2, generateJobRun('prod', CheckInStatus.TIMEOUT)],
+      [3, generateJobRun('prod', CheckInStatus.TIMEOUT)],
+      [4, generateJobRun('prod', CheckInStatus.TIMEOUT)],
+      [5, generateJobRun('prod', CheckInStatus.MISSED)],
+      [6, generateJobRun('prod', CheckInStatus.OK)],
+      [7, generateJobRun('prod', CheckInStatus.MISSED)],
+      [8, generateJobRun('prod', CheckInStatus.TIMEOUT)],
+    ];
+    const mergedData = mergeBuckets(bucketData);
+    const expectedMerged = [
+      {
+        startTs: 1,
+        endTs: 8,
+        width: 8,
+        roundedLeft: true,
+        roundedRight: true,
+        envMapping: generateEnvMapping('prod', [1, 2, 5, 0]),
+      },
+    ];
+
+    expect(mergedData).toEqual(expectedMerged);
+  });
+});

Some files were not shown because too many files changed in this diff