feat(collections-list): Add timeseries collection stats COMPASS-8179 #7584
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds timeseries collection statistics (bucket count and average bucket size) to the collections list UI. The stats are displayed in a tooltip when hovering over the timeseries badge.
Key Changes:
- Added
bucket_countandavg_bucket_sizefields to collection types and models - Implemented aggregation pipeline logic to calculate bucket statistics from MongoDB storage stats
- Created tooltip with formatted bucket statistics for timeseries collections
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/data-service/src/types.ts | Added optional bucket_count and avg_bucket_size fields to CollectionStats interface |
| packages/data-service/src/data-service.ts | Implemented aggregation pipeline stages to extract and calculate timeseries bucket statistics |
| packages/collection-model/lib/model.js | Added bucket_count and avg_bucket_size as number properties |
| packages/collection-model/index.d.ts | Added optional bucket_count and avg_bucket_size to CollectionProps interface |
| packages/databases-collections-list/src/collections.tsx | Updated timeseries badge to display tooltip with bucket stats; removed timeseries from view-only conditions for indexes and index size columns |
| packages/databases-collections-list/src/collections.spec.tsx | Updated test data and added test coverage for timeseries badge tooltip |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case 'timeseries': | ||
| return { id, name: id, variant: 'darkgray', icon: 'TimeSeries' }; | ||
| case 'timeseries': { | ||
| let hint: React.ReactNode = undefined; |
There was a problem hiding this comment.
[nitpick] Redundant initialization. In TypeScript/JavaScript, variables without explicit initialization are already undefined. Remove = undefined for cleaner code.
| let hint: React.ReactNode = undefined; | |
| let hint: React.ReactNode; |
| totalBucketSize: { | ||
| $first: { | ||
| $ifNull: [ | ||
| { | ||
| $multiply: [ | ||
| '$storageStats.timeseries.avgBucketSize', | ||
| '$storageStats.timeseries.bucketCount', | ||
| ], | ||
| }, | ||
| '$storageStats.totalBucketSize', | ||
| ], | ||
| }, | ||
| }, |
There was a problem hiding this comment.
This calculation multiplies avgBucketSize by bucketCount to derive totalBucketSize, but the logic and rationale are unclear. Add a comment explaining why this multiplication is necessary and when each field in the $ifNull array is expected to be available versus null.
There was a problem hiding this comment.
I don't think the logic needs to be explained, but given that we're clearly trying to support different setups (or server versions), a short note or link to docs could be useful. Especially if part of this is older sever versions, which is something we might be able to clean up once those versions are EOL.
There was a problem hiding this comment.
Do we already know what type of collection it is? This is run per collectionName so I'm wondering if we can just run different queries depending on the type of collection and if that won't be safer and easier to read.
(This is just me wondering, I don't know if that would necessarily be better.)
| avgBucketSize: { | ||
| $cond: { | ||
| if: { $ne: ['$avgBucketSizeFromStats', null] }, | ||
| then: '$avgBucketSizeFromStats', | ||
| else: { | ||
| $cond: { | ||
| if: { | ||
| $and: [ | ||
| { $ne: ['$numBuckets', null] }, | ||
| { $ne: ['$numBuckets', 0] }, | ||
| ], | ||
| }, | ||
| then: { | ||
| $divide: [ | ||
| { $toDouble: '$totalBucketSize' }, | ||
| { $toDouble: '$numBuckets' }, | ||
| ], | ||
| }, | ||
| else: null, | ||
| }, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
This nested conditional logic for calculating avgBucketSize has multiple fallback paths but lacks explanation. Add a comment describing the calculation strategy: when avgBucketSize is directly available from stats vs. when it needs to be calculated from totalBucketSize/numBuckets, and what MongoDB versions or configurations affect this.
| const result = inspectTable(screen, 'collections-list'); | ||
| const badge = result.trs[3].querySelector( |
There was a problem hiding this comment.
Using a hardcoded array index (result.trs[3]) makes this test brittle. If the order of collections changes or collections are added/removed, this test will break. Consider selecting the timeseries collection by name or filtering for the collection with timeseries properties instead.
| const result = inspectTable(screen, 'collections-list'); | |
| const badge = result.trs[3].querySelector( | |
| const timeseriesRow = screen.getByTestId('collections-list-row-timeseries'); | |
| const badge = timeseriesRow.querySelector( |
|
Assigned |
paula-stacho
left a comment
paula-stacho
left a comment
There was a problem hiding this comment.
Left a small suggestion, otherwise looks good!
Description
Add timeseries collection stats in a tooltip for the timeseries property badge.
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes