Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion packages/billing/lib/clients/orb/adapters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,21 @@ import { uuidv7 } from 'uuidv7';

import { Err, Ok } from '@nangohq/utils';

import { envs } from '../../envs.js';
import { putOrbCustomerSchema } from './types.js';

import type { BillingAddress, BillingCustomer, BillingEvent, BillingInvoicingDetails, Result, UsageMetric } from '@nangohq/types';
import type Orb from 'orb-billing';

// True once the S3-fed pipeline is authoritative for billing (see
// BILLING_EVENTS_CUTOVER_AT in packages/utils). While this returns false,
// HTTP-emitted events stay unsuffixed so customer billable metrics keep
// matching them. Once it flips true, HTTP events pick up the "_http"
// suffix and become the defensive shadow.
function cutoverActive(): boolean {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The cutoverActive() cutover-active check is duplicated across two packages (billing and metering). If the cutover comparison ever needs to change — logging, timezone handling, grace-period logic, or date format — both copies must be updated in sync. Consider extracting a shared helper into @nangohq/utils (or a billing-adjacent shared module) so the two packages share one source of truth.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/billing/lib/clients/orb/adapters.ts, line 16:

<comment>The `cutoverActive()` cutover-active check is duplicated across two packages (`billing` and `metering`). If the cutover comparison ever needs to change — logging, timezone handling, grace-period logic, or date format — both copies must be updated in sync. Consider extracting a shared helper into `@nangohq/utils` (or a billing-adjacent shared module) so the two packages share one source of truth.</comment>

<file context>
@@ -8,6 +8,15 @@ import { putOrbCustomerSchema } from './types.js';
+// HTTP-emitted events stay unsuffixed so customer billable metrics keep
+// matching them. Once it flips true, HTTP events pick up the "_http"
+// suffix and become the defensive shadow.
+function cutoverActive(): boolean {
+    return !!envs.BILLING_EVENTS_CUTOVER_AT && new Date() >= new Date(envs.BILLING_EVENTS_CUTOVER_AT);
+}
</file context>

return !!envs.BILLING_EVENTS_CUTOVER_AT && new Date() >= new Date(envs.BILLING_EVENTS_CUTOVER_AT);
}

export function toOrbEvent(event: BillingEvent): Orb.Events.EventIngestParams.Event {
const { idempotencyKey, timestamp, accountId, ...rest } = event.properties;

Expand All @@ -23,8 +33,9 @@ export function toOrbEvent(event: BillingEvent): Orb.Events.EventIngestParams.Ev
}
}

const suffix = cutoverActive() ? (envs.BILLING_EVENTS_HTTP_EVENT_NAME_SUFFIX ?? '') : '';
return {
event_name: event.type,

Copy link
Copy Markdown
Collaborator

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?

const http_suffix = today >= Aug1stMidnight ? 'http' : '';

// reversed for the s3 export suffix

Then you don't have anything to do, just be in front of your computer and confirm correct events are being injested

Copy link
Copy Markdown
Contributor Author

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

event_name: `${event.type}${suffix}`,
idempotency_key: idempotencyKey || uuidv7(),
external_customer_id: accountId.toString(),
timestamp: timestamp.toISOString(),
Expand Down
29 changes: 29 additions & 0 deletions packages/billing/lib/clients/orb/adapters.unit.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, expect, it, vi } from 'vitest';

import { envs } from '../../envs.js';
import { fromOrbAddress, fromOrbCustomer, orbMetricToUsageMetric, toOrbEvent, toOrbPutCustomerPayload } from './adapters.js';

import type { BillingEvent, BillingInvoicingDetails } from '@nangohq/types';
Expand Down Expand Up @@ -66,6 +67,34 @@ describe('toOrbEvent', () => {
expect(result.external_customer_id).toBe('42');
expect(result.timestamp).toBe('2024-01-15T10:00:00.000Z');
});

it('appends the HTTP suffix to event_name once the cutover has passed', () => {
const originalSuffix = envs.BILLING_EVENTS_HTTP_EVENT_NAME_SUFFIX;
const originalCutover = envs.BILLING_EVENTS_CUTOVER_AT;
try {
(envs as any).BILLING_EVENTS_HTTP_EVENT_NAME_SUFFIX = '_http';
(envs as any).BILLING_EVENTS_CUTOVER_AT = '2000-01-01T00:00:00Z';
const event: BillingEvent = { type: 'proxy', properties: { ...baseProperties } as any };
expect(toOrbEvent(event).event_name).toBe('proxy_http');
} finally {
(envs as any).BILLING_EVENTS_HTTP_EVENT_NAME_SUFFIX = originalSuffix;
(envs as any).BILLING_EVENTS_CUTOVER_AT = originalCutover;
}
});

