feat(preprod): add preprod dashboard frontend#105919
Conversation
Add EAP dataset support for mobile app size metrics: - PreprodSize class with automatic sub_item_type filtering - Aggregate definitions (max) for size metrics - Attribute definitions for app_id, app_name, build_version, etc. - Referrer for preprod size queries - Test case base class for preprod size metrics testing - Backend tests for events-stats endpoint
Add frontend support for mobile app size metrics dashboard: - MobileAppSizeConfig dataset configuration - MobileAppSizeFilters component for widget builder - Dataset selector with Mobile Builds option (alphabetically ordered) - Integration with EAP trace item search query builder - Support for max(max_install_size) and max(max_download_size) aggregates - Widget builder slideout modifications for preprodSize dataset - Tests for MobileAppSizeConfig
…ze-dashboard-frontend
…ze-dashboard-frontend
trevor-e
changed the base branch from
master
to
telkins/app-size-dashboard-backend
github-actions
bot
added
Scope: Frontend
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
static/app/views/dashboards/widgetBuilder/components/groupBySelector.tsx
Outdated
Show resolved
Hide resolved
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
static/app/views/dashboards/widgetBuilder/components/newWidgetBuilder.tsx
Show resolved
Hide resolved
| if (itemType === TraceItemDataset.SPANS) { | ||
| return SENTRY_SPAN_STRING_TAGS; | ||
| } | ||
| if (itemType === TraceItemDataset.PREPROD) { |
There was a problem hiding this comment.
fwiw all of these implicit configs are not super discoverable, it would be nice to have an exhaustive enum that forces us to handle every case.
gggritso
left a comment
gggritso
left a comment
There was a problem hiding this comment.
Cool, LGTM! I left a few comments, but it's mostly me mulling the differences between this dataset and the others. Besides, the differences are isolated to the dataset config (the system works!) I think this is good to go. Thanks for bearing with us on this 👍🏻
|
|
||
| if (isEventsStats(data)) { | ||
| const seriesData = data.data | ||
| .filter( |
There was a problem hiding this comment.
IMO time suddenly becoming non-linear is a worse expectation break than people seeing "0" on the chart, but this is going to be moot soon anyway, since the updated charts are in EA right now and will hopefully GA soon.
At that point, we'll need to remove this logic completely and we can use the helpers. I'm going to file a ticket to take a look at that later.
Also worth mentioning that this kind of expectation break of X axis gaps may be harmful in other places. We have code that infers the size of the time series buckets from the first few points, for example, which might break, and so on.
| // Only add numeric size fields for use in aggregate functions | ||
| // String fields like app_id, app_name, build_version are only used | ||
| // for filtering and will be available via the search bar | ||
| const mobileAppSizeFields: Record<string, FieldValueOption> = { | ||
| 'field:install_size': { | ||
| label: 'Install Size', | ||
| value: { | ||
| kind: FieldValueKind.TAG, | ||
| meta: {name: 'install_size', dataType: 'number'}, | ||
| }, | ||
| }, | ||
| 'field:download_size': { | ||
| label: 'Download Size', | ||
| value: { | ||
| kind: FieldValueKind.TAG, | ||
| meta: {name: 'download_size', dataType: 'number'}, | ||
| }, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
I think that's okay but for what it's worth I think that only makes sense because you're artificially limiting which aggregations are available. In other cases we show all the options and grey out whichever are not supported by the current aggregate, which is nicer because if someone changes the aggregate the fields might become available. Worth considering this, since I think you could enable the other aggregates! People can decide for themselves what they want to aggregate, no?
# Conflicts: # static/app/views/explore/constants.tsx # static/app/views/explore/contexts/traceItemAttributeContext.tsx
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| state.dataset === WidgetType.LOGS || | ||
| state.dataset === WidgetType.TRACEMETRICS | ||
| state.dataset === WidgetType.TRACEMETRICS || | ||
| state.dataset === WidgetType.PREPROD_APP_SIZE |
There was a problem hiding this comment.
Missing PREPROD_APP_SIZE in EAP type check
Low Severity
The isEAPType check in buildSteps/groupByStep/groupBySelector.tsx includes WidgetType.SPANS, WidgetType.LOGS, and WidgetType.TRACEMETRICS, but not the newly added WidgetType.PREPROD_APP_SIZE. This causes the TypeBadge component (which renders attribute type labels like "string"/"number") to not appear for PREPROD_APP_SIZE fields in the group by selector, creating an inconsistent UI compared to other EAP-based widget types.
3 participants
Adds support for a new
Mobile Buildsdataset within the custom widget builder UI. I created a new flag to hide this functionality for now.