fix(record count): do not count records from deleted connections #4919
+180
−25
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Record counts table entries are not deleted when a connection is soft deleted (only when hard deleted) which means that records are still counted until the connection is hard deleted. Since we delete connections from different places in the codebase it would require a big refactoring to delete the record counts table when connections is soft deleted. Main pain point is connections are soft-deleted when an integration is soft-deleted, deep inside the shared package, which hasn't any knowledge of the records packages :( Instead I decided to account for the deleted connections when we report the record counts. The logic was already quite cumbersome and wasn't supporting pagination.
TBonnin
commented
Oct 27, 2025
| case 'function_logs': { | ||
| const billingUsage = await this.getBillingUsage(accountId); | ||
| if (billingUsage.isErr()) { | ||
| logger.warning(`Failed to fetch billing usage for accountId: ${accountId}`, billingUsage.error); |
There was a problem hiding this comment.
unrelated. I just noticed we were logging the error twice. Here and in the catch
kaposke
approved these changes
Oct 27, 2025
| throw res.error; | ||
| } | ||
| for (const value of res.value) { | ||
| const recordCount = recordCounts.value.find((r) => r.connection_id === value.connection.id); |
There was a problem hiding this comment.
Potentially avoidable o(n^2), but probably negligible compared to queries?
There was a problem hiding this comment.
This entire analytics logic only works because the scale is small. We are gonna need to come up with a better solution as our user base of customers saving records expands
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Record counts table entries are not deleted when a connection is soft deleted (only when hard deleted) which means that records are still counted until the connection is hard deleted.
Since we delete connections from different places in the codebase it would require a big refactoring to delete the record counts table when connections is soft deleted. Main pain point is connections are soft-deleted when an integration is soft-deleted, deep inside the shared package, which hasn't any knowledge of the records packages :(
Instead I decided to account for the deleted connections when we report the record counts. The logic was already quite cumbersome and wasn't supporting pagination.
Exclude soft-deleted connections from record metrics + add pagination helpers
Soft-deleted connections were still represented in the
record_countstable, so the "Records" usage metric billed/observed more data than actually active. This PR avoids modifying the underlying table and instead filters record-count rows at read-time. It introduces paginated accessors in both the Records and Connections layers and rewrites the metering cron to join these two paginated streams, aggregating only active (non-deleted) connections. Additional integration tests ensure the new iterators and counting logic behave as expected.Key Changes
• Added
records.paginateRecordCounts()– offset-based async generator that yields batches fromRECORD_COUNTS_TABLE.• Added
connectionService.paginateConnections()– cursor-based async generator returning active connections with related account/env rows; supports optionalconnectionIdsfilter.• Re-implemented
observability.exportRecordsMetrics()inpackages/metering/lib/crons/usage.tsto: a) page through record counts, b) fetch matching connections, c) skip deleted connections, and d) aggregate metrics per account.• Updated unit/integration tests for Records and Connection services to cover new pagination helpers and counting logic.
• Minor cleanup: removed unused
environmentServiceimport, dropped redundant warning log in usage tracker.Affected Areas
•
packages/metering/lib/crons/usage.ts•
packages/shared/lib/services/connection.service.ts•
packages/records/lib/models/records.ts• Associated integration test suites
•
packages/account-usage/lib/usage.ts(small log change)This summary was automatically generated by @propel-code-bot