-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(billing): time-based cutover for HTTP↔S3 event-name suffix swap #6705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
21abfe7
93d90b1
290b323
8f7c551
9140129
a1cc334
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,15 @@ const cronMinute = envs.CRON_BILLING_EVENTS_S3_HOURLY_EXPORT_MINUTE; | |
| const bucket = envs.BILLING_EVENTS_S3_BUCKET; | ||
| const roleArn = envs.BILLING_EVENTS_S3_WRITER_ROLE_ARN; | ||
| const region = envs.BILLING_EVENTS_S3_REGION; | ||
| const eventNameSuffix = envs.BILLING_EVENTS_S3_EVENT_NAME_SUFFIX ?? ''; | ||
|
|
||
| // Keyed on the DATA DAY, not the wall clock. Matters at the seam: the | ||
| // Aug 1 00:15 UTC firing exports July 31 data, and July 31 is still | ||
| // pre-cutover — so those rows must ship as "_s3" shadow, not canonical. | ||
| // See BILLING_EVENTS_CUTOVER_AT in packages/utils. | ||
| function dayIsPostCutover(day: string): boolean { | ||
| if (!envs.BILLING_EVENTS_CUTOVER_AT) return false; | ||
| return new Date(`${day}T00:00:00Z`) >= new Date(envs.BILLING_EVENTS_CUTOVER_AT); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: S3 events can be shadow-suffixed even when their emitted Orb Prompt for AI agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ack — this is a real edge case if CUTOVER_AT is ever set to a non-midnight instant, and in that case using day-end (as suggested) would prevent losing a day of customer billing on the Example: if CUTOVER_AT = 2026-08-01T14:00:00Z, day 2026-08-01 has day-start = 2026-08-01T00:00:00Z < 14:00Z → current check says "pre-cutover" → file ships as records_s3. Customer plans For our config (CUTOVER_AT = 2026-08-01T00:00:00Z exactly midnight, both dev and prod) day-start and day-end give the same answer for every day, so this doesn't manifest. The whole
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The edge case is real for non-midnight cutovers, but it doesn’t apply here because CUTOVER_AT is midnight in both dev and prod. Since this cutover mechanism is being removed next month, the parent comment is too broad for this PR. |
||
| } | ||
|
|
||
| const LOCK_KEY = 'lock:cron:billingEventsS3Export'; | ||
| // Cron fires hourly; lock should expire well before the next tick. | ||
|
|
@@ -214,13 +222,12 @@ export function billingEventsS3ExportCron(): void { | |
| }); | ||
| } | ||
|
|
||
| function addEventNameSuffix(eventName: MetricSpec['canonicalEventName']): string { | ||
| if (eventName == 'data_transfer') { | ||
| function addEventNameSuffix(eventName: MetricSpec['canonicalEventName'], day: string): string { | ||
| if (eventName === 'data_transfer') { | ||
| // data_transfer is a new event and thus doesn't need the suffix to support live traffic cutoff. | ||
| return eventName; | ||
| } | ||
|
|
||
| return `${eventName}${eventNameSuffix}`; | ||
| return `${eventName}${dayIsPostCutover(day) ? '' : '_s3'}`; | ||
| } | ||
|
|
||
| export async function exec(): Promise<void> { | ||
|
|
@@ -236,7 +243,7 @@ export async function exec(): Promise<void> { | |
| let anyFailure = false; | ||
| try { | ||
| for (const metric of METRICS) { | ||
| const eventName = addEventNameSuffix(metric.canonicalEventName); | ||
| const eventName = addEventNameSuffix(metric.canonicalEventName, day); | ||
| const key = objectKey({ day, eventName }); | ||
| const start = process.hrtime.bigint(); | ||
| // Tracks which step is in flight so a catch can tag the failure | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -352,7 +352,14 @@ export const ENVS = z.object({ | |||||
| BILLING_INGEST_MAX_RETRY: z.coerce.number().optional().default(3), | ||||||
| BILLING_EVENTS_S3_BUCKET: z.string().optional(), | ||||||
| BILLING_EVENTS_S3_WRITER_ROLE_ARN: z.string().optional(), | ||||||
| BILLING_EVENTS_S3_EVENT_NAME_SUFFIX: z.string().optional(), | ||||||
| // Temporary. ISO 8601 timestamp at which the S3-fed pipeline becomes | ||||||
| // authoritative for billing. Before this instant, S3 events ship as | ||||||
| // "<name>_s3" shadow and HTTP events ship canonical (unsuffixed). At | ||||||
| // and after this instant, the two swap roles — HTTP events pick up | ||||||
| // the "_http" suffix and S3 events become canonical. Unset (or set | ||||||
| // to a future date) to defer or roll back the cutover. Remove once | ||||||
| // the HTTP emission path is retired. | ||||||
| BILLING_EVENTS_CUTOVER_AT: z.string().datetime().optional(), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Valid ISO 8601 cutover timestamps with explicit offsets can make the service fail env validation, because Prompt for AI agents
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have this config which if Im not mistaken is a correct value
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That value looks valid as a Z timestamp. The parent comment was about explicit-offset ISO 8601 values, so it still applies to that case. |
||||||
| BILLING_EVENTS_S3_REGION: z.string().optional().default('us-west-2'), | ||||||
| // DLQ bucket Orb writes to when it can't ingest a billing event. Watched by the | ||||||
| // metering DLQ monitor cron (CRON_BILLING_EVENTS_S3_DLQ_MONITOR_MINUTE). | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we make the cut-over time-based?
Then you don't have anything to do, just be in front of your computer and confirm correct events are being injested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good call, but still I might want to have a flag for disabling it just in case .... let me work on this