feat(aci): Register new metric detector chartcuterie chart in frontend #50
Conversation
|
Add Chartcuterie Support for Metric Detector Charts in Frontend This PR introduces frontend support for rendering metric detector charts using Chartcuterie by largely duplicating the approach used for metric alert charts. Major changes include implementing new chart config logic for detectors, registering new chart types for metric detectors, and updating the core Chartcuterie configuration and types to handle and distinguish detector-based chart events. The PR adapts existing metric alert chart options to accept detector and open period data, enabling notifications for the new detector system as an interim solution before planned UI improvements. Key Changes• Added new file Affected Areas• This summary was automatically generated by @propel-code-bot |
![propel-test-bot[bot]](https://avatars.githubusercontent.com/u/175999184?s=60&v=4){kind=small}
| series[0]!.z = 1; | ||
| series[0]!.color = colors[0]; | ||
|
|
||
| const dataArr = timeseriesData[0]!.data; |
There was a problem hiding this comment.
[CriticalError]
Potential array bounds issue: timeseriesData[0]!.data assumes the arrays have at least one element. If timeseriesData is empty, this will cause a runtime error when accessing array properties.
| const dataArr = timeseriesData[0]!.data; | |
| if (!timeseriesData.length) { | |
| throw new Error('Timeseries data is required for metric detector chart'); | |
| } | |
| const dataArr = timeseriesData[0].data; |
⚡ Committable suggestion
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Context for Agents
[**CriticalError**]
Potential array bounds issue: `timeseriesData[0]!.data` assumes the arrays have at least one element. If `timeseriesData` is empty, this will cause a runtime error when accessing array properties.
```suggestion
if (!timeseriesData.length) {
throw new Error('Timeseries data is required for metric detector chart');
}
const dataArr = timeseriesData[0].data;
```
⚡ **Committable suggestion**
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
File: static/app/views/detectors/components/details/metric/charts/metricDetectorChartOptions.tsx
Line: 182| }; | ||
| } | ||
|
|
||
| function getWaitingForDataRange(dataArr: any) { |
There was a problem hiding this comment.
[BestPractice]
Type safety issue: getWaitingForDataRange parameter dataArr is typed as any, which defeats TypeScript's type checking. This could lead to runtime errors if the wrong data structure is passed.
| function getWaitingForDataRange(dataArr: any) { | |
| function getWaitingForDataRange(dataArr: Array<{value: number; name: string}>): {startIndex: number; endIndex: number} { |
⚡ Committable suggestion
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Context for Agents
[**BestPractice**]
Type safety issue: `getWaitingForDataRange` parameter `dataArr` is typed as `any`, which defeats TypeScript's type checking. This could lead to runtime errors if the wrong data structure is passed.
```suggestion
function getWaitingForDataRange(dataArr: Array<{value: number; name: string}>): {startIndex: number; endIndex: number} {
```
⚡ **Committable suggestion**
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
File: static/app/views/detectors/components/details/metric/charts/metricDetectorChartOptions.tsx
Line: 414| function getWaitingForDataRange(dataArr: any) { | ||
| if (dataArr[0].value > 0) { |
There was a problem hiding this comment.
[CriticalError]
Potential array bounds issue: The function doesn't validate that dataArr has elements before accessing dataArr[0] and dataArr[i]. An empty array will cause runtime errors.
Suggested Change
| function getWaitingForDataRange(dataArr: any) { | |
| if (dataArr[0].value > 0) { | |
| function getWaitingForDataRange(dataArr: Array<{value: number; name: string}>): {startIndex: number; endIndex: number} { | |
| if (!dataArr.length) { | |
| return {startIndex: 0, endIndex: 0}; | |
| } | |
| if (dataArr[0].value > 0) { | |
| return {startIndex: 0, endIndex: 0}; | |
| } |
⚡ Committable suggestion
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Context for Agents
[**CriticalError**]
Potential array bounds issue: The function doesn't validate that `dataArr` has elements before accessing `dataArr[0]` and `dataArr[i]`. An empty array will cause runtime errors.
<details>
<summary>Suggested Change</summary>
```suggestion
function getWaitingForDataRange(dataArr: Array<{value: number; name: string}>): {startIndex: number; endIndex: number} {
if (!dataArr.length) {
return {startIndex: 0, endIndex: 0};
}
if (dataArr[0].value > 0) {
return {startIndex: 0, endIndex: 0};
}
```
⚡ **Committable suggestion**
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
</details>
File: static/app/views/detectors/components/details/metric/charts/metricDetectorChartOptions.tsx
Line: 415
Adds a copy of the metric alert chartcuterie chart options, but for detectors. Should output the same chart, but it accepts detector and open period data instead of rule and incident data.
This is a stopgap to get notifications working in the new system asap. My intention is to later on put our new chart UI here so that the new alerts will look like the new UI.
Copied from getsentry#101171
Original PR: getsentry#101171