Update telemetry: emit auth scopes in user state #4944

samgst-amazon · 2024-10-07T21:29:51Z

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Description

Changed the emitUserState function to add authScopes to the telemetry record. Scopes were previously not recorded with this metric.

Checklist

My code follows the code style of this project
I have added tests to cover my changes
A short description of the change has been added to the CHANGELOG if the change is customer-facing in the IDE.
I have added metrics for my changes (if required)

License

I confirm that my contribution is made under the terms of the Apache 2.0 license.

…on to capture authScopes

github-actions · 2024-10-07T21:46:55Z

Qodana Community for JVM

4 new problems were found

Inspection name	Severity	Problems
`Unused import directive`	🔶 Warning	2
`Usage of redundant or deprecated syntax or deprecated symbols`	🔶 Warning	1
`Variable declaration could be moved inside 'when'`	◽️ Notice	1

💡 Qodana analysis was run in the pull request mode: only the changed files were checked
☁️ View the detailed Qodana report

Contact Qodana team

Contact us at [email protected]

Or via our issue tracker: https://jb.gg/qodana-issue
Or share your feedback: https://jb.gg/qodana-discussions

samgst-amazon · 2024-10-07T22:10:54Z

The userState() function in AuthTelemetry did not have a parameter for authScopes-- hence just calling the TelemetryService function directly to add that datum in place. That's how VSCode was doing it

...community/src/software/aws/toolkits/jetbrains/core/gettingstarted/GettingStartedAuthUtils.kt

rli · 2024-10-08T02:25:28Z

...community/src/software/aws/toolkits/jetbrains/core/gettingstarted/GettingStartedAuthUtils.kt

+            createTime(Instant.now())
+            unit(Unit.NONE)
+            value(1.0)


these are implicit

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.

This metric is currently in telemetryOverride. We need to use the one from common.

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.

rli · 2024-10-08T02:27:31Z

...community/src/software/aws/toolkits/jetbrains/core/gettingstarted/GettingStartedAuthUtils.kt

+    val scopes = ToolkitAuthManager.getInstance().listConnections().flatMap { connection ->
+        when (connection) {
+            is ProfileSsoManagedBearerSsoConnection -> connection.scopes
+            is LegacyManagedBearerSsoConnection -> connection.scopes
+            else -> emptyList()
+        }
+    }.toSet()


honestly it seems odd that we inspect every connection instead of what the extension is actively using, which seems more useful

VScode seems to be doing this same thing: grabbing the scopes from every SSO connection returned from listConnections so that's why I did it this way.

https://github.com/aws/aws-toolkit-vscode/blob/de9d7cfcd8930079bc4d9e105323ab64cf843302/packages/core/src/extension.ts#L288

…brains/core/gettingstarted/GettingStartedAuthUtils.kt Co-authored-by: Richard Li <[email protected]>

samgst-amazon · 2024-10-15T23:25:19Z

...community/src/software/aws/toolkits/jetbrains/core/gettingstarted/GettingStartedAuthUtils.kt

-        authStatus = getAuthStatus(project),
-        passive = true
-    )
+    val explorerConnection = ToolkitConnectionManager.getInstance(project).activeConnection()


Right now, I am just grabbing the connection being used by the explorer. I see that the activeConnection() function is marked as deprecated, but there is no alternative for getting the explorer connection from what I've seen. The getActiveConnectionForFeature function seems to work only for Q, CodeCatalyst, and CodeWhisperer.

If we want to look at all the active connections, I think it would be easy enough to use a function like:

aws-toolkit-jetbrains/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/gettingstarted/editor/GettingStartedTelemetryUtils.kt

Line 23 in 80abea5

fun getEnabledConnectionsForTelemetry(project: Project?): Set<AuthFormId> {

to get the scopes from each active connection. If we are looking at enabled connections in this Telemetry metric, why not the scopes for those enabled connections?

…onnected/expired.

github-actions · 2024-10-16T17:16:32Z

Qodana Community for JVM

It seems all right 👌

No new problems were found according to the checks applied

💡 Qodana analysis was run in the pull request mode: only the changed files were checked
☁️ View the detailed Qodana report

Contact Qodana team

Contact us at [email protected]

Or via our issue tracker: https://jb.gg/qodana-issue
Or share your feedback: https://jb.gg/qodana-discussions

manodnyab · 2024-10-17T20:11:00Z

...community/src/software/aws/toolkits/jetbrains/core/gettingstarted/GettingStartedAuthUtils.kt

+            createTime(Instant.now())
+            unit(Unit.NONE)
+            value(1.0)


This metric is currently in telemetryOverride. We need to use the one from common.

manodnyab · 2024-10-17T20:13:03Z

...c/software/aws/toolkits/jetbrains/core/gettingstarted/editor/GettingStartedTelemetryUtils.kt

+    val codeCatalystConnection = checkBearerConnectionValidity(project, BearerTokenFeatureSet.CODECATALYST)
+    addScopes(codeCatalystConnection)
+
+    val codeWhispererConnection = checkBearerConnectionValidity(project, BearerTokenFeatureSet.CODEWHISPERER)


I wonder if we still need this since Q or we should just record Q

From Dhruvi: " let's not be concerned about old customers since most of them have migrated"

I'll remove it.

manodnyab · 2024-10-17T20:20:15Z

...c/software/aws/toolkits/jetbrains/core/gettingstarted/editor/GettingStartedTelemetryUtils.kt

+        }
+    }
+
+    val explorerConnection = checkIamConnectionValidity(project)


We could also probably check the connection type and use the addScopes function only if the connection type is IDC. We can probably just hardcode it anyway

Check the connection type of the explorer only? Would that not ignore the other connections/scopes available if those other features are using the IdC connection type while the explorer isnt?

…on instead

