Skip to content

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.

Sign up for GitHub

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

Merged
merged 20 commits into from
Nov 7, 2024
Merged

Update telemetry: emit auth scopes in user state #4944

Changes from 7 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
f157c53
add authScopes to emitUserState telemetry
samgst-amazon Oct 7, 2024
f0d455d
add authScopes to emitUserState telemetry
samgst-amazon Oct 7, 2024
924a2b0
Replace userState function with direct TelemetryService record functi…
samgst-amazon Oct 7, 2024
4512625
add value parameter
samgst-amazon Oct 7, 2024
a4db555
fix whitespace
samgst-amazon Oct 7, 2024
4e1af21
Update plugins/core/jetbrains-community/src/software/aws/toolkits/jet…
samgst-amazon Oct 8, 2024
df30b58
Check current explorerConnection for scopes only
samgst-amazon Oct 15, 2024
2d053c5
Add function to grab scopes from each activeConnection used that is c…
samgst-amazon Oct 16, 2024
13c8cd0
remove TelemetryOverride definition and use common's telemetry functi…
samgst-amazon Oct 18, 2024
d05315f
remove unused imports
samgst-amazon Oct 18, 2024
744a2d3
Change the way explorer connection scope is checked
samgst-amazon Oct 18, 2024
098fa1d
remove check for CodeWhispererConnection: merged into qConnection & q…
samgst-amazon Oct 21, 2024
e6082fe
add common logic to one function
samgst-amazon Nov 1, 2024
214f4d8
Merge branch 'main' into samgst/emitAuthScopesInUserState
samgst-amazon Nov 4, 2024
e3e4838
Merge branch 'main' into samgst/emitAuthScopesInUserState
samgst-amazon Nov 4, 2024
5f780a1
Merge branch 'main' into samgst/emitAuthScopesInUserState
samgst-amazon Nov 5, 2024
1404bb1
Merge branch 'main' into samgst/emitAuthScopesInUserState
samgst-amazon Nov 6, 2024
82708b7
Merge branch 'main' into samgst/emitAuthScopesInUserState
samgst-amazon Nov 7, 2024
3ade8d8
Update telemetryOverride.json
samgst-amazon Nov 7, 2024
0858d1b
Merge branch 'main' into samgst/emitAuthScopesInUserState
samgst-amazon Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions ...munity/src/software/aws/toolkits/jetbrains/core/gettingstarted/GettingStartedAuthUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,14 @@
package software.aws.toolkits.jetbrains.core.gettingstarted

import com.intellij.openapi.project.Project
import software.aws.toolkits.jetbrains.services.telemetry.TelemetryService
import software.amazon.awssdk.services.toolkittelemetry.model.Unit
import software.aws.toolkits.core.utils.tryOrNull
import software.aws.toolkits.jetbrains.core.credentials.AwsConnectionManager

Check warning on line 10 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/gettingstarted/GettingStartedAuthUtils.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unused import directive 

Unused import directive
import software.aws.toolkits.jetbrains.core.credentials.LegacyManagedBearerSsoConnection
import software.aws.toolkits.jetbrains.core.credentials.ManagedBearerSsoConnection
import software.aws.toolkits.jetbrains.core.credentials.ProfileSsoManagedBearerSsoConnection
import software.aws.toolkits.jetbrains.core.credentials.ToolkitAuthManager

Check warning on line 14 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/gettingstarted/GettingStartedAuthUtils.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unused import directive 

Unused import directive
import software.aws.toolkits.jetbrains.core.credentials.ToolkitConnectionManager
import software.aws.toolkits.jetbrains.core.credentials.loginSso
import software.aws.toolkits.jetbrains.core.credentials.pinning.CodeWhispererConnection
Expand All @@ -26,6 +30,7 @@
import software.aws.toolkits.telemetry.AuthTelemetry
import software.aws.toolkits.telemetry.FeatureId
import software.aws.toolkits.telemetry.Result
import java.time.Instant

fun requestCredentialsForCodeWhisperer(
project: Project,
Expand Down Expand Up @@ -229,13 +234,26 @@
}

fun emitUserState(project: Project) {
AuthTelemetry.userState(
project,
source = getStartupState().toString(),
authEnabledConnections = getEnabledConnections(project),
authStatus = getAuthStatus(project),
passive = true
)
val explorerConnection = ToolkitConnectionManager.getInstance(project).activeConnection()

Check warning on line 237 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/gettingstarted/GettingStartedAuthUtils.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Usage of redundant or deprecated syntax or deprecated symbols 

'activeConnection(): ToolkitConnection?' is deprecated. Fragile API. Probably leads to unexpected behavior. Use only for toolkit explorer dropdown state.

Check notice on line 237 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/gettingstarted/GettingStartedAuthUtils.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Variable declaration could be moved inside 'when' 

Variable declaration could be moved into 'when'
Copy link
Contributor Author

@samgst-amazon samgst-amazon Oct 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@samgst-amazon samgst-amazon Oct 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

val scopes = when (explorerConnection) {
is ManagedBearerSsoConnection -> explorerConnection.scopes
else -> emptyList()
}

TelemetryService.getInstance().record(project) {
datum("auth_userState") {
createTime(Instant.now())
unit(Unit.NONE)
value(1.0)
Copy link
Contributor

@rli rli Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are implicit

Copy link
Contributor Author

@samgst-amazon samgst-amazon Oct 8, 2024

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.

Copy link
Contributor

@manodnyab manodnyab Oct 17, 2024

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.

Copy link
Contributor Author

@samgst-amazon samgst-amazon Oct 17, 2024

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.

passive(true)
metadata("source", getStartupState().toString())
metadata("authStatus", getAuthStatus(project).toString())
metadata("authEnabledConnections", getEnabledConnections(project))
if (getAuthStatus(project).toString() != "notConnected") {
metadata("authScopes", scopes.joinToString(","))
}
}
}
}

const val CODEWHISPERER_AUTH_LEARN_MORE_LINK = "https://docs.aws.amazon.com/codewhisperer/latest/userguide/codewhisperer-auth.html"
Loading