Add SEARCH_RESPONSE_TIME event (#4044)

* Add `SEARCH_RESPONSE_TIME` event Signed-off-by: Olga Bulat <[email protected]> * Add and update tests Signed-off-by: Olga Bulat <[email protected]> * Rename variables Signed-off-by: Olga Bulat <[email protected]> * Join the two events into one Signed-off-by: Olga Bulat <[email protected]> --------- Signed-off-by: Olga Bulat <[email protected]> Co-authored-by: Victoria A <[email protected]>
WordPress · Apr 9, 2024 · 54d26f1 · 54d26f1
1 parent d25fa3a
commit 54d26f1
Show file tree

Hide file tree

Showing 6 changed files with 229 additions and 7 deletions.
diff --git 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<string, Media>,
 > {
@@ -28,6 +31,54 @@ class MediaService<T extends Media> {
     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<T extends Media> {
    */
   async search(
     params: PaginatedSearchQuery | PaginatedCollectionQuery
-  ): Promise<MediaResult<Record<string, Media>>> {
+  ): Promise<{
+    eventPayload: SearchTimeEventPayload | undefined
+    data: MediaResult<Record<string, Media>>
+  }> {
     // 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<MediaResult<T[]>>(
       this.mediaType,
       params as unknown as Record<string, string>
     )
-    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
@@ -27,6 +27,7 @@ import { deepFreeze } from "~/utils/deep-freeze"
 interface SearchFetchState extends Omit<FetchState, "hasStarted"> {
   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
@@ -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
@@ -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
@@ -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
@@ -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)