fix(tracemetrics): Improve metrics analytics

[k-fish]

This elides metric counts when the metric name is empty, and improves testing around metric panel etc.

[cursor]

Bug: Aggregate mode reports wrong table result length

The getAttributes function always uses resultLengthBox.current (samples table length) for table_result_length, regardless of whether resultMode is 'aggregates' or 'metric samples'. When in aggregate mode, it should use aggregatesResultLengthBox.current instead. This causes incorrect analytics data to be reported for the aggregates table result length.

 

[cursor]

Bug: Aggregate mode reports wrong table result sort

The getAttributes function always uses formattedSortBysBox.current (samples sort) for table_result_sort, regardless of whether resultMode is 'aggregates' or 'metric samples'. When in aggregate mode, it should use formattedAggregateSortBysBox.current instead. The formattedAggregateSortBysBox is defined and included in the aggregates useEffect dependency array but is never actually used in the analytics payload.

 

[sentry]

Bug: metricPanelsWithGroupBys and metricPanelsWithFilters incorrectly count all non-empty metrics due to .map().length instead of filtering for specific conditions.
_{Severity: MEDIUM | Confidence: High}

🔍 Detailed Analysis

The metricPanelsWithGroupBys and metricPanelsWithFilters variables are intended to count the number of metricQueries that satisfy specific conditions (having group-bys or filters) after filtering out empty trace metrics. However, the current implementation uses .map() to transform the filtered metricQueries into an array of booleans, and then .length is called on this array. This incorrectly counts the total number of non-empty metricQueries (the length of the boolean array) instead of counting only those metricQueries for which the boolean condition was true. For example, if there are two non-empty metrics but only one has group-bys, the code will return 2 instead of 1, leading to an overcounting of these panel types.

💡 Suggested Fix

Replace the .map().length pattern with a single .filter() call that combines the initial non-empty metric check with the specific condition. For example: metricQueries.filter(mq => !isEmptyTraceMetric(mq.metric) && mq.queryParams.groupBys.some((gb: string) => gb.trim().length > 0)).length;

🤖 Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: static/app/views/explore/hooks/useAnalytics.tsx#L912-L917

Potential issue: The `metricPanelsWithGroupBys` and `metricPanelsWithFilters` variables
are intended to count the number of `metricQueries` that satisfy specific conditions
(having group-bys or filters) after filtering out empty trace metrics. However, the
current implementation uses `.map()` to transform the filtered `metricQueries` into an
array of booleans, and then `.length` is called on this array. This incorrectly counts
the total number of non-empty `metricQueries` (the length of the boolean array) instead
of counting only those `metricQueries` for which the boolean condition was `true`. For
example, if there are two non-empty metrics but only one has group-bys, the code will
return `2` instead of `1`, leading to an overcounting of these panel types.

_{Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 6298179}

[cursor]

Bug: Info log uses total count, analytics uses filtered count

The trackAnalytics call uses nonEmptyMetricQueries.length for metric_queries_count, but the info log uses metricQueries.length. This inconsistency means the analytics event and the info log report different values for the same metric. Based on the PR's intent to elide empty metrics, the info call likely needs to use nonEmptyMetricQueries.length as well.

Additional Locations (1)

• static/app/views/explore/hooks/useAnalytics.tsx#L941-L942