feat(aci): Metric monitor form should default to number of errors #104878
Conversation
scttcper
left a comment
There was a problem hiding this comment.
i never did figure out a good solution for removing is:unresolved when switching
| aggregateFunction: 'count()', | ||
| interval: 60 * 60, // One hour in seconds | ||
| query: '', | ||
| query: 'is:unresolved', |
There was a problem hiding this comment.
Bug: Error-specific query persists when switching to Spans dataset
The default query 'is:unresolved' is now set for the Errors dataset, but when users switch to the Spans dataset, this error-specific filter persists (as shown in the apdex test at line 731). The is:unresolved filter is meaningful for Errors but not for Spans. The dataset onChange handler in metric.tsx resets the aggregate and detection type when the dataset changes but does not clear or reset the query field. This results in submitting an irrelevant filter with Spans queries.
Additional Locations (1)
| const queryFromUrl = decodeScalar(location.query.query, '') ?? ''; | ||
| const queryFromUrl = decodeScalar(location.query.query); | ||
| const defaultQuery = DEFAULT_THRESHOLD_METRIC_FORM_DATA.query ?? ''; | ||
| const query = queryFromUrl === undefined ? defaultQuery : queryFromUrl; |
There was a problem hiding this comment.
Bug: Empty query URL parameter incorrectly gets default value
When a user explicitly provides ?query= in the URL (intending an empty query filter), decodeScalar('') returns undefined because the function treats empty strings as falsy. The new logic then checks queryFromUrl === undefined and replaces it with the default 'is:unresolved'. This differs from the old behavior where decodeScalar(location.query.query, '') would preserve the empty string. Users navigating with an explicit empty query parameter (e.g., from shared links or other app sections) would unexpectedly get a pre-filled filter instead of no filter.
When creating a new metric monitor, use "number of errors" as the default since it is the most commonly-used metric alert.