-
Notifications
You must be signed in to change notification settings - Fork 273
Update telemetry: emit auth scopes in user state #4944
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
f157c53
f0d455d
924a2b0
4512625
a4db555
4e1af21
df30b58
2d053c5
13c8cd0
d05315f
744a2d3
098fa1d
e6082fe
214f4d8
e3e4838
5f780a1
1404bb1
82708b7
3ade8d8
0858d1b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,6 +70,37 @@ fun getEnabledConnectionsForTelemetry(project: Project?): Set<AuthFormId> { | |
| fun getEnabledConnections(project: Project?): String = | ||
| getEnabledConnectionsForTelemetry(project).joinToString(",") | ||
|
|
||
| fun getAuthScopesForTelemetry(project: Project?): Set<String> { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The function above repeats some of the logic used here, can we move the common logic to a function? |
||
| project ?: return emptySet() | ||
| val scopes = mutableSetOf<String>() | ||
|
|
||
| fun addScopes(connection: ActiveConnection) { | ||
| if (connection !is ActiveConnection.NotConnected) { | ||
| val connectionScopes = connection.activeConnectionBearer?.scopes | ||
| if (connectionScopes != null) { | ||
| scopes.addAll(connectionScopes) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| val explorerConnection = checkIamConnectionValidity(project) | ||
|
||
| addScopes(explorerConnection) | ||
|
|
||
| val codeCatalystConnection = checkBearerConnectionValidity(project, BearerTokenFeatureSet.CODECATALYST) | ||
| addScopes(codeCatalystConnection) | ||
|
|
||
| val codeWhispererConnection = checkBearerConnectionValidity(project, BearerTokenFeatureSet.CODEWHISPERER) | ||
|
||
| addScopes(codeWhispererConnection) | ||
|
|
||
| val qConnection = checkBearerConnectionValidity(project, BearerTokenFeatureSet.Q) | ||
| addScopes(qConnection) | ||
|
|
||
| return scopes | ||
| } | ||
|
|
||
| fun getAuthScopes(project: Project?): String = | ||
| getAuthScopesForTelemetry(project).joinToString(",") | ||
|
|
||
| fun getStartupState(): StartUpState { | ||
| val hasStartedToolkitBefore = tryOrNull { | ||
| getPersistentStateComponentStorageLocation(AwsSettings::class.java)?.exists() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are implicit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are implicit if you use the userState function in AuthTelemetry-- which just calls the TelemetryService function using these values. I thought I had to add them here since I was not using the userState function and they weren't being set. I did not see a method signature in AuthTelemetry that would let me pass in the extra authScopes datum-- and I wasn't sure where/if I could update the userState method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This metric is currently in telemetryOverride. We need to use the one from common.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that our AuthTelemetry is being generated. no version of AuthTelemetry.userState() accepts authScopes as a parameter. I think this might be why? :
https://github.com/aws/aws-toolkit-common/blob/49de773b65efdeaadea5c30859cf286ccd74ec5b/telemetry/definitions/commonDefinitions.json#L2708C1-L2710C19
I'll try making some changes to common locally to see if I can generate a version of the function that does.