Skip to content

Commit 620cd4d

Browse files
vinistevamtuna1207chaitanyapotti
authored
feat: Report coverage latency in shield metrics (#7133)
## Explanation This PR adds latency reporting to shield coverage and signature coverage results. The latency (in milliseconds) is included as part of the `metrics` property in each coverage response, allowing downstream consumers and analytics systems to track and analyze the performance of coverage checks. Unit tests have been updated to verify correct latency values in the metrics. ### Changes - Add `metrics.latency` to coverage and signature coverage responses. - Update backend and test logic to use `Date.now()` for latency measurements. - Extend unit test coverage to validate latency reporting in result metrics. <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> Fixes MetaMask/MetaMask-planning#6136 ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [x] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Add `metrics.latency` to coverage and signature coverage responses, measuring end-to-end latency (including retries) and update tests and changelog accordingly. > > - **Backend**: > - Add `metrics.latency` to `GetCoverageResultResponse` and `CoverageResult` returned by `ShieldRemoteBackend`. > - Measure end-to-end latency in `#getCoverageResult` using `Date.now()`, including retries/delays via polling policy. > - **Types**: > - Extend `CoverageResult` and `GetCoverageResultResponse` with `metrics.latency` in `src/types.ts` and `src/backend.ts`. > - **Tests**: > - Update `src/backend.test.ts` to assert presence and correctness of `metrics.latency` for both transaction and signature coverage, including retry-delay scenarios. > - **Changelog**: > - Document addition of latency metrics in `packages/shield-controller/CHANGELOG.md`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit fc9d432. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Tuna <tunguyenanh@tunatech.org> Co-authored-by: Chaitanya Potti <chaitanya.potti@gmail.com>
1 parent ed5fc9a commit 620cd4d

File tree

4 files changed

+141
-7
lines changed

4 files changed

+141
-7
lines changed

packages/shield-controller/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Added
11+
12+
- Added metrics in the Shield coverage response to track the latency ( [#7133](https://github.com/MetaMask/core/pull/7133))
13+
1014
## [2.0.0]
1115

1216
### Changed

packages/shield-controller/src/backend.test.ts

Lines changed: 110 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ function setup({
5151
}
5252

5353
describe('ShieldRemoteBackend', () => {
54+
beforeEach(() => {
55+
jest.clearAllMocks();
56+
});
57+
5458
it('should check coverage', async () => {
5559
const { backend, fetchMock, getAccessToken } = setup();
5660

@@ -70,14 +74,24 @@ describe('ShieldRemoteBackend', () => {
7074

7175
const txMeta = generateMockTxMeta();
7276
const coverageResult = await backend.checkCoverage({ txMeta });
73-
expect(coverageResult).toStrictEqual({ coverageId, ...result });
77+
expect({
78+
coverageId: coverageResult.coverageId,
79+
message: result.message,
80+
reasonCode: result.reasonCode,
81+
status: result.status,
82+
}).toStrictEqual({
83+
coverageId,
84+
...result,
85+
});
86+
expect(typeof coverageResult.metrics.latency).toBe('number');
7487
expect(fetchMock).toHaveBeenCalledTimes(2);
7588
expect(getAccessToken).toHaveBeenCalledTimes(2);
7689
});
7790

7891
it('should check coverage with delay', async () => {
92+
const pollInterval = 100;
7993
const { backend, fetchMock, getAccessToken } = setup({
80-
getCoverageResultPollInterval: 100,
94+
getCoverageResultPollInterval: pollInterval,
8195
});
8296

8397
// Mock init coverage check.
@@ -101,13 +115,38 @@ describe('ShieldRemoteBackend', () => {
101115
} as unknown as Response);
102116

103117
const txMeta = generateMockTxMeta();
118+
119+
// generateMockTxMeta also use Date.now() to set the time, only do this after generateMockTxMeta
120+
// Mock Date.now() to control latency measurement
121+
// Simulate latency that includes the retry delay (poll interval + processing time)
122+
let callCount = 0;
123+
const startTime = 1000;
124+
const expectedLatency = pollInterval + 50; // poll interval + processing time
125+
const nowSpy = jest.spyOn(Date, 'now').mockImplementation(() => {
126+
callCount += 1;
127+
// First call: start of #getCoverageResult
128+
if (callCount === 1) {
129+
return startTime;
130+
}
131+
// Final call: end of #getCoverageResult (after retry delay)
132+
return startTime + expectedLatency;
133+
});
134+
104135
const coverageResult = await backend.checkCoverage({ txMeta });
105-
expect(coverageResult).toStrictEqual({
136+
137+
expect(coverageResult).toMatchObject({
106138
coverageId,
107-
...result,
139+
status: result.status,
140+
message: result.message,
141+
reasonCode: result.reasonCode,
108142
});
143+
expect(coverageResult.metrics.latency).toBe(expectedLatency);
144+
// Latency should include the retry delay (at least the poll interval)
145+
expect(coverageResult.metrics.latency).toBeGreaterThanOrEqual(pollInterval);
109146
expect(fetchMock).toHaveBeenCalledTimes(3);
110147
expect(getAccessToken).toHaveBeenCalledTimes(2);
148+
149+
nowSpy.mockRestore();
111150
});
112151

113152
it('should throw on init coverage check failure', async () => {
@@ -187,6 +226,66 @@ describe('ShieldRemoteBackend', () => {
187226
await new Promise((resolve) => setTimeout(resolve, 10));
188227
});
189228

229+
it('returns latency in coverageResult', async () => {
230+
const { backend, fetchMock } = setup();
231+
232+
fetchMock.mockResolvedValueOnce({
233+
status: 200,
234+
json: jest.fn().mockResolvedValue({ coverageId: 'coverageId' }),
235+
} as unknown as Response);
236+
237+
const result = { status: 'covered', message: 'ok', reasonCode: 'E104' };
238+
fetchMock.mockResolvedValueOnce({
239+
status: 200,
240+
json: jest.fn().mockResolvedValue(result),
241+
} as unknown as Response);
242+
243+
let nowValue = 1000;
244+
const latencyMs = 123;
245+
const nowSpy = jest.spyOn(Date, 'now').mockImplementation(() => {
246+
const val = nowValue;
247+
nowValue += latencyMs;
248+
return val;
249+
});
250+
251+
const txMeta = generateMockTxMeta();
252+
const coverageResult = await backend.checkCoverage({ txMeta });
253+
expect(coverageResult.metrics.latency).toBe(latencyMs);
254+
255+
nowSpy.mockRestore();
256+
});
257+
258+
it('returns latency in signatureCoverageResult', async () => {
259+
const { backend, fetchMock } = setup();
260+
261+
fetchMock.mockResolvedValueOnce({
262+
status: 200,
263+
json: jest.fn().mockResolvedValue({ coverageId: 'coverageId' }),
264+
} as unknown as Response);
265+
266+
const result = { status: 'covered', message: 'ok', reasonCode: 'E104' };
267+
fetchMock.mockResolvedValueOnce({
268+
status: 200,
269+
json: jest.fn().mockResolvedValue(result),
270+
} as unknown as Response);
271+
272+
let nowValue = 2000;
273+
const latencyMs = 456;
274+
const nowSpy = jest.spyOn(Date, 'now').mockImplementation(() => {
275+
const val = nowValue;
276+
nowValue += latencyMs;
277+
return val;
278+
});
279+
280+
const signatureRequest = generateMockSignatureRequest();
281+
const coverageResult = await backend.checkSignatureCoverage({
282+
signatureRequest,
283+
});
284+
expect(coverageResult.metrics.latency).toBe(latencyMs);
285+
286+
nowSpy.mockRestore();
287+
});
288+
190289
describe('checkSignatureCoverage', () => {
191290
it('should check signature coverage', async () => {
192291
const { backend, fetchMock, getAccessToken } = setup();
@@ -209,10 +308,16 @@ describe('ShieldRemoteBackend', () => {
209308
const coverageResult = await backend.checkSignatureCoverage({
210309
signatureRequest,
211310
});
212-
expect(coverageResult).toStrictEqual({
311+
expect({
312+
coverageId: coverageResult.coverageId,
313+
message: result.message,
314+
reasonCode: result.reasonCode,
315+
status: result.status,
316+
}).toStrictEqual({
213317
coverageId,
214318
...result,
215319
});
320+
expect(typeof coverageResult.metrics.latency).toBe('number');
216321
expect(fetchMock).toHaveBeenCalledTimes(2);
217322
expect(getAccessToken).toHaveBeenCalledTimes(2);
218323
});

packages/shield-controller/src/backend.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ export type GetCoverageResultResponse = {
5757
message?: string;
5858
reasonCode?: string;
5959
status: CoverageStatus;
60+
metrics: {
61+
latency?: number;
62+
};
6063
};
6164

6265
export class ShieldRemoteBackend implements ShieldBackend {
@@ -117,6 +120,7 @@ export class ShieldRemoteBackend implements ShieldBackend {
117120
message: coverageResult.message,
118121
reasonCode: coverageResult.reasonCode,
119122
status: coverageResult.status,
123+
metrics: coverageResult.metrics,
120124
};
121125
}
122126

@@ -143,6 +147,7 @@ export class ShieldRemoteBackend implements ShieldBackend {
143147
message: coverageResult.message,
144148
reasonCode: coverageResult.reasonCode,
145149
status: coverageResult.status,
150+
metrics: coverageResult.metrics,
146151
};
147152
}
148153

@@ -220,15 +225,20 @@ export class ShieldRemoteBackend implements ShieldBackend {
220225

221226
const headers = await this.#createHeaders();
222227

228+
// Start measuring total end-to-end latency including retries and delays
229+
const startTime = Date.now();
230+
223231
const getCoverageResultFn = async (signal: AbortSignal) => {
224232
const res = await this.#fetch(coverageResultUrl, {
225233
method: 'POST',
226234
headers,
227235
body: JSON.stringify(reqBody),
228236
signal,
229237
});
238+
230239
if (res.status === 200) {
231-
return (await res.json()) as GetCoverageResultResponse;
240+
// Return the result without latency here - we'll add total latency after polling completes
241+
return (await res.json()) as Omit<GetCoverageResultResponse, 'metrics'>;
232242
}
233243

234244
// parse the error message from the response body
@@ -242,7 +252,19 @@ export class ShieldRemoteBackend implements ShieldBackend {
242252
throw new HttpError(res.status, errorMessage);
243253
};
244254

245-
return this.#pollingPolicy.start(requestId, getCoverageResultFn);
255+
const result = await this.#pollingPolicy.start(
256+
requestId,
257+
getCoverageResultFn,
258+
);
259+
260+
// Calculate total end-to-end latency including all retries and delays
261+
const now = Date.now();
262+
const totalLatency = now - startTime;
263+
264+
return {
265+
...result,
266+
metrics: { latency: totalLatency },
267+
} as GetCoverageResultResponse;
246268
}
247269

248270
async #createHeaders() {

packages/shield-controller/src/types.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ export type CoverageResult = {
66
message?: string;
77
reasonCode?: string;
88
status: CoverageStatus;
9+
metrics: {
10+
latency?: number;
11+
};
912
};
1013

1114
export const coverageStatuses = ['covered', 'malicious', 'unknown'] as const;

0 commit comments

Comments
 (0)