refactor: wire backfill pipeline and remove per-location Check documents#3313
refactor: wire backfill pipeline and remove per-location Check documents#3313gorkem-bwl wants to merge 1 commit intofeat/globalping-check-servicesfrom
Conversation
227141c to
0671981
Compare
4e7e341 to
325537e
Compare
9052c4f to
de5adf5
Compare
325537e to
3b858de
Compare
Br0wnHammer
left a comment
There was a problem hiding this comment.
Please see my comments. @gorkem-bwl
| $match: { | ||
| "metadata.monitorId": monitorObjectId, | ||
| createdAt: { $gte: startDate, $lte: endDate }, | ||
| locationResults: { $exists: true, $not: { $size: 0 } }, |
There was a problem hiding this comment.
Nit: The $exists: true + $not: { $size: 0 } combo can behave unexpectedly if locationResults is null (it exists but isn't an array, so $size won't work as intended). Safer to use:
locationResults: { $type: "array", $ne: [] }
This guarantees we're working with a non-empty array in all cases.
| locationResults: { $exists: true, $not: { $size: 0 } }, | ||
| }, | ||
| }, | ||
| { $unwind: "$locationResults" }, |
There was a problem hiding this comment.
Perf: Since we're now doing $unwind on locationResults, it's important that the $match stage narrows the dataset as much as possible before that expansion happens.
| const locCheck = this.checkService.buildCheck(locStatus); | ||
| if (locCheck) { | ||
| this.buffer.addToBuffer(locCheck); | ||
| .then((locationResults) => { |
There was a problem hiding this comment.
The dual write path here (buffer mutation vs DB update) depends on whether the buffer has flushed by the time runChecks resolves. Can we add a comment explaining this is intentionally fire-and-forget? Also could a flush mid-callback cause a lost write?
| const locCheck = this.checkService.buildCheck(locStatus); | ||
| if (locCheck) { | ||
| this.buffer.addToBuffer(locCheck); | ||
| .then((locationResults) => { |
There was a problem hiding this comment.
This .then() chain doesn't have a .catch(). If runChecks or updateLocationResults rejects, the error gets silently swallowed. Since this is fire-and-forget, we won't notice failures unless we explicitly log them.
## Changes - Remove location field from CheckMetadata, MonitorStatusResponse, Check model metadata schema, and location index - Remove location from checkService.buildCheck() and MongoChecksRepository toDocument/mapMetadata - Rewrite SuperSimpleQueueHelper Step 2b: instead of creating separate Check documents per Globalping location, backfill locationResults into the same Check document after the local check is buffered - Rewrite findLocationChecksByMonitorId to use $unwind on locationResults subdocument array instead of querying by metadata.location - Remove metadata.location filter from findUptimeDateRangeChecks - Pass locationResults through in toEntity ## Benefits Single Check document per monitoring cycle now contains both local results (top-level fields) and per-location Globalping results (locationResults array). This eliminates document proliferation and simplifies queries.
de5adf5 to
a23e4de
Compare
Summary
locationResultsinto the same Check document as the local checklocationfrom CheckMetadata, MonitorStatusResponse, metadata schema, location index, buildCheck(), toEntity(), and toDocument()GlobalpingCheckResult[](simplified shape without monitorId/teamId/type)findLocationChecksByMonitorIdto$unwindlocationResults instead of querying bymetadata.locationmetadata.locationfilter fromfindUptimeDateRangeChecksTest plan
npm run buildin/serverpasses with zero errorslocationResultsfieldlocationResultsarray gets backfilled after 5-15slocationResultsvia$unwindupdateOne)locationResultshas no effect on status pipeline