Merge pull request #5119 from Shopify/12-17-send_a_list_of_request_id…

…s_to_monorail

Send a list of request ids to monorail

packages/cli-kit/src/private/node/request-ids.test.ts

@@ -0,0 +1,68 @@
+import {MAX_REQUEST_IDS, requestIdsCollection} from './request-ids.js'
+import {describe, test, expect, beforeEach} from 'vitest'
+
+describe('RequestIDCollection', () => {
+  beforeEach(() => {
+    requestIdsCollection.clear()
+  })
+
+  test('starts with an empty collection', () => {
+    expect(requestIdsCollection.getRequestIds()).toEqual([])
+  })
+
+  test('adds request IDs to collection', () => {
+    // When
+    requestIdsCollection.addRequestId('request-1')
+    requestIdsCollection.addRequestId('request-2')
+
+    // Then
+    expect(requestIdsCollection.getRequestIds()).toEqual(['request-1', 'request-2'])
+  })
+
+  test('ignores undefined or null request IDs', () => {
+    // When
+    requestIdsCollection.addRequestId(undefined)
+    requestIdsCollection.addRequestId(null)
+    requestIdsCollection.addRequestId('request-1')
+
+    // Then
+    expect(requestIdsCollection.getRequestIds()).toEqual(['request-1'])
+  })
+
+  test('limits collection to MAX_REQUEST_IDS', () => {
+    // When
+    for (let i = 0; i < MAX_REQUEST_IDS + 20; i++) {
+      requestIdsCollection.addRequestId(`request-${i}`)
+    }
+
+    // Then
+    expect(requestIdsCollection.getRequestIds()).toHaveLength(100)
+    expect(requestIdsCollection.getRequestIds()[0]).toBe('request-0')
+    expect(requestIdsCollection.getRequestIds()[99]).toBe('request-99')
+  })
+
+  test('clear() removes all request IDs', () => {
+    // Given
+    requestIdsCollection.addRequestId('request-1')
+    requestIdsCollection.addRequestId('request-2')
+
+    // When
+    requestIdsCollection.clear()
+
+    // Then
+    expect(requestIdsCollection.getRequestIds()).toEqual([])
+  })
+
+  test('maintains singleton instance', () => {
+    // Given
+    requestIdsCollection.addRequestId('request-1')
+
+    // When
+    const sameInstance = requestIdsCollection
+    sameInstance.addRequestId('request-2')
+
+    // Then
+    expect(requestIdsCollection.getRequestIds()).toEqual(['request-1', 'request-2'])
+    expect(sameInstance.getRequestIds()).toEqual(['request-1', 'request-2'])
+  })
+})

packages/cli-kit/src/private/node/request-ids.ts

@@ -0,0 +1,43 @@
+export const MAX_REQUEST_IDS = 100
+
+/**
+ * Manages collection of request IDs during command execution
+ */
+class RequestIDCollection {
+  private static instance: RequestIDCollection
+
+  static getInstance(): RequestIDCollection {
+    if (!RequestIDCollection.instance) {
+      RequestIDCollection.instance = new RequestIDCollection()
+    }
+    return RequestIDCollection.instance
+  }
+
+  private requestIds: string[] = []
+
+  /**
+   * Add a request ID to the collection
+   * We only report the first MAX_REQUEST_IDS request IDs.
+   */
+  addRequestId(requestId: string | undefined | null) {
+    if (requestId && this.requestIds.length < MAX_REQUEST_IDS) {
+      this.requestIds.push(requestId)
+    }
+  }
+
+  /**
+   * Get all collected request IDs
+   */
+  getRequestIds(): string[] {
+    return this.requestIds
+  }
+
+  /**
+   * Clear all stored request IDs
+   */
+  clear() {
+    this.requestIds = []
+  }
+}
+
+export const requestIdsCollection = RequestIDCollection.getInstance()

packages/cli-kit/src/public/node/analytics.ts

