Skip to content

Commit

Permalink
telemetry(auth): debounce aws_loadCredentials and add debugging inf…
Browse files Browse the repository at this point in the history
…ormation (#6499)

## Problem
This metric is currently being spammed in telemetry at unusual volume.
It is suspected that this is connected to this change:
https://github.com/aws/aws-toolkit-vscode/pull/5979/files, but the
connection is not yet clear.

## Solution
- use the `withTelemetryContext` decorator to emit a `function_call`
metric to capture additional information. This metric should allow us to
determine what is calling this function so much. Added techdebt test to
remove this debugging information before the following release.
- debounce the current emit line that is causing the high volume
emission.


## Future Work 
- `debounce` reimplements a special case of `debounceWithCancel`, these
can be redone to reduce code and improve reliability.

---

- Treat all work as PUBLIC. Private `feature/x` branches will not be
squash-merged at release time.
- Your code changes must meet the guidelines in
[CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).
- License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.
  • Loading branch information
Hweinstock authored Feb 5, 2025
1 parent 69db397 commit 100eb07
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 6 deletions.
1 change: 1 addition & 0 deletions packages/core/src/auth/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ export class Auth implements AuthService, ConnectionManager {
*
* Use {@link Auth.listConnections} to avoid API calls to the SSO service.
*/
@withTelemetryContext({ name: 'listAndTraverseConnections', class: authClassName })
public listAndTraverseConnections(): AsyncCollection<Connection> {
async function* load(this: Auth) {
await loadIamProfilesIntoStore(this.store, this.iamProfileProvider)
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/auth/deprecated/loginManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ import * as localizedText from '../../shared/localizedText'
import { DefaultStsClient } from '../../shared/clients/stsClient'
import { findAsync } from '../../shared/utilities/collectionUtils'
import { telemetry } from '../../shared/telemetry/telemetry'
import { withTelemetryContext } from '../../shared/telemetry/util'

const loginManagerClassName = 'LoginManager'
/**
* @deprecated Replaced by `Auth` in `src/credentials/auth.ts`
*/
Expand Down Expand Up @@ -131,6 +133,7 @@ export class LoginManager {

private static didTryAutoConnect = false

@withTelemetryContext({ name: 'tryAutoConnect', class: loginManagerClassName })
public static async tryAutoConnect(awsContext: AwsContext = globals.awsContext): Promise<boolean> {
if (isAutomation()) {
return false
Expand Down
27 changes: 21 additions & 6 deletions packages/core/src/auth/providers/credentialsProviderManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
*/

import { getLogger } from '../../shared/logger'
import { telemetry } from '../../shared/telemetry/telemetry'
import { AwsLoadCredentials, telemetry } from '../../shared/telemetry/telemetry'
import { withTelemetryContext } from '../../shared/telemetry/util'
import { cancellableDebounce } from '../../shared/utilities/functionUtils'
import {
asString,
CredentialsProvider,
Expand All @@ -14,6 +16,7 @@ import {
} from './credentials'
import { CredentialsProviderFactory } from './credentialsProviderFactory'

const credentialsProviderManagerClassName = 'CredentialsProviderManager'
/**
* Responsible for providing the Toolkit with all available CredentialsProviders.
* Providers may be registered directly or created with supplied CredentialsProviderFactories.
Expand All @@ -23,13 +26,18 @@ export class CredentialsProviderManager {
private readonly providerFactories: CredentialsProviderFactory[] = []
private readonly providers: CredentialsProvider[] = []

@withTelemetryContext({ name: 'getAllCredentialsProvider', class: credentialsProviderManagerClassName, emit: true })
public async getAllCredentialsProviders(): Promise<CredentialsProvider[]> {
let providers: CredentialsProvider[] = []

for (const provider of this.providers) {
if (await provider.isAvailable()) {
const telemType = credentialsProviderToTelemetryType(provider.getCredentialsId().credentialSource)
telemetry.aws_loadCredentials.emit({ credentialSourceId: telemType, value: 1 })
telemetry.aws_loadCredentials.emit({
credentialSourceId: credentialsProviderToTelemetryType(
provider.getCredentialsId().credentialSource
),
value: 1,
})
providers = providers.concat(provider)
} else {
getLogger().verbose('auth: "%s" provider unavailable', provider.getCredentialsId().credentialTypeId)
Expand All @@ -43,18 +51,25 @@ export class CredentialsProviderManager {
if (!providerType) {
continue
}
const telemType = credentialsProviderToTelemetryType(providerType)
telemetry.aws_loadCredentials.emit({ credentialSourceId: telemType, value: refreshed.length })

void this.emitWithDebounce({
credentialSourceId: credentialsProviderToTelemetryType(providerType),
value: refreshed.length,
})
providers = providers.concat(refreshed)
}

return providers
}

private emitWithDebounce = cancellableDebounce(
(m: AwsLoadCredentials) => telemetry.aws_loadCredentials.emit(m),
100
).promise
/**
* Returns a map of `CredentialsProviderId` string-forms to object-forms,
* from all credential sources. Only available providers are returned.
*/
@withTelemetryContext({ name: 'getCredentialProviderNames', class: credentialsProviderManagerClassName })
public async getCredentialProviderNames(): Promise<{ [key: string]: CredentialsId }> {
const m: { [key: string]: CredentialsId } = {}
for (const o of await this.getAllCredentialsProviders()) {
Expand Down
7 changes: 7 additions & 0 deletions packages/core/src/test/techdebt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,11 @@ describe('tech debt', function () {
// This is relevant for the use of `fs.cpSync` in the copyFiles scripts.
assert.ok(semver.lt(minNodejs, '18.0.0'), 'with node18+, we can remove the dependency on @types/node@18')
})

it('remove debugging telemetry', async function () {
fixByDate(
'2025-02-11',
'Remove debugging telemetry in `packages/core/src/auth/providers/credentialsProviderManager.ts`. Should only need to remove the `emit: true` in the decorator.'
)
})
})

0 comments on commit 100eb07

Please sign in to comment.