From 54d26f12eccc671aee3c5810b2776a2ea8f09d21 Mon Sep 17 00:00:00 2001 From: Olga Bulat Date: Tue, 9 Apr 2024 11:00:34 +0300 Subject: [PATCH] Add `SEARCH_RESPONSE_TIME` event (#4044) * Add `SEARCH_RESPONSE_TIME` event Signed-off-by: Olga Bulat * Add and update tests Signed-off-by: Olga Bulat * Rename variables Signed-off-by: Olga Bulat * Join the two events into one Signed-off-by: Olga Bulat --------- Signed-off-by: Olga Bulat Co-authored-by: Victoria A <52001888+adjeiv@users.noreply.github.com> --- frontend/src/data/media-service.ts | 63 ++++++++- frontend/src/stores/media/index.ts | 10 +- frontend/src/stores/search.ts | 4 +- frontend/src/types/analytics.ts | 21 +++ .../unit/specs/data/media-service.spec.ts | 128 ++++++++++++++++++ .../unit/specs/stores/media-store.spec.js | 10 +- 6 files changed, 229 insertions(+), 7 deletions(-) create mode 100644 frontend/test/unit/specs/data/media-service.spec.ts diff --git a/frontend/src/data/media-service.ts b/frontend/src/data/media-service.ts index 1d56e69766d..1a3e1a9d11b 100644 --- a/frontend/src/data/media-service.ts +++ b/frontend/src/data/media-service.ts @@ -6,9 +6,12 @@ import type { import type { ApiService } from "~/data/api-service" import type { DetailFromMediaType, Media } from "~/types/media" import { AUDIO, type SupportedMediaType } from "~/constants/media" +import type { Events } from "~/types/analytics" import type { AxiosResponse } from "axios" +export type SearchTimeEventPayload = Events["SEARCH_RESPONSE_TIME"] + export interface MediaResult< T extends Media | Media[] | Record, > { @@ -28,6 +31,54 @@ class MediaService { this.mediaType = mediaType } + /** + * Processes AxiosResponse from a search query to construct + * SEARCH_RESPONSE_TIME analytics event payload. + * @param response - Axios response + * @param requestDatetime - datetime before request was sent + */ + buildEventPayload( + response: AxiosResponse, + requestDatetime: Date + ): SearchTimeEventPayload | undefined { + const REQUIRED_HEADERS = ["date", "cf-cache-status", "cf-ray"] + + const responseHeaders = response.headers + if (!REQUIRED_HEADERS.every((header) => header in responseHeaders)) { + return + } + + const responseDatetime = new Date(responseHeaders["date"]) + if (responseDatetime < requestDatetime) { + // response returned was from the local cache + return + } + + const cfRayIATA = responseHeaders["cf-ray"].split("-")[1] + if (cfRayIATA === undefined) { + return + } + + const elapsedSeconds = Math.floor( + (responseDatetime.getTime() - requestDatetime.getTime()) / 1000 + ) + + const responseUrl = + response.request?.responseURL ?? response.request?.res?.responseUrl + if (!responseUrl) { + return + } + const url = new URL(responseUrl) + + return { + cfCacheStatus: String(responseHeaders["cf-cache-status"]), + cfRayIATA: String(cfRayIATA), + elapsedTime: elapsedSeconds, + queryString: url.search, + mediaType: this.mediaType, + } + } + /** * Decodes the text data to avoid encoding problems. * Also, converts the results from an array of media @@ -54,17 +105,25 @@ class MediaService { */ async search( params: PaginatedSearchQuery | PaginatedCollectionQuery - ): Promise>> { + ): Promise<{ + eventPayload: SearchTimeEventPayload | undefined + data: MediaResult> + }> { // Add the `peaks` param to all audio searches automatically if (this.mediaType === AUDIO) { params.peaks = "true" } + const requestDatetime = new Date() + const res = await this.apiService.query>( this.mediaType, params as unknown as Record ) - return this.transformResults(res.data) + + const eventPayload = this.buildEventPayload(res, requestDatetime) + + return { eventPayload, data: this.transformResults(res.data) } } /** diff --git a/frontend/src/stores/media/index.ts b/frontend/src/stores/media/index.ts index 2da62aad110..f9ae991d1c8 100644 --- a/frontend/src/stores/media/index.ts +++ b/frontend/src/stores/media/index.ts @@ -27,6 +27,7 @@ import { deepFreeze } from "~/utils/deep-freeze" interface SearchFetchState extends Omit { hasStarted: boolean } +import type { SearchTimeEventPayload } from "~/data/media-service" export type MediaStoreResult = { count: number @@ -448,6 +449,12 @@ export const useMediaStore = defineStore("media", { this.currentPage = 0 }, + recordSearchTime(payload: SearchTimeEventPayload | undefined) { + if (payload) { + this.$nuxt.$sendCustomEvent("SEARCH_RESPONSE_TIME", payload) + } + }, + /** * @param mediaType - the mediaType to fetch (do not use 'All_media' here) * @param shouldPersistMedia - whether the existing media should be added to or replaced. @@ -470,7 +477,8 @@ export const useMediaStore = defineStore("media", { try { const accessToken = this.$nuxt.$openverseApiToken const service = initServices[mediaType](accessToken) - const data = await service.search(queryParams) + const { eventPayload, data } = await service.search(queryParams) + this.recordSearchTime(eventPayload) const mediaCount = data.result_count let errorData: FetchingError | undefined /** diff --git a/frontend/src/stores/search.ts b/frontend/src/stores/search.ts index 6ba3460e6d3..9e8834ac333 100644 --- a/frontend/src/stores/search.ts +++ b/frontend/src/stores/search.ts @@ -121,10 +121,10 @@ export function buildCollectionQuery( const { collection, ...params } = collectionParams const query: PaginatedCollectionQuery = { + collection, + unstable__collection: collection, ...params, ...getSensitiveQuery("API"), - unstable__collection: collection, - collection, } if ("tag" in query) { query.unstable__tag = query.tag diff --git a/frontend/src/types/analytics.ts b/frontend/src/types/analytics.ts index 554c1e6a6f8..b44a82e33be 100644 --- a/frontend/src/types/analytics.ts +++ b/frontend/src/types/analytics.ts @@ -1,6 +1,7 @@ import type { MediaType, SearchType, + SupportedMediaType, SupportedSearchType, } from "~/constants/media" import type { ReportReason } from "~/constants/content-report" @@ -435,6 +436,7 @@ export type Events = { /** the reasons for why this result is considered sensitive */ sensitivities: string } + /** * Description: The user expands collapsed tags or collapses the expanded ones. * @@ -457,6 +459,25 @@ export type Events = { /** The search type when the network error occurred */ searchType: SupportedSearchType } + + /** + * Description: Time client-side search responses. Gives us observability into + * real user experience of search timings. + * Questions: + * - How long does it take for the client to receive a response to search requests? + */ + SEARCH_RESPONSE_TIME: { + /** the media type being searched */ + mediaType: SupportedMediaType + /** the Cloudflare cache status, denoting whether the request hit Cloudflare or went all the way to our servers */ + cfCacheStatus: string + /** the IATA location identifier as part of the `cf-ray` header, indicating the data centre the request passed through */ + cfRayIATA: string + /** how many seconds it took to receive a response for the request */ + elapsedTime: number + /** full query string */ + queryString: string + } } /** diff --git a/frontend/test/unit/specs/data/media-service.spec.ts b/frontend/test/unit/specs/data/media-service.spec.ts new file mode 100644 index 00000000000..c4f233301da --- /dev/null +++ b/frontend/test/unit/specs/data/media-service.spec.ts @@ -0,0 +1,128 @@ +import { mockCreateApiService } from "~~/test/unit/test-utils/api-service-mock" + +import { initServices } from "~/stores/media/services" + +const API_IMAGES_ENDPOINT = "images/" +const API_AUDIO_ENDPOINT = "audio/" +const BASE_URL = "https://www.mockapiservice.openverse.engineering/v1/" + +beforeAll(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + jest.useFakeTimers("modern") + jest.setSystemTime(new Date("Tue, 17 Dec 2019 20:20:00 GMT")) +}) + +afterAll(() => { + jest.useRealTimers() +}) + +describe("Media Service search and recordSearchTime", () => { + it("should not send a SEARCH_RESPONSE_TIME analytics event if any required header is missing", async () => { + mockCreateApiService((axiosMockAdapter) => { + axiosMockAdapter.onGet().reply(200, {}) + }) + + const res = await initServices.image().search({}) + + expect(res.eventPayload).not.toBeDefined() + }) + + it("should not send a SEARCH_RESPONSE_TIME analytics event if the response was locally cached", async () => { + mockCreateApiService((axiosMockAdapter) => { + axiosMockAdapter.onGet().reply(() => { + return [ + 200, + {}, + { + date: "Tue, 17 Dec 2019 19:00:00 GMT", + "cf-ray": "230b030023ae284c-SJC", + "cf-cache-status": "HIT", + }, + ] + }) + }) + + const res = await initServices.audio().search({}) + + expect(res.eventPayload).not.toBeDefined() + }) + + it("should not send a SEARCH_RESPONSE_TIME analytics event if the cf-ray is malformed", async () => { + mockCreateApiService((axiosMockAdapter) => { + axiosMockAdapter.onGet().reply((config) => { + // force config.url so the responseURL is set in the AxiosRequest + config.url = BASE_URL + config.url + return [ + 200, + {}, + { + date: "Tue, 17 Dec 2019 20:30:00 GMT", + "cf-ray": "230b030023ae284c", + "cf-cache-status": "HIT", + }, + ] + }) + }) + + const res = await initServices.audio().search({}) + + expect(res.eventPayload).not.toBeDefined() + }) + + it("should send SEARCH_RESPONSE_TIME analytics with correct parameters", async () => { + mockCreateApiService((axiosMockAdapter) => { + axiosMockAdapter + .onGet(API_IMAGES_ENDPOINT, { params: { q: "apple" } }) + .reply((config) => { + config.url = BASE_URL + config.url + "?q=apple" + return [ + 200, + {}, + { + date: "Tue, 17 Dec 2019 20:20:02 GMT", + "cf-ray": "230b030023ae2822-SJC", + "cf-cache-status": "HIT", + }, + ] + }) + + axiosMockAdapter + .onGet(API_AUDIO_ENDPOINT, { params: { q: "table", peaks: "true" } }) + .reply((config) => { + config.url = BASE_URL + config.url + "?q=table&peaks=true" + return [ + 200, + {}, + { + date: "Tue, 17 Dec 2019 20:20:03 GMT", + "cf-ray": "240b030b23ae2822-LHR", + "cf-cache-status": "MISS", + }, + ] + }) + }) + + const IMAGE_QUERY_PARAMS = { q: "apple" } + const imageRes = await initServices.image().search(IMAGE_QUERY_PARAMS) + + expect(imageRes.eventPayload).toEqual({ + cfCacheStatus: "HIT", + cfRayIATA: "SJC", + elapsedTime: 2, + queryString: "?q=apple", + mediaType: "image", + }) + + const AUDIO_QUERY_PARAMS = { q: "table" } + const audioRes = await initServices.audio().search(AUDIO_QUERY_PARAMS) + + expect(audioRes.eventPayload).toEqual({ + cfCacheStatus: "MISS", + cfRayIATA: "LHR", + elapsedTime: 3, + queryString: "?q=table&peaks=true", + mediaType: "audio", + }) + }) +}) diff --git a/frontend/test/unit/specs/stores/media-store.spec.js b/frontend/test/unit/specs/stores/media-store.spec.js index 1e612faa378..cc6b35396a7 100644 --- a/frontend/test/unit/specs/stores/media-store.spec.js +++ b/frontend/test/unit/specs/stores/media-store.spec.js @@ -42,7 +42,10 @@ const searchResults = (mediaType) => ({ }) const mockImplementation = (mediaType) => () => - Promise.resolve({ ...searchResults(mediaType) }) + Promise.resolve({ + searchTimeEvent: undefined, + data: { ...searchResults(mediaType) }, + }) const mockSearchAudio = jest.fn().mockImplementation(mockImplementation(AUDIO)) const mockSearchImage = jest.fn().mockImplementation(mockImplementation(IMAGE)) const mockGetMediaDetail = jest.fn() @@ -329,7 +332,10 @@ describe("Media Store", () => { shouldPersistMedia: true, mediaType: IMAGE, } - const emptyResult = { result_count: 0, page_count: 0, results: [] } + const emptyResult = { + searchTimeEvent: undefined, + data: { result_count: 0, page_count: 0, results: [] }, + } mockSearchImage.mockResolvedValueOnce(emptyResult) await mediaStore.fetchSingleMediaType(params)