plugins/core/jetbrains-community/resources/telemetryOverride.json

samgst-amazon · 2024-10-18T22:57:24Z

...y/src/software/aws/toolkits/jetbrains/core/gettingstarted/editor/GettingStartedPanelUtils.kt

+        ActiveConnection.ValidIam(connectionType = connectionType, activeConnectionIam = currConn)
+    }
+}
+


Wrote this because the other function was using isCredentialSSO would always return IAM, even in cases where it should be IAM_IDC.

The other function is for the getting started auth which contains a dialog that accepts a session name.
Where does this function determine the credential type?

I saw that there was a CredentialType enum attached to the selected credentials so I thought I could use that. If the credentialType is an SsoProfile, does that not mean it is part of an IDC session?

…Scopes

manodnyab · 2024-10-31T20:57:13Z

...c/software/aws/toolkits/jetbrains/core/gettingstarted/editor/GettingStartedTelemetryUtils.kt

 fun getEnabledConnections(project: Project?): String =
    getEnabledConnectionsForTelemetry(project).joinToString(",")

+fun getAuthScopesForTelemetry(project: Project?): Set<String> {


The function above repeats some of the logic used here, can we move the common logic to a function?

manodnyab · 2024-10-31T20:57:19Z

...y/src/software/aws/toolkits/jetbrains/core/gettingstarted/editor/GettingStartedPanelUtils.kt

+        ActiveConnection.ValidIam(connectionType = connectionType, activeConnectionIam = currConn)
+    }
+}
+


The other function is for the getting started auth which contains a dialog that accepts a session name.
Where does this function determine the credential type?

missed {

* add authScopes to emitUserState telemetry * add authScopes to emitUserState telemetry * Replace userState function with direct TelemetryService record function to capture authScopes * add value parameter * fix whitespace * Update plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/gettingstarted/GettingStartedAuthUtils.kt Co-authored-by: Richard Li <[email protected]> * Check current explorerConnection for scopes only * Add function to grab scopes from each activeConnection used that is connected/expired. * remove TelemetryOverride definition and use common's telemetry function instead * remove unused imports * Change the way explorer connection scope is checked * remove check for CodeWhispererConnection: merged into qConnection & qScopes * add common logic to one function * Update telemetryOverride.json missed { --------- Co-authored-by: Richard Li <[email protected]>

samgst-amazon added 3 commits October 7, 2024 11:56

add authScopes to emitUserState telemetry

f157c53

add authScopes to emitUserState telemetry

f0d455d

Replace userState function with direct TelemetryService record functi…

924a2b0

…on to capture authScopes

samgst-amazon changed the title ~~Telemetry: emit auth scopes in user state~~ Update telemetry: emit auth scopes in user state Oct 7, 2024

samgst-amazon added 2 commits October 7, 2024 14:56

add value parameter

4512625

fix whitespace

a4db555

samgst-amazon marked this pull request as ready for review October 7, 2024 22:20

samgst-amazon requested a review from a team as a code owner October 7, 2024 22:20

rli approved these changes Oct 8, 2024

View reviewed changes

samgst-amazon and others added 2 commits October 8, 2024 11:06

Update plugins/core/jetbrains-community/src/software/aws/toolkits/jet…

4e1af21

…brains/core/gettingstarted/GettingStartedAuthUtils.kt Co-authored-by: Richard Li <[email protected]>

Check current explorerConnection for scopes only

df30b58

samgst-amazon commented Oct 15, 2024

View reviewed changes

Add function to grab scopes from each activeConnection used that is c…

2d053c5

…onnected/expired.

manodnyab reviewed Oct 17, 2024

View reviewed changes

samgst-amazon added 2 commits October 18, 2024 10:56

remove TelemetryOverride definition and use common's telemetry functi…

13c8cd0

…on instead

remove unused imports

d05315f

samgst-amazon commented Oct 18, 2024

View reviewed changes

plugins/core/jetbrains-community/resources/telemetryOverride.json

This comment was marked as outdated.

Sign in to view

Change the way explorer connection scope is checked

744a2d3

samgst-amazon commented Oct 18, 2024

View reviewed changes

remove check for CodeWhispererConnection: merged into qConnection & q…

098fa1d

…Scopes

manodnyab reviewed Oct 31, 2024

View reviewed changes

samgst-amazon added 6 commits November 1, 2024 13:00

add common logic to one function

e6082fe

Merge branch 'main' into samgst/emitAuthScopesInUserState

214f4d8

Merge branch 'main' into samgst/emitAuthScopesInUserState

e3e4838

Merge branch 'main' into samgst/emitAuthScopesInUserState

5f780a1

Merge branch 'main' into samgst/emitAuthScopesInUserState

1404bb1

Merge branch 'main' into samgst/emitAuthScopesInUserState

82708b7

samgst-amazon added 2 commits November 7, 2024 12:15

Update telemetryOverride.json

3ade8d8

missed {

Merge branch 'main' into samgst/emitAuthScopesInUserState

0858d1b

samgst-amazon merged commit 82f2275 into main Nov 7, 2024
16 of 17 checks passed

samgst-amazon deleted the samgst/emitAuthScopesInUserState branch November 7, 2024 21:14

Update telemetry: emit auth scopes in user state #4944

Update telemetry: emit auth scopes in user state #4944

Uh oh!

Conversation

samgst-amazon commented Oct 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Types of changes

Description

Checklist

License

Uh oh!

github-actions bot commented Oct 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Qodana Community for JVM

Uh oh!

samgst-amazon commented Oct 7, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Qodana Community for JVM

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

samgst-amazon commented Oct 7, 2024 •

edited

Loading

github-actions bot commented Oct 7, 2024 •

edited

Loading

github-actions bot commented Oct 16, 2024 •

edited

Loading