From d08fff9d1021715f2beb6fe149a460fc6a7215a4 Mon Sep 17 00:00:00 2001 From: Vinicius Stevam Date: Wed, 12 Nov 2025 13:52:42 +0000 Subject: [PATCH 1/4] add latency metrics to coverage results and update tests --- .../shield-controller/src/backend.test.ts | 64 +++++++++++++++++++ packages/shield-controller/src/backend.ts | 39 +++++++++-- packages/shield-controller/src/types.ts | 3 + packages/shield-controller/tests/utils.ts | 3 + 4 files changed, 102 insertions(+), 7 deletions(-) diff --git a/packages/shield-controller/src/backend.test.ts b/packages/shield-controller/src/backend.test.ts index 79adef1327a..b4a605b6847 100644 --- a/packages/shield-controller/src/backend.test.ts +++ b/packages/shield-controller/src/backend.test.ts @@ -51,6 +51,10 @@ function setup({ } describe('ShieldRemoteBackend', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('should check coverage', async () => { const { backend, fetchMock, getAccessToken } = setup(); @@ -187,6 +191,66 @@ describe('ShieldRemoteBackend', () => { await new Promise((resolve) => setTimeout(resolve, 10)); }); + it('returns latency in coverageResult', async () => { + const { backend, fetchMock } = setup(); + + fetchMock.mockResolvedValueOnce({ + status: 200, + json: jest.fn().mockResolvedValue({ coverageId: 'coverageId' }), + } as unknown as Response); + + const result = { status: 'covered', message: 'ok', reasonCode: 'E104' }; + fetchMock.mockResolvedValueOnce({ + status: 200, + json: jest.fn().mockResolvedValue(result), + } as unknown as Response); + + let nowValue = 1000; + const latencyMs = 123; + const nowSpy = jest.spyOn(Date, 'now').mockImplementation(() => { + const val = nowValue; + nowValue += latencyMs; + return val; + }); + + const txMeta = generateMockTxMeta(); + const coverageResult = await backend.checkCoverage({ txMeta }); + expect(coverageResult.metrics.latency).toBe(latencyMs); + + nowSpy.mockRestore(); + }); + + it('returns latency in signatureCoverageResult', async () => { + const { backend, fetchMock } = setup(); + + fetchMock.mockResolvedValueOnce({ + status: 200, + json: jest.fn().mockResolvedValue({ coverageId: 'coverageId' }), + } as unknown as Response); + + const result = { status: 'covered', message: 'ok', reasonCode: 'E104' }; + fetchMock.mockResolvedValueOnce({ + status: 200, + json: jest.fn().mockResolvedValue(result), + } as unknown as Response); + + let nowValue = 2000; + const latencyMs = 456; + const nowSpy = jest.spyOn(Date, 'now').mockImplementation(() => { + const val = nowValue; + nowValue += latencyMs; + return val; + }); + + const signatureRequest = generateMockSignatureRequest(); + const coverageResult = await backend.checkSignatureCoverage({ + signatureRequest, + }); + expect(coverageResult.metrics.latency).toBe(latencyMs); + + nowSpy.mockRestore(); + }); + describe('checkSignatureCoverage', () => { it('should check signature coverage', async () => { const { backend, fetchMock, getAccessToken } = setup(); diff --git a/packages/shield-controller/src/backend.ts b/packages/shield-controller/src/backend.ts index a748bedcde0..2a45600d211 100644 --- a/packages/shield-controller/src/backend.ts +++ b/packages/shield-controller/src/backend.ts @@ -57,6 +57,9 @@ export type GetCoverageResultResponse = { message?: string; reasonCode?: string; status: CoverageStatus; + metrics: { + latency?: number; + }; }; export class ShieldRemoteBackend implements ShieldBackend { @@ -117,6 +120,7 @@ export class ShieldRemoteBackend implements ShieldBackend { message: coverageResult.message, reasonCode: coverageResult.reasonCode, status: coverageResult.status, + metrics: coverageResult.metrics, }; } @@ -143,6 +147,7 @@ export class ShieldRemoteBackend implements ShieldBackend { message: coverageResult.message, reasonCode: coverageResult.reasonCode, status: coverageResult.status, + metrics: coverageResult.metrics, }; } @@ -209,6 +214,19 @@ export class ShieldRemoteBackend implements ShieldBackend { return (await res.json()) as InitCoverageCheckResponse; } + async #timedFetch( + url: string, + options: RequestInit, + ): Promise<{ response: Response; latency: number }> { + const startTime = Date.now(); + const response = await this.#fetch(url, options); + + return { + response, + latency: Date.now() - startTime, + }; + } + async #getCoverageResult( requestId: string, coverageId: string, @@ -221,14 +239,21 @@ export class ShieldRemoteBackend implements ShieldBackend { const headers = await this.#createHeaders(); const getCoverageResultFn = async (signal: AbortSignal) => { - const res = await this.#fetch(coverageResultUrl, { - method: 'POST', - headers, - body: JSON.stringify(reqBody), - signal, - }); + const { response: res, latency } = await this.#timedFetch( + coverageResultUrl, + { + method: 'POST', + headers, + body: JSON.stringify(reqBody), + signal, + }, + ); + if (res.status === 200) { - return (await res.json()) as GetCoverageResultResponse; + return { + ...(await res.json()), + metrics: { latency }, + } as GetCoverageResultResponse; } // parse the error message from the response body diff --git a/packages/shield-controller/src/types.ts b/packages/shield-controller/src/types.ts index ab00daabd85..1c9a911c691 100644 --- a/packages/shield-controller/src/types.ts +++ b/packages/shield-controller/src/types.ts @@ -6,6 +6,9 @@ export type CoverageResult = { message?: string; reasonCode?: string; status: CoverageStatus; + metrics: { + latency?: number; + }; }; export const coverageStatuses = ['covered', 'malicious', 'unknown'] as const; diff --git a/packages/shield-controller/tests/utils.ts b/packages/shield-controller/tests/utils.ts index 70f00b0be8d..d3b2d89f588 100644 --- a/packages/shield-controller/tests/utils.ts +++ b/packages/shield-controller/tests/utils.ts @@ -83,6 +83,9 @@ export function getRandomCoverageResult() { status: getRandomCoverageStatus(), message: 'message', reasonCode: 'reasonCode', + metrics: { + latency: 0, + }, }; } From 865ad7cbac451b6ed2f023e43919e917fc8f9711 Mon Sep 17 00:00:00 2001 From: Vinicius Stevam Date: Wed, 12 Nov 2025 14:07:03 +0000 Subject: [PATCH 2/4] add changelog --- packages/shield-controller/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/shield-controller/CHANGELOG.md b/packages/shield-controller/CHANGELOG.md index fcd14bc3abe..eef13f145e6 100644 --- a/packages/shield-controller/CHANGELOG.md +++ b/packages/shield-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Added metrics in the Shield coverage response to track the latency ( [#7133](https://github.com/MetaMask/core/pull/7133)) + ## [2.0.0] ### Changed From 39b6ea98f218f85cbfbc2bc9ad12e0d4b779b8e9 Mon Sep 17 00:00:00 2001 From: Tuna Date: Thu, 13 Nov 2025 14:35:37 +0700 Subject: [PATCH 3/4] fix: check coverage result total latency --- .../shield-controller/src/backend.test.ts | 32 ++++++++++-- packages/shield-controller/src/backend.ts | 51 +++++++++---------- 2 files changed, 53 insertions(+), 30 deletions(-) diff --git a/packages/shield-controller/src/backend.test.ts b/packages/shield-controller/src/backend.test.ts index b4a605b6847..87fb05fcd7c 100644 --- a/packages/shield-controller/src/backend.test.ts +++ b/packages/shield-controller/src/backend.test.ts @@ -80,8 +80,9 @@ describe('ShieldRemoteBackend', () => { }); it('should check coverage with delay', async () => { + const pollInterval = 100; const { backend, fetchMock, getAccessToken } = setup({ - getCoverageResultPollInterval: 100, + getCoverageResultPollInterval: pollInterval, }); // Mock init coverage check. @@ -105,13 +106,38 @@ describe('ShieldRemoteBackend', () => { } as unknown as Response); const txMeta = generateMockTxMeta(); + + // generateMockTxMeta also use Date.now() to set the time, only do this after generateMockTxMeta + // Mock Date.now() to control latency measurement + // Simulate latency that includes the retry delay (poll interval + processing time) + let callCount = 0; + const startTime = 1000; + const expectedLatency = pollInterval + 50; // poll interval + processing time + const nowSpy = jest.spyOn(Date, 'now').mockImplementation(() => { + callCount += 1; + // First call: start of #getCoverageResult + if (callCount === 1) { + return startTime; + } + // Final call: end of #getCoverageResult (after retry delay) + return startTime + expectedLatency; + }); + const coverageResult = await backend.checkCoverage({ txMeta }); - expect(coverageResult).toStrictEqual({ + + expect(coverageResult).toMatchObject({ coverageId, - ...result, + status: result.status, + message: result.message, + reasonCode: result.reasonCode, }); + expect(coverageResult.metrics.latency).toBe(expectedLatency); + // Latency should include the retry delay (at least the poll interval) + expect(coverageResult.metrics.latency).toBeGreaterThanOrEqual(pollInterval); expect(fetchMock).toHaveBeenCalledTimes(3); expect(getAccessToken).toHaveBeenCalledTimes(2); + + nowSpy.mockRestore(); }); it('should throw on init coverage check failure', async () => { diff --git a/packages/shield-controller/src/backend.ts b/packages/shield-controller/src/backend.ts index 2a45600d211..0db6e677ef2 100644 --- a/packages/shield-controller/src/backend.ts +++ b/packages/shield-controller/src/backend.ts @@ -214,19 +214,6 @@ export class ShieldRemoteBackend implements ShieldBackend { return (await res.json()) as InitCoverageCheckResponse; } - async #timedFetch( - url: string, - options: RequestInit, - ): Promise<{ response: Response; latency: number }> { - const startTime = Date.now(); - const response = await this.#fetch(url, options); - - return { - response, - latency: Date.now() - startTime, - }; - } - async #getCoverageResult( requestId: string, coverageId: string, @@ -238,22 +225,20 @@ export class ShieldRemoteBackend implements ShieldBackend { const headers = await this.#createHeaders(); + // Start measuring total end-to-end latency including retries and delays + const startTime = Date.now(); + const getCoverageResultFn = async (signal: AbortSignal) => { - const { response: res, latency } = await this.#timedFetch( - coverageResultUrl, - { - method: 'POST', - headers, - body: JSON.stringify(reqBody), - signal, - }, - ); + const res = await this.#fetch(coverageResultUrl, { + method: 'POST', + headers, + body: JSON.stringify(reqBody), + signal, + }); if (res.status === 200) { - return { - ...(await res.json()), - metrics: { latency }, - } as GetCoverageResultResponse; + // Return the result without latency here - we'll add total latency after polling completes + return (await res.json()) as Omit; } // parse the error message from the response body @@ -267,7 +252,19 @@ export class ShieldRemoteBackend implements ShieldBackend { throw new HttpError(res.status, errorMessage); }; - return this.#pollingPolicy.start(requestId, getCoverageResultFn); + const result = await this.#pollingPolicy.start( + requestId, + getCoverageResultFn, + ); + + // Calculate total end-to-end latency including all retries and delays + const now = Date.now(); + const totalLatency = now - startTime; + + return { + ...result, + metrics: { latency: totalLatency }, + } as GetCoverageResultResponse; } async #createHeaders() { From 65555f47fe5e6f7790efbfe44d9c2fd18a61e5db Mon Sep 17 00:00:00 2001 From: Tuna Date: Thu, 13 Nov 2025 14:44:38 +0700 Subject: [PATCH 4/4] fix: test case --- .../shield-controller/src/backend.test.ts | 19 +++++++++++++++++-- packages/shield-controller/tests/utils.ts | 3 --- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/packages/shield-controller/src/backend.test.ts b/packages/shield-controller/src/backend.test.ts index 87fb05fcd7c..b5777f6bbc7 100644 --- a/packages/shield-controller/src/backend.test.ts +++ b/packages/shield-controller/src/backend.test.ts @@ -74,7 +74,16 @@ describe('ShieldRemoteBackend', () => { const txMeta = generateMockTxMeta(); const coverageResult = await backend.checkCoverage({ txMeta }); - expect(coverageResult).toStrictEqual({ coverageId, ...result }); + expect({ + coverageId: coverageResult.coverageId, + message: result.message, + reasonCode: result.reasonCode, + status: result.status, + }).toStrictEqual({ + coverageId, + ...result, + }); + expect(typeof coverageResult.metrics.latency).toBe('number'); expect(fetchMock).toHaveBeenCalledTimes(2); expect(getAccessToken).toHaveBeenCalledTimes(2); }); @@ -299,10 +308,16 @@ describe('ShieldRemoteBackend', () => { const coverageResult = await backend.checkSignatureCoverage({ signatureRequest, }); - expect(coverageResult).toStrictEqual({ + expect({ + coverageId: coverageResult.coverageId, + message: result.message, + reasonCode: result.reasonCode, + status: result.status, + }).toStrictEqual({ coverageId, ...result, }); + expect(typeof coverageResult.metrics.latency).toBe('number'); expect(fetchMock).toHaveBeenCalledTimes(2); expect(getAccessToken).toHaveBeenCalledTimes(2); }); diff --git a/packages/shield-controller/tests/utils.ts b/packages/shield-controller/tests/utils.ts index d3b2d89f588..70f00b0be8d 100644 --- a/packages/shield-controller/tests/utils.ts +++ b/packages/shield-controller/tests/utils.ts @@ -83,9 +83,6 @@ export function getRandomCoverageResult() { status: getRandomCoverageStatus(), message: 'message', reasonCode: 'reasonCode', - metrics: { - latency: 0, - }, }; }