feat: only log each warning message once for invalid metrics

Author: Hweinstock

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

@@ -8,7 +8,7 @@ import { env, version } from 'vscode'
 import * as os from 'os'
 import { getLogger } from '../logger/logger'
 import { fromExtensionManifest, Settings } from '../settings'
-import { memoize, once } from '../utilities/functionUtils'
+import { memoize, once, oncePerUniqueArg } from '../utilities/functionUtils'
 import {
     isInDevEnv,
     extensionVersion,
@@ -329,7 +329,7 @@ function validateMetadata(metricName: string, metadata: MetadataObj, fatal: bool
         ' Consider using `.run()` instead of `.emit()`, which will set these properties automatically. ' +
         'See https://github.com/aws/aws-toolkit-vscode/blob/master/docs/telemetry.md#guidelines'
     const msgPrefix = 'invalid Metric: '
-    const logger = getLogger('telemetry')
+    const logger = getTelemetryLogger()
     const logOrThrow = (msg: string, includeSuffix: boolean) => {
         const fullMsg = msgPrefix + msg + (includeSuffix ? preferRunSuffix : '')
         logger.warn(fullMsg)
@@ -346,10 +346,16 @@ function validateMetadata(metricName: string, metadata: MetadataObj, fatal: bool
 
     // TODO: there are many instances in the toolkit where we emit metrics with missing fields. If those can be removed, we can configure this to throw in CI.
     if (metadata.missingFields) {
-        logger.warn(msgPrefix + `"${metricName} emitted with missing fields: ${metadata.missingFields}`, false)
+        logWarningOnce(`"${metricName}" emitted with missing fields: ${metadata.missingFields}`)
     }
 }
 
+function getTelemetryLogger() {
+    return getLogger('telemetry')
+}
+
+const logWarningOnce = oncePerUniqueArg((m: string) => getTelemetryLogger().warn(m))
+
 /**
  * Potentially helpful values for the 'source' field in telemetry.
  */

packages/core/src/shared/utilities/collectionUtils.ts

@@ -580,25 +580,25 @@ export function isPresent<T>(value: T | undefined): value is T {
     return value !== undefined
 }
 
-export class CircularBuffer {
-    private buffer = new Set<number>()
+export class CircularBuffer<T> {
+    private buffer = new Set<T>()
     private maxSize: number
 
     constructor(size: number) {
         this.maxSize = size
     }
 
-    add(value: number): void {
+    add(value: T): void {
         if (this.buffer.size >= this.maxSize) {
             // Set iterates its keys in insertion-order.
             // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set
             const firstKey = this.buffer.keys().next().value
-            this.buffer.delete(firstKey)
+            this.buffer.delete(firstKey as T)
         }
         this.buffer.add(value)
     }
 
-    contains(value: number): boolean {
+    contains(value: T): boolean {
         return this.buffer.has(value)
     }
 

packages/core/src/shared/utilities/functionUtils.ts

@@ -3,6 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import { CircularBuffer } from './collectionUtils'
 import { Timeout } from './timeoutUtils'
 
 /**
@@ -156,3 +157,41 @@ export function keyedDebounce<T, U extends any[], K extends string = string>(
         return promise
     }
 }
+/**
+ * Creates a function that runs only for unique arguments that haven't been seen before.
+ *
+ * This utility tracks all unique inputs it has seen and only executes the callback
+ * for new inputs. Unlike `onceChanged` which only compares with the previous invocation,
+ * this function maintains a history of all arguments it has processed.
+ *
+ * @param fn The function to execute for unique arguments
+ * @param overflow The maximum number of unique arguments to store.
+ * @returns A wrapped function that only executes for new unique arguments
+ *
+ * @example
+ * ```ts
+ * const logOnce = oncePerUniqueArg((message) => console.log(message))
+ *
+ * logOnce('hello') // prints: hello
+ * logOnce('world') // prints: world
+ * logOnce('hello') // nothing happens (already seen)
+ * logOnce('test')  // prints: test
+ * ```
+ */
+export function oncePerUniqueArg<T, U extends any[]>(
+    fn: (...args: U) => T,
+    overflow?: number
+): (...args: U) => T | undefined {
+    const seen = new CircularBuffer(overflow ?? 1000)
+
+    return (...args) => {
+        const signature = args.map(String).join(':')
+
+        if (!seen.contains(signature)) {
+            seen.add(signature)
+            return fn(...args)
+        }
+
+        return undefined
+    }
+}

packages/core/src/shared/utilities/processUtils.ts

@@ -74,7 +74,7 @@ export class ChildProcessTracker {
         cpu: 50,
     }
     static readonly logger = logger.getLogger('childProcess')
-    static readonly loggedPids = new CircularBuffer(1000)
+    static readonly loggedPids = new CircularBuffer<number>(1000)
     #processByPid: Map<number, ChildProcess> = new Map<number, ChildProcess>()
     #pids: PollingSet<number>
 

packages/core/src/test/shared/utilities/functionUtils.test.ts

@@ -4,7 +4,7 @@
  */
 
 import assert from 'assert'
-import { once, onceChanged, debounce } from '../../../shared/utilities/functionUtils'
+import { once, onceChanged, debounce, oncePerUniqueArg } from '../../../shared/utilities/functionUtils'
 import { installFakeClock } from '../../testUtil'
 
 describe('functionUtils', function () {
@@ -48,6 +48,65 @@ describe('functionUtils', function () {
         fn('arg1', arg2_)
         assert.strictEqual(counter, 3)
     })
+
+    it('oncePerUniqueArg()', function () {
+        let counter = 0
+        const fn = oncePerUniqueArg((s: string) => {
+            counter++
+            return `processed-${s}`
+        })
+
+        // First call with unique arg should execute
+        const result1 = fn('hello')
+        assert.strictEqual(result1, 'processed-hello')
+        assert.strictEqual(counter, 1)
+
+        // Second call with same arg should not execute
+        const result2 = fn('hello')
+        assert.strictEqual(result2, undefined)
+        assert.strictEqual(counter, 1)
+
+        // Call with new arg should execute
+        const result3 = fn('world')
+        assert.strictEqual(result3, 'processed-world')
+        assert.strictEqual(counter, 2)
+
+        // Repeated calls with seen args should not execute
+        fn('hello')
+        fn('world')
+        assert.strictEqual(counter, 2)
+
+        // New arg should execute
+        const result4 = fn('test')
+        assert.strictEqual(result4, 'processed-test')
+        assert.strictEqual(counter, 3)
+    })
+
+    it('oncePerUniqueArg() with overflow limit', function () {
+        let counter = 0
+        // Create function with small overflow limit
+        const fn = oncePerUniqueArg((s: string) => {
+            counter++
+            return counter
+        }, 2)
+
+        // Fill the buffer
+        fn('one')
+        fn('two')
+        assert.strictEqual(counter, 2)
+
+        // This should execute since it's a new value
+        fn('three')
+        assert.strictEqual(counter, 3)
+
+        // 'one' should now be treated as new again since it was evicted
+        fn('one')
+        assert.strictEqual(counter, 4, 'one should still be in the buffer')
+
+        // 'three' should still be in the buffer (not executed)
+        fn('three')
+        assert.strictEqual(counter, 4, 'three should still be in the buffer')
+    })
 })
 
 describe('debounce', function () {