@@ -10,6 +10,7 @@ import {recordMetrics} from '../../private/node/otel-metrics.js'
 import {runWithRateLimit} from '../../private/node/conf-store.js'
 import {reportingRateLimit} from '../../private/node/constants.js'
 import {getLastSeenUserIdAfterAuth} from '../../private/node/session.js'
+import {requestIdsCollection} from '../../private/node/request-ids.js'
 import {Interfaces} from '@oclif/core'
 
 export type CommandExitMode =
@@ -154,6 +155,7 @@ async function buildPayload({config, errorMessage, exitMode}: ReportAnalyticsEve
       cmd_all_timing_active_ms: totalTimeWithoutSubtimers,
       cmd_all_exit: exitMode,
       user_id: await getLastSeenUserIdAfterAuth(),
+      request_ids: requestIdsCollection.getRequestIds(),
     },
     sensitive: {
       args: startArgs.join(' '),

packages/cli-kit/src/public/node/api/graphql.test.ts

@@ -1,17 +1,26 @@
 import {graphqlRequest, graphqlRequestDoc} from './graphql.js'
-import {retryAwareRequest} from '../../../private/node/api.js'
+import * as api from '../../../private/node/api.js'
 import * as debugRequest from '../../../private/node/api/graphql.js'
 import {buildHeaders} from '../../../private/node/api/headers.js'
+import {requestIdsCollection} from '../../../private/node/request-ids.js'
+import * as metadata from '../metadata.js'
 import {GraphQLClient} from 'graphql-request'
 import {test, vi, describe, expect, beforeEach} from 'vitest'
-import {Headers} from 'node-fetch'
 import {TypedDocumentNode} from '@graphql-typed-document-node/core'
 
-vi.mock('../../../private/node/api.js')
+let mockedRequestId = 'request-id-123'
+
 vi.mock('graphql-request', async () => {
   const actual = await vi.importActual('graphql-request')
   const client = vi.fn()
-  client.prototype.rawRequest = vi.fn()
+  client.prototype.rawRequest = () => {
+    return {
+      status: 200,
+      headers: new Headers({
+        'x-request-id': mockedRequestId,
+      }),
+    }
+  }
 
   return {
     ...(actual as object),
@@ -20,20 +29,18 @@ vi.mock('graphql-request', async () => {
 })
 vi.spyOn(debugRequest, 'debugLogRequestInfo').mockResolvedValue(undefined)
 
-const mockedAddress = 'mockedAddress'
+const mockedAddress = 'http://localhost:3000'
 const mockVariables = {some: 'variables'}
 const mockToken = 'token'
 const mockedAddedHeaders = {some: 'header'}
 
 beforeEach(async () => {
-  vi.mocked(retryAwareRequest).mockResolvedValue({
-    status: 200,
-    headers: {} as Headers,
-  })
+  requestIdsCollection.clear()
 })
 
 describe('graphqlRequest', () => {
   test('calls debugLogRequestInfo once', async () => {
+    // When
     await graphqlRequest({
       query: 'query',
       api: 'mockApi',
@@ -42,6 +49,8 @@ describe('graphqlRequest', () => {
       addedHeaders: mockedAddedHeaders,
       variables: mockVariables,
     })
+
+    // Then
     expect(GraphQLClient).toHaveBeenCalledWith(mockedAddress, {
       agent: expect.any(Object),
       headers: {
@@ -50,16 +59,44 @@ describe('graphqlRequest', () => {
       },
     })
     expect(debugRequest.debugLogRequestInfo).toHaveBeenCalledOnce()
-    const receivedObject = {
-      request: expect.any(Function),
+  })
+
+  test('Logs the request ids to metadata and requestIdCollection', async () => {
+    // Given
+    const metadataSpyOn = vi.spyOn(metadata, 'addPublicMetadata').mockImplementation(async () => {})
+
+    // When
+    await graphqlRequest({
+      query: 'query',
+      api: 'mockApi',
       url: mockedAddress,
-    }
-    expect(retryAwareRequest).toHaveBeenCalledWith(receivedObject, expect.any(Function), undefined)
+      token: mockToken,
+      addedHeaders: mockedAddedHeaders,
+      variables: mockVariables,
+    })
+
+    mockedRequestId = 'request-id-456'
+
+    await graphqlRequest({
+      query: 'query',
+      api: 'mockApi',
+      url: mockedAddress,
+      token: mockToken,
+      addedHeaders: mockedAddedHeaders,
+      variables: mockVariables,
+    })
+
+    // Then
+    expect(requestIdsCollection.getRequestIds()).toEqual(['request-id-123', 'request-id-456'])
+    expect(metadataSpyOn).toHaveBeenCalledTimes(2)
+    expect(metadataSpyOn.mock.calls[0]![0]()).toEqual({cmd_all_last_graphql_request_id: 'request-id-123'})
+    expect(metadataSpyOn.mock.calls[1]![0]()).toEqual({cmd_all_last_graphql_request_id: 'request-id-456'})
   })
 })
 
 describe('graphqlRequestDoc', () => {
   test('converts document before querying', async () => {
+    // Given
     const document = {
       kind: 'Document',
       definitions: [
@@ -80,6 +117,9 @@ describe('graphqlRequestDoc', () => {
       ],
     } as unknown as TypedDocumentNode<unknown, unknown>
 
+    const retryAwareSpy = vi.spyOn(api, 'retryAwareRequest')
+
+    // When
     await graphqlRequestDoc({
       query: document,
       api: 'mockApi',
@@ -89,7 +129,8 @@ describe('graphqlRequestDoc', () => {
       variables: mockVariables,
     })
 
-    expect(retryAwareRequest).toHaveBeenCalledWith(
+    // Then
+    expect(retryAwareSpy).toHaveBeenCalledWith(
       {
         request: expect.any(Function),
         url: mockedAddress,
@@ -102,7 +143,7 @@ describe('graphqlRequestDoc', () => {
       `query QueryName {
   example
 }`,
-      'mockedAddress',
+      'http://localhost:3000',
       mockVariables,
       expect.anything(),
     )

packages/cli-kit/src/public/node/api/graphql.ts

@@ -2,6 +2,7 @@ import {buildHeaders, httpsAgent} from '../../../private/node/api/headers.js'
 import {debugLogRequestInfo, errorHandler} from '../../../private/node/api/graphql.js'
 import {addPublicMetadata, runWithTimer} from '../metadata.js'
 import {retryAwareRequest} from '../../../private/node/api.js'
+import {requestIdsCollection} from '../../../private/node/request-ids.js'
 import {
   GraphQLClient,
   rawRequest,
@@ -105,11 +106,10 @@ async function performGraphQLRequest<TResult>(options: PerformGraphQLRequestOpti
 async function logLastRequestIdFromResponse(response: GraphQLResponse<unknown>) {
   try {
     const requestId = response.headers.get('x-request-id')
-    await addPublicMetadata(async () => {
-      return {
-        cmd_all_last_graphql_request_id: requestId ?? undefined,
-      }
-    })
+    requestIdsCollection.addRequestId(requestId)
+    await addPublicMetadata(() => ({
+      cmd_all_last_graphql_request_id: requestId ?? undefined,
+    }))
     // eslint-disable-next-line no-catch-all/no-catch-all
   } catch {
     // no problem if unable to get request ID.

packages/cli-kit/src/public/node/monorail.ts

@@ -10,7 +10,7 @@ const url = 'https://monorail-edge.shopifysvc.com/v1/produce'
 type Optional<T> = T | null
 
 // This is the topic name of the main event we log to Monorail, the command tracker
-export const MONORAIL_COMMAND_TOPIC = 'app_cli3_command/1.15'
+export const MONORAIL_COMMAND_TOPIC = 'app_cli3_command/1.16'
 
 export interface Schemas {
   [MONORAIL_COMMAND_TOPIC]: {