it('does not append the HTTP suffix before the cutover instant', () => {
const originalSuffix = envs.BILLING_EVENTS_HTTP_EVENT_NAME_SUFFIX;
const originalCutover = envs.BILLING_EVENTS_CUTOVER_AT;
try {
(envs as any).BILLING_EVENTS_HTTP_EVENT_NAME_SUFFIX = '_http';
(envs as any).BILLING_EVENTS_CUTOVER_AT = '9999-01-01T00:00:00Z';
const event: BillingEvent = { type: 'proxy', properties: { ...baseProperties } as any };
expect(toOrbEvent(event).event_name).toBe('proxy');
} finally {
(envs as any).BILLING_EVENTS_HTTP_EVENT_NAME_SUFFIX = originalSuffix;
(envs as any).BILLING_EVENTS_CUTOVER_AT = originalCutover;
}
});
});

// ─── toOrbPutCustomerPayload ──────────────────────────────────────────────────
Expand Down
12 changes: 11 additions & 1 deletion packages/metering/lib/crons/billingEventsS3Export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,16 @@ 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 ?? '';

// True once the S3-fed pipeline is authoritative for billing (see
// BILLING_EVENTS_CUTOVER_AT in packages/utils). While this returns false,
// S3 events keep their pre-cutover suffix (e.g. "_shadow") so they don't
// collide with the HTTP-fed canonical events. Once it flips true, S3
// events land under the canonical (unsuffixed) name customer billable
// metrics filter on.
function cutoverActive(): boolean {
return !!envs.BILLING_EVENTS_CUTOVER_AT && new Date() >= new Date(envs.BILLING_EVENTS_CUTOVER_AT);
}

const LOCK_KEY = 'lock:cron:billingEventsS3Export';
// Cron fires hourly; lock should expire well before the next tick.
Expand Down Expand Up @@ -176,6 +185,7 @@ export async function exec(): Promise<void> {
return;
}
const day = yesterdayUTC();
const eventNameSuffix = cutoverActive() ? '' : (envs.BILLING_EVENTS_S3_EVENT_NAME_SUFFIX ?? '');
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
let anyFailure = false;
try {
for (const metric of METRICS) {
Expand Down
16 changes: 16 additions & 0 deletions packages/utils/lib/environment/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,23 @@ 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(),
// Temporary. Applied to S3-emitted Orb event_name only BEFORE the
// S3-fed cutover (i.e. while BILLING_EVENTS_CUTOVER_AT is unset or in
// the future). After the cutover, S3 events land unsuffixed under the
// canonical event names.
BILLING_EVENTS_S3_EVENT_NAME_SUFFIX: z.string().optional(),
// Temporary. Applied to HTTP-fed Orb event_name only AFTER the
// S3-fed cutover (i.e. once BILLING_EVENTS_CUTOVER_AT has passed) so
// HTTP emissions remain queryable as a defensive backup. Remove once
// the HTTP emission path is retired.
BILLING_EVENTS_HTTP_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 are
// shadow (suffixed) and HTTP events are canonical (unsuffixed). At
// and after this instant, the two swap roles. Unset (or set to a
// future date) to defer or roll back the cutover. Remove once the
// HTTP emission path is retired.

@TBonnin TBonnin Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure you need environment variables for the suffix. they are never gonna be dynamic and always be http | s3. right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, good point. Ill keep the S3 one bc was already there and there was some divergence btw dev an prod, but the HTTP one can be removed and hardcoded

BILLING_EVENTS_CUTOVER_AT: z.string().datetime().optional(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 z.string().datetime() only accepts Z timestamps by default. Allowing offsets keeps the env var aligned with the documented ISO 8601 format operators are likely to use.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/utils/lib/environment/parse.ts, line 368:

<comment>Valid ISO 8601 cutover timestamps with explicit offsets can make the service fail env validation, because `z.string().datetime()` only accepts `Z` timestamps by default. Allowing offsets keeps the env var aligned with the documented ISO 8601 format operators are likely to use.</comment>

<file context>
@@ -349,11 +349,23 @@ export const ENVS = z.object({
+    // and after this instant, the two swap roles. 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(),
     BILLING_EVENTS_S3_REGION: z.string().optional().default('us-west-2'),
 
</file context>
Suggested change
BILLING_EVENTS_CUTOVER_AT: z.string().datetime().optional(),
BILLING_EVENTS_CUTOVER_AT: z.string().datetime({ offset: true }).optional(),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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 - name: BILLING_EVENTS_CUTOVER_AT value: "2026-08-01T00:00:00Z"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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'),

// ClickHouse
Expand Down
Loading