Skip to content

Commit 9f7480d

Browse files
committed
fix(eventbroker): Use timestamp not changeTime for event metrics
Because: - We want to record event latency in event-broker - And we previously using changeTime, which is not correct This Commit: - Uses timestamp on the message event to correctly record the event lattency Closes: FXA-13395
1 parent 64c32d0 commit 9f7480d

2 files changed

Lines changed: 51 additions & 4 deletions

File tree

packages/fxa-event-broker/src/pubsub-proxy/pubsub-proxy.controller.spec.ts

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,17 @@ jest.mock('@sentry/node', () => ({
2020
const TEST_TOKEN =
2121
'eyJhbGciOiJSUzI1NiIsImtpZCI6IjdkNjgwZDhjNzBkNDRlOTQ3MTMzY2JkNDk5ZWJjMWE2MWMzZDVhYmMiLCJ0eXAiOiJKV1QifQ.eyJhdWQiOiJodHRwczovL2V4YW1wbGUuY29tIiwiYXpwIjoiMTEzNzc0MjY0NDYzMDM4MzIxOTY0IiwiZW1haWwiOiJnYWUtZ2NwQGFwcHNwb3QuZ3NlcnZpY2VhY2NvdW50LmNvbSIsImVtYWlsX3ZlcmlmaWVkIjp0cnVlLCJleHAiOjE1NTAxODU5MzUsImlhdCI6MTU1MDE4MjMzNSwiaXNzIjoiaHR0cHM6Ly9hY2NvdW50cy5nb29nbGUuY29tIiwic3ViIjoiMTEzNzc0MjY0NDYzMDM4MzIxOTY0In0.QVjyqpmadTyDZmlX2u3jWd1kJ68YkdwsRZDo-QxSPbxjug4ucLBwAs2QePrcgZ6hhkvdc4UHY4YF3fz9g7XHULNVIzX5xh02qXEH8dK6PgGndIWcZQzjSYfgO-q-R2oo2hNM5HBBsQN4ARtGK_acG-NGGWM3CQfahbEjZPAJe_B8M7HfIu_G5jOLZCw2EUcGo8BvEwGcLWB2WqEgRM0-xt5-UPzoa3-FpSPG7DHk7z9zRUeq6eB__ldb-2o4RciJmjVwHgnYqn3VvlX9oVKEgXpNFhKuYA-mWh5o7BCwhujSMmFoBOh6mbIXFcyf5UiVqKjpqEbqPGo_AvKvIQ9VTQ';
2222
const TEST_CLIENT_ID = 'abc1234';
23-
const CHANGE_TIME = Date.now();
23+
// Fixed epoch so assertions are deterministic
24+
const FIXED_NOW = 1743657600000;
2425

2526
const createValidSubscriptionMessage = (): string => {
2627
return Buffer.from(
2728
JSON.stringify({
2829
capabilities: ['cap1', 'cap2'],
29-
changeTime: Math.trunc(Date.now() / 1000),
30+
changeTime: Date.now(),
3031
event: dto.SUBSCRIPTION_UPDATE_EVENT,
3132
isActive: true,
33+
timestamp: Date.now(),
3234
uid: 'uid1234',
3335
})
3436
).toString('base64');
@@ -38,6 +40,7 @@ const createValidUpdateMessage = (): string => {
3840
return Buffer.from(
3941
JSON.stringify({
4042
event: dto.SUBSCRIPTION_UPDATE_EVENT,
43+
timestamp: Date.now(),
4144
uid: 'uid1234',
4245
})
4346
).toString('base64');
@@ -47,6 +50,7 @@ const createValidDeleteMessage = (): string => {
4750
return Buffer.from(
4851
JSON.stringify({
4952
event: dto.DELETE_EVENT,
53+
timestamp: Date.now(),
5054
uid: 'uid1234',
5155
})
5256
).toString('base64');
@@ -56,6 +60,7 @@ const createValidProfileMessage = (): string => {
5660
return Buffer.from(
5761
JSON.stringify({
5862
event: dto.PROFILE_CHANGE_EVENT,
63+
timestamp: Date.now(),
5964
uid: 'uid1234',
6065
locale: 'en-us',
6166
totpEnabled: false,
@@ -69,8 +74,9 @@ const createValidProfileMessage = (): string => {
6974
const createValidPasswordMessage = (): string => {
7075
return Buffer.from(
7176
JSON.stringify({
72-
changeTime: CHANGE_TIME,
77+
changeTime: FIXED_NOW,
7378
event: dto.PASSWORD_CHANGE_EVENT,
79+
timestamp: Date.now(),
7480
uid: 'uid1234',
7581
})
7682
).toString('base64');
@@ -93,6 +99,10 @@ describe('PubsubProxy Controller', () => {
9399
};
94100

95101
beforeEach(async () => {
102+
// we can't use fakeTimers here because it causes issues with
103+
// axios/nock, but for other tests this gets us what we need
104+
jest.spyOn(Date, 'now').mockReturnValue(FIXED_NOW);
105+
96106
jwtset = {
97107
generateDeleteSET: jest.fn().mockResolvedValue(TEST_TOKEN),
98108
generatePasswordSET: jest.fn().mockResolvedValue(TEST_TOKEN),
@@ -146,6 +156,10 @@ describe('PubsubProxy Controller', () => {
146156
controller = module.get<PubsubProxyController>(PubsubProxyController);
147157
});
148158

159+
afterEach(() => {
160+
jest.restoreAllMocks();
161+
});
162+
149163
it('should be defined', () => {
150164
expect(controller).toBeDefined();
151165
});
@@ -225,6 +239,39 @@ describe('PubsubProxy Controller', () => {
225239
}
226240
});
227241

242+
it('records proxy.success timing using message.timestamp, not changeTime', async () => {
243+
// Simulate a password event where changeTime is years-old (the credential
244+
// generation timestamp) but timestamp reflects when the event was queued.
245+
const MESSAGE_TIMESTAMP = FIXED_NOW - 150;
246+
const STALE_CHANGE_TIME = FIXED_NOW - 7 * 365 * 24 * 60 * 60 * 1000;
247+
248+
const message = Buffer.from(
249+
JSON.stringify({
250+
changeTime: STALE_CHANGE_TIME,
251+
event: dto.PASSWORD_CHANGE_EVENT,
252+
timestamp: MESSAGE_TIMESTAMP,
253+
uid: 'uid1234',
254+
})
255+
).toString('base64');
256+
257+
mockWebhook();
258+
try {
259+
await controller.proxy(
260+
{
261+
message: { data: message, messageId: 'test-message' },
262+
subscription: 'test-sub',
263+
},
264+
TEST_CLIENT_ID
265+
);
266+
} catch (_) {}
267+
268+
expect(mockMetricValue.timing).toHaveBeenCalledWith('proxy.success', 150, {
269+
clientId: TEST_CLIENT_ID,
270+
statusCode: '200',
271+
type: dto.PASSWORD_CHANGE_EVENT,
272+
});
273+
});
274+
228275
it('logs an error on invalid message payloads', async () => {
229276
const message = Buffer.from('invalid payload').toString('base64');
230277
expect.assertions(3);

packages/fxa-event-broker/src/pubsub-proxy/pubsub-proxy.controller.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ export class PubsubProxyController {
116116
try {
117117
const response = await axios(requestOptions);
118118
const now = Date.now();
119-
this.metrics.timing('proxy.success', now - message.changeTime, {
119+
this.metrics.timing('proxy.success', now - message.timestamp, {
120120
clientId,
121121
statusCode: response.status.toString(),
122122
type: message.event,

0 commit comments

Comments
 (0)