Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SEARCH_RESPONSE_TIME event #4044

Merged
merged 4 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 61 additions & 2 deletions frontend/src/data/media-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
> {
Expand All @@ -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
Expand All @@ -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) }
}

/**
Expand Down
10 changes: 9 additions & 1 deletion frontend/src/stores/media/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
/**
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/stores/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions frontend/src/types/analytics.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type {
MediaType,
SearchType,
SupportedMediaType,
SupportedSearchType,
} from "~/constants/media"
import type { ReportReason } from "~/constants/content-report"
Expand Down Expand Up @@ -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.
*
Expand All @@ -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
}
}

/**
Expand Down
128 changes: 128 additions & 0 deletions frontend/test/unit/specs/data/media-service.spec.ts
Original file line number Diff line number Diff line change
@@ -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",
})
})
})
10 changes: 8 additions & 2 deletions frontend/test/unit/specs/stores/media-store.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)

Expand Down