Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a centralized Cloud Tasks manager for metering threshold-check enqueues, including supporting utilities for task-id deduplication and enqueue error classification, and wires it into the payments API module.
Changes:
- Added
MeteringThresholdTasksManagerto enqueue threshold-check tasks with name-based deduplication. - Added utilities for Cloud Tasks config validation, deterministic task-id building, and enqueue error classification (with unit tests).
- Registered a
CloudTasksClientprovider and the new manager inpayments-apiDI.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/entitlements/metering/src/lib/utils/requireMeteringCloudTasksConfig.ts | New helper to fail fast when required Cloud Tasks config is missing. |
| libs/entitlements/metering/src/lib/utils/requireMeteringCloudTasksConfig.spec.ts | Unit tests for Cloud Tasks config requirement behavior. |
| libs/entitlements/metering/src/lib/utils/classifyEnqueueError.ts | New helper to bucket Cloud Tasks enqueue errors into bounded “reason” tags. |
| libs/entitlements/metering/src/lib/utils/classifyEnqueueError.spec.ts | Unit tests for enqueue error classification. |
| libs/entitlements/metering/src/lib/utils/buildThresholdTaskId.ts | New helper to build deterministic, bucketed task ids for deduplication. |
| libs/entitlements/metering/src/lib/utils/buildThresholdTaskId.spec.ts | Unit tests for task id stability, bucketing, and sanitization. |
| libs/entitlements/metering/src/lib/metering-threshold-tasks.manager.ts | New NestJS manager that enqueues threshold-check tasks and reports metrics. |
| libs/entitlements/metering/src/lib/metering-threshold-tasks.manager.spec.ts | Unit tests verifying enqueue payload, scheduling, dedup handling, and metrics. |
| libs/entitlements/metering/src/index.ts | Exports the new manager and utilities from the package entrypoint. |
| apps/payments/api/src/app/app.module.ts | Registers a CloudTasksClient factory and adds MeteringThresholdTasksManager to providers. |
Comments suppressed due to low confidence (1)
libs/entitlements/metering/src/lib/utils/buildThresholdTaskId.ts:30
buildThresholdTaskIdembeds a sanitizeduserIdentifierdirectly into the Cloud Tasks task name. The accompanying unit test explicitly uses an email address (user@example.com), which means task names (and any logs/metrics containing them) will include PII. For dedup you only need a stable identifier, so consider hashing theuserIdentifier(and optionallyslug) into a fixed-length, non-reversible string (e.g., sha256 hex) before building the task id to avoid PII exposure and also cap the task-id length.
const bucket = Math.floor(now.getTime() / bucketSizeMs);
const slug = sanitizeForTaskId(payload.slug);
const user = sanitizeForTaskId(payload.userIdentifier);
return `threshold-${slug}-${user}-${bucket}`;
}
function sanitizeForTaskId(input: string): string {
return input.replace(/[^A-Za-z0-9-_]/g, '_');
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+7
to
+22
| export function requireMeteringCloudTasksConfig( | ||
| meteringConfig: MeteringConfig | ||
| ): MeteringCloudTasksConfig { | ||
| if (!meteringConfig.cloudTasks) { | ||
| throw new Error( | ||
| 'MeteringConfig.cloudTasks is required for threshold task enqueue' | ||
| ); | ||
| } | ||
| if (!meteringConfig.cloudTasks.threshold.queueName) { | ||
| throw new Error( | ||
| 'MeteringConfig.cloudTasks.threshold.queueName is required' | ||
| ); | ||
| } | ||
| if (!meteringConfig.cloudTasks.threshold.taskUrl) { | ||
| throw new Error('MeteringConfig.cloudTasks.threshold.taskUrl is required'); | ||
| } |
Comment on lines
137
to
148
| MeteringConfig, | ||
| MeteringAuthGuard, | ||
| { | ||
| provide: CloudTasksClient, | ||
| useFactory: (meteringConfig: MeteringConfig) => | ||
| CloudTaskClientFactory({ | ||
| cloudTasks: requireMeteringCloudTasksConfig(meteringConfig), | ||
| }), | ||
| inject: [MeteringConfig], | ||
| }, | ||
| MeteringThresholdTasksManager, | ||
| MeteringWebhookManager, |
julianpoy
force-pushed
the
ENT-13
branch
2 times, most recently
from
fb04be7 to
d7d81a6
Compare
Because: - We want to have a centralized place to work with cloud tasks for thresholds when metering This commit: - Adds MeteringThresholdTasksManager Closes ENT-13
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
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.
Because
This pull request
Issue that this pull request solves
Closes ENT-13