feat(telemetry): add awsRegion in commonMetadata #2941

related: a50b83a

Also rename "filters" to "excludeKeys" because "filters" is ambiguous:
it can mean include or exclude.

Author: justinmk3

src/shared/telemetry/telemetryClient.ts

@@ -19,6 +19,7 @@ import { DevSettings } from '../settings'
 import { ClassToInterfaceType } from '../utilities/tsUtils'
 
 export const ACCOUNT_METADATA_KEY = 'awsAccount'
+export const REGION_KEY = 'awsRegion'
 export const COMPUTE_REGION_KEY = 'computeRegion'
 
 export enum AccountStatus {

src/shared/telemetry/telemetryLogger.ts

@@ -14,9 +14,9 @@ export interface MetricQuery {
     readonly metricName: string
 
     /**
-     * Attributes to filter out of the metadata
+     * Exclude metadata items matching these keys.
      */
-    readonly filters?: string[]
+    readonly excludeKeys?: string[]
 }
 
 interface Metadata {
@@ -30,11 +30,11 @@ function isValidEntry(datum: MetadataEntry): datum is Required<MetadataEntry> {
 /**
  * Telemetry currently sends metadata as an array of key/value pairs, but this is unintuitive for JS
  */
-const mapMetadata = (filters: string[]) => (metadata: Required<MetricDatum>['Metadata']) => {
+const mapMetadata = (excludeKeys: string[]) => (metadata: Required<MetricDatum>['Metadata']) => {
     const result: Metadata = {}
     return metadata
         .filter(isValidEntry)
-        .filter(a => !filters.includes(a.Key))
+        .filter(a => !excludeKeys.includes(a.Key))
         .reduce((a, b) => ((a[b.Key] = b.Value), a), result)
 }
 
@@ -80,7 +80,7 @@ export class TelemetryLogger {
     public query(query: MetricQuery): Metadata[] {
         return this.queryFull(query)
             .map(m => m.Metadata ?? [])
-            .map(mapMetadata(query.filters ?? []))
+            .map(mapMetadata(query.excludeKeys ?? []))
     }
 
     /**

src/shared/telemetry/telemetryService.ts

@@ -11,7 +11,7 @@ import { AwsContext } from '../awsContext'
 import { isReleaseVersion, isAutomation } from '../vscode/env'
 import { getLogger } from '../logger'
 import { MetricDatum } from './clienttelemetry'
-import { DefaultTelemetryClient } from './telemetryClient'
+import { DefaultTelemetryClient, REGION_KEY } from './telemetryClient'
 import { DefaultTelemetryPublisher } from './telemetryPublisher'
 import { TelemetryFeedback } from './telemetryClient'
 import { TelemetryPublisher } from './telemetryPublisher'
@@ -241,6 +241,9 @@ export class DefaultTelemetryService {
         if (this.computeRegion) {
             commonMetadata.push({ Key: COMPUTE_REGION_KEY, Value: this.computeRegion })
         }
+        if (!event?.Metadata?.some((m: any) => m?.Key == REGION_KEY)) {
+            commonMetadata.push({ Key: REGION_KEY, Value: globals.regionProvider.guessDefaultRegion() })
+        }
 
         if (event.Metadata) {
             event.Metadata.push(...commonMetadata)

src/test/codewhisperer/tracker/codewhispererCodeCoverageTracker.test.ts

@@ -318,7 +318,7 @@ describe('codewhispererCodecoverageTracker', function () {
             tracker.flush()
             const data = globals.telemetry.logger.query({
                 metricName: 'codewhisperer_codePercentage',
-                filters: ['awsAccount'],
+                excludeKeys: ['awsAccount'],
             })
             assert.strictEqual(data.length, 0)
         })

src/test/testUtil.ts

@@ -136,8 +136,11 @@ export function installFakeClock(): FakeTimers.InstalledClock {
  * Unlike {@link assertTelemetry}, this function does not do any transformations to
  * handle fields being converted into strings. It will also not return `passive` or `value`.
  */
-export function getMetrics<K extends MetricName>(name: K, ...filters: string[]): readonly Partial<MetricShapes[K]>[] {
-    const query = { metricName: name, filters: ['awsAccount', ...filters] }
+export function getMetrics<K extends MetricName>(
+    name: K,
+    ...excludeKeys: string[]
+): readonly Partial<MetricShapes[K]>[] {
+    const query = { metricName: name, excludeKeys: ['awsAccount', 'awsRegion', ...excludeKeys] }
 
     return globals.telemetry.logger.query(query) as unknown as Partial<MetricShapes[K]>[]
 }
@@ -149,13 +152,16 @@ export function getMetrics<K extends MetricName>(name: K, ...filters: string[]):
 export function assertTelemetry<K extends MetricName>(name: K, expected: MetricShapes[K]): void | never {
     const expectedCopy = { ...expected } as { -readonly [P in keyof MetricShapes[K]]: MetricShapes[K][P] }
     const passive = expectedCopy?.passive
-    const query = { metricName: name, filters: ['awsAccount', 'duration'] }
+    const query = { metricName: name, excludeKeys: ['awsAccount', 'duration'] }
     delete expectedCopy['passive']
 
     Object.keys(expectedCopy).forEach(
         k => ((expectedCopy as any)[k] = (expectedCopy as Record<string, any>)[k]?.toString())
     )
 
+    // Telemetry client should add awsRegion to all metrics.
+    ;(expectedCopy as any)['awsRegion'] = globals.regionProvider.guessDefaultRegion()
+
     const metadata = globals.telemetry.logger.query(query)
     assert.ok(metadata.length > 0, `Telemetry did not contain any metrics with the name "${name}"`)
     assert.deepStrictEqual(metadata[0], expectedCopy)