Skip to content

Conversation

jasonyuezhang
Copy link
Owner

Follow-up to getsentry#100532, with a few small fixes:

  • Always assign isOther, if it's a groupBy
  • Use groupData for order rather than the event data
  • Mark delayed data

Supports ENG-5608


Copied from getsentry#100918
Original PR: getsentry#100918

Copy link

Fix and Improve Grouped TimeSeries Transformation

This PR refines the conversion logic for transforming grouped event stats into TimeSeries objects inside useSortedTimeSeries.tsx. The changes ensure that the isOther property for time series metadata is always correctly set for groupBy scenarios, ensure the time series order is sourced consistently from groupData.order, and apply a delayed data marker using markDelayedData. These updates are follow-ups to recent changes that introduced this transformation logic, offering improved accuracy and reliability when reporting or visualizing grouped timeseries data.

Key Changes

• Added import and invocation of markDelayedData to annotate delayed time series data.
• Always assign timeSeries.meta.isOther for groupBy results, instead of only when group name is 'Other'.
• Switched order determination from seriesData.order to groupData.order to correctly source group order.
• Adjusted construction and return value of convertEventsStatsToTimeSeriesData to handle delayed series and meta order assignments robustly.
• Update to TypeScript type imports for TimeSeriesMeta.

Affected Areas

static/app/views/insights/common/queries/useSortedTimeSeries.tsx
• Time series conversion logic when handling grouped and multi-axis event stats

This summary was automatically generated by @propel-code-bot

}

return [serie.meta.order ?? 0, serie];
return [timeSeries.meta.order ?? 0, delayedTimeSeries];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CriticalError]

Potential bug: The return value references timeSeries.meta.order but order is only assigned to delayedTimeSeries.meta.order. This will always return 0 when order is provided, potentially causing incorrect sorting.

Suggested change
return [timeSeries.meta.order ?? 0, delayedTimeSeries];
return [delayedTimeSeries.meta.order ?? 0, delayedTimeSeries];

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 bug: The return value references `timeSeries.meta.order` but `order` is only assigned to `delayedTimeSeries.meta.order`. This will always return 0 when order is provided, potentially causing incorrect sorting.

```suggestion
  return [delayedTimeSeries.meta.order ?? 0, delayedTimeSeries];
```

⚡ **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/insights/common/queries/useSortedTimeSeries.tsx
Line: 295

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants