telemetry: set more MetricBase fields aws#5255

Problem:
`telemetry.run()` and `runCommand` do not set some "standard" fields on
failure.

Solution:
Update `telemetry.run()` and `runCommand`.

ref aws/aws-toolkit-common#769

Author: justinmk3

package-lock.json

@@ -69,7 +69,7 @@
         "webpack-merge": "^5.10.0"
     },
     "dependencies": {
-        "@aws-toolkits/telemetry": "^1.0.221",
+        "@aws-toolkits/telemetry": "^1.0.223",
         "vscode-nls": "^5.2.0",
         "vscode-nls-dev": "^4.0.4"
     }

package.json

@@ -21,7 +21,12 @@ import {
     PlanIterationLimitError,
     UnknownApiError,
 } from '../errors'
-import { ToolkitError, isAwsError, isCodeWhispererStreamingServiceException } from '../../shared/errors'
+import {
+    ToolkitError,
+    isAwsError,
+    isCodeWhispererStreamingServiceException,
+    getHttpStatusCode,
+} from '../../shared/errors'
 import { getCodewhispererConfig } from '../../codewhisperer/client/codewhisperer'
 import { LLMResponseType } from '../types'
 import { createCodeWhispererChatStreamingClient } from '../../shared/clients/codewhispererChatClient'
@@ -191,7 +196,7 @@ export class FeatureDevClient {
                     e.message,
                     'GeneratePlan',
                     e.name,
-                    e.$metadata?.httpStatusCode ?? streamResponseErrors[e.name] ?? 500
+                    getHttpStatusCode(e) ?? streamResponseErrors[e.name] ?? 500
                 )
             }
 

packages/core/src/amazonqFeatureDev/client/featureDev.ts

@@ -10,6 +10,7 @@ import {
     CodewhispererCompletionType,
     CodewhispererLanguage,
     CodewhispererTriggerType,
+    MetricBase,
     Result,
 } from '../../shared/telemetry/telemetry'
 import { References } from '../client/codewhisperer'
@@ -673,7 +674,7 @@ export class TransformByQStoppedError extends ToolkitError {
     }
 }
 
-export interface CodeScanTelemetryEntry {
+export interface CodeScanTelemetryEntry extends MetricBase {
     codewhispererCodeScanJobId?: string
     codewhispererLanguage: CodewhispererLanguage
     codewhispererCodeScanProjectBytes?: number

packages/core/src/codewhisperer/models/model.ts

@@ -45,6 +45,7 @@ import { triggerPayloadToChatRequest } from './chatRequest/converter'
 import { AuthUtil } from '../../../codewhisperer/util/authUtil'
 import { openUrl } from '../../../shared/utilities/vsCodeUtils'
 import { randomUUID } from '../../../common/crypto'
+import { getHttpStatusCode } from '../../../shared/errors'
 
 export interface ChatControllerMessagePublishers {
     readonly processPromptChatMessage: MessagePublisher<PromptMessage>
@@ -590,7 +591,7 @@ export class ChatController {
             )
             await this.messenger.sendAIResponse(response, session, tabID, triggerID, triggerPayload)
         } catch (e: any) {
-            this.telemetryHelper.recordMessageResponseError(triggerPayload, tabID, e?.$metadata?.httpStatusCode ?? 0)
+            this.telemetryHelper.recordMessageResponseError(triggerPayload, tabID, getHttpStatusCode(e) ?? 0)
             // clears session, record telemetry before this call
             this.processException(e, tabID)
         }

packages/core/src/codewhispererChat/controllers/chat/controller.ts

@@ -22,7 +22,7 @@ import { ChatSession } from '../../../clients/chat/v0/chat'
 import { ChatException } from './model'
 import { CWCTelemetryHelper } from '../telemetryHelper'
 import { ChatPromptCommandType, TriggerPayload } from '../model'
-import { ToolkitError } from '../../../../shared/errors'
+import { getHttpStatusCode, getRequestId, ToolkitError } from '../../../../shared/errors'
 import { keys } from '../../../../shared/utilities/tsUtils'
 import { getLogger } from '../../../../shared/logger/logger'
 import { FeatureAuthState } from '../../../../codewhisperer/util/authUtil'
@@ -218,8 +218,8 @@ export class Messenger {
 
                 if (error instanceof CodeWhispererStreamingServiceException) {
                     errorMessage = error.message
-                    statusCode = error.$metadata?.httpStatusCode ?? 0
-                    requestID = error.$metadata.requestId
+                    statusCode = getHttpStatusCode(error) ?? 0
+                    requestID = getRequestId(error)
                 }
 
                 this.showChatExceptionMessage(

packages/core/src/codewhispererChat/controllers/chat/messenger/messenger.ts

@@ -522,11 +522,15 @@ function hasFault<T>(error: T): error is T & { $fault: 'client' | 'server' } {
 }
 
 function hasMetadata<T>(error: T): error is T & Pick<CodeWhispererStreamingServiceException, '$metadata'> {
-    return typeof (error as { $metadata?: unknown }).$metadata === 'object'
+    return typeof (error as { $metadata?: unknown })?.$metadata === 'object'
+}
+
+function hasResponse<T>(error: T): error is T & Pick<ServiceException, '$response'> {
+    return typeof (error as { $response?: unknown })?.$response === 'object'
 }
 
 function hasName<T>(error: T): error is T & { name: string } {
-    return typeof (error as { name?: unknown }).name === 'string'
+    return typeof (error as { name?: unknown })?.name === 'string'
 }
 
 // eslint-disable-next-line @typescript-eslint/naming-convention
@@ -569,6 +573,17 @@ export function getRequestId(err: unknown): string | undefined {
     }
 }
 
+export function getHttpStatusCode(err: unknown): number | undefined {
+    if (hasResponse(err) && err?.$response?.statusCode !== undefined) {
+        return err?.$response?.statusCode
+    }
+    if (hasMetadata(err) && err.$metadata?.httpStatusCode !== undefined) {
+        return err.$metadata?.httpStatusCode
+    }
+
+    return undefined
+}
+
 export function isFilesystemError(err: unknown): boolean {
     if (
         err instanceof vscode.FileSystemError ||

packages/core/src/shared/errors.ts

@@ -15,7 +15,13 @@ import {
     MetricShapes,
     TelemetryBase,
 } from './telemetry.gen'
-import { getRequestId, getTelemetryReason, getTelemetryReasonDesc, getTelemetryResult } from '../errors'
+import {
+    getHttpStatusCode,
+    getRequestId,
+    getTelemetryReason,
+    getTelemetryReasonDesc,
+    getTelemetryResult,
+} from '../errors'
 import { entries, NumericKeys } from '../utilities/tsUtils'
 
 const AsyncLocalStorage: typeof AsyncLocalStorageClass =
@@ -151,14 +157,14 @@ export class TelemetrySpan<T extends MetricBase = MetricBase> {
     public stop(err?: unknown): void {
         const duration = this.startTime !== undefined ? globals.clock.Date.now() - this.startTime.getTime() : undefined
 
-        // TODO: add reasonDesc/requestId to `MetricBase` so we can remove `as any` below.
         this.emit({
             duration,
             result: getTelemetryResult(err),
             reason: getTelemetryReason(err),
             reasonDesc: getTelemetryReasonDesc(err),
             requestId: getRequestId(err),
-        } as any as Partial<T>)
+            httpStatusCode: getHttpStatusCode(err),
+        } as Partial<T>)
 
         this.#startTime = undefined
     }

packages/core/src/shared/telemetry/spans.ts

@@ -15,11 +15,14 @@ import {
     resolveErrorMessageToDisplay,
     scrubNames,
     ToolkitError,
+    UnknownError,
 } from '../../shared/errors'
 import { CancellationError } from '../../shared/utilities/timeoutUtils'
 import { UnauthorizedException } from '@aws-sdk/client-sso'
 import { AWSError } from 'aws-sdk'
 import { AccessDeniedException } from '@aws-sdk/client-sso-oidc'
+import { OidcClient } from '../../auth/sso/clients'
+import { SamCliError } from '../../shared/sam/cli/samCliInvokerUtils'
 
 class TestAwsError extends Error implements AWSError {
     constructor(
@@ -67,33 +70,58 @@ function fakeAwsErrorUnauth() {
     return e as UnauthorizedException
 }
 
+function trySetCause(err: Error | undefined, cause: unknown) {
+    if (err && !(err instanceof ToolkitError)) {
+        ;(err as any).cause = cause
+    }
+}
+
 /** Creates a deep "cause chain", to test that error handler correctly gets the most relevant error. */
-function fakeErrorChain(rootCause?: Error) {
+export function fakeErrorChain(err1?: Error, err2?: Error, err3?: Error, err4?: Error) {
     try {
-        if (rootCause) {
-            throw rootCause
+        if (err1) {
+            throw err1
         } else {
             throw new Error('generic error 1')
         }
     } catch (e1) {
         try {
-            const e = fakeAwsErrorAccessDenied()
-            ;(e as any).cause = e1
+            const e = err2 ? err2 : new UnknownError(e1)
+            trySetCause(e, e1)
             throw e
         } catch (e2) {
             try {
-                const e = fakeAwsErrorUnauth()
-                ;(e as any).cause = e2
-                return e
+                // new Error('error 3')
+                const e = err3 ? err3 : new SamCliError('sam error', { cause: e2 as Error })
+                trySetCause(e, e2)
+                throw e
             } catch (e3) {
-                throw ToolkitError.chain(e3, 'ToolkitError message', {
-                    documentationUri: vscode.Uri.parse('https://docs.aws.amazon.com/toolkit-for-vscode/'),
-                })
+                const e = err4
+                    ? err4
+                    : ToolkitError.chain(e3, 'ToolkitError message', {
+                          documentationUri: vscode.Uri.parse('https://docs.aws.amazon.com/toolkit-for-vscode/'),
+                      })
+                trySetCause(e, e3)
+
+                return e
             }
         }
     }
 }
 
+/** Sends a (broken) request to the AWS OIDC service, to get a "real" error response. */
+export async function getAwsServiceError(): Promise<Error> {
+    const oidc = OidcClient.create('us-east-1')
+    return oidc
+        .createToken({
+            clientId: 'AWS IDE Extensions',
+            clientSecret: 'xx',
+            deviceCode: 'xx',
+            grantType: 'urn:ietf:params:oauth:grant-type:device_code',
+        })
+        .catch(e => e)
+}
+
 describe('ToolkitError', function () {
     it('can store an error message', function () {
         const error = new ToolkitError('uh oh!')
@@ -408,13 +436,20 @@ describe('util', function () {
 
     it('formatError()', function () {
         assert.deepStrictEqual(
-            formatError(fakeErrorChain()),
+            formatError(
+                fakeErrorChain(undefined, fakeAwsErrorAccessDenied(), new Error('err 3'), fakeAwsErrorUnauth())
+            ),
             'unauthorized-name: unauthorized message [unauthorized-code] (requestId: be62f79a-e9cf-41cd-a755-e6920c56e4fb)'
         )
     })
 
     it('getErrorMsg()', function () {
-        assert.deepStrictEqual(getErrorMsg(fakeErrorChain()), 'unauthorized message')
+        assert.deepStrictEqual(
+            getErrorMsg(
+                fakeErrorChain(undefined, fakeAwsErrorAccessDenied(), new Error('err 3'), fakeAwsErrorUnauth())
+            ),
+            'unauthorized message'
+        )
         assert.deepStrictEqual(getErrorMsg(undefined), undefined)
         const err = new TestAwsError('ValidationException', 'aws validation msg 1', new Date())
         assert.deepStrictEqual(getErrorMsg(err), 'aws validation msg 1')

packages/core/src/test/shared/errors.test.ts

@@ -9,6 +9,7 @@ import { TelemetrySpan, TelemetryTracer } from '../../../shared/telemetry/spans'
 import { MetricName, MetricShapes } from '../../../shared/telemetry/telemetry'
 import { assertTelemetry, getMetrics, installFakeClock } from '../../testUtil'
 import { selectFrom } from '../../../shared/utilities/tsUtils'
+import { getAwsServiceError } from '../errors.test'
 
 describe('TelemetrySpan', function () {
     let clock: ReturnType<typeof installFakeClock>
@@ -77,7 +78,7 @@ describe('TelemetryTracer', function () {
         tracer = new TelemetryTracer()
     })
 
-    describe('record', function () {
+    describe('record()', function () {
         it('only writes to the active span in the current context', function () {
             tracer.apigateway_copyUrl.run(() => {
                 tracer.run(metricName, () => tracer.record({ source: 'bar' }))
@@ -122,7 +123,7 @@ describe('TelemetryTracer', function () {
         })
     })
 
-    describe('instrument', function () {
+    describe('instrument()', function () {
         async function assertPositive(n: number): Promise<number> {
             if (n <= 0) {
                 throw new Error()
@@ -165,7 +166,7 @@ describe('TelemetryTracer', function () {
         })
     })
 
-    describe('increment', function () {
+    describe('increment()', function () {
         it('starts at 0 for uninitialized fields', function () {
             tracer.vscode_executeCommand.run(span => {
                 span.record({ command: 'foo' })
@@ -195,7 +196,7 @@ describe('TelemetryTracer', function () {
         })
     })
 
-    describe('run', function () {
+    describe('run()', function () {
         it('returns the result of the function', function () {
             const result = tracer.run(metricName, () => 'foo')
 
@@ -214,7 +215,27 @@ describe('TelemetryTracer', function () {
             assertTelemetry(metricName, { result: 'Succeeded', source: 'bar' })
         })
 
-        describe('nested run', function () {
+        it('records standard fields on failed service request', async function () {
+            await assert.rejects(async () => {
+                await tracer.aws_loginWithBrowser.run(async () => {
+                    throw await getAwsServiceError()
+                })
+            })
+
+            assertTelemetry('aws_loginWithBrowser', {
+                result: 'Failed',
+                reason: 'InvalidRequestException',
+                reasonDesc: 'Invalid client ID provided',
+                httpStatusCode: '400',
+            })
+            const metric = getMetrics('aws_loginWithBrowser')[0]
+            assert.match(metric.awsAccount ?? '', /not-set|n\/a|\d+/)
+            assert.match(metric.awsRegion ?? '', /not-set|\w+-\w+-\d+/)
+            assert.match(String(metric.duration) ?? '', /\d+/)
+            assert.match(metric.requestId ?? '', /[a-z0-9-]+/)
+        })
+
+        describe('nested run()', function () {
             const nestedName = 'nested_metric' as MetricName
 
             it('can record metadata in nested spans', function () {