Skip to content

add more telemetry info for NoSuchFileException failures #5087

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

Closed
wants to merge 3 commits into from
Closed

add more telemetry info for NoSuchFileException failures #5087

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
96bc69e
add telemetry info
samgst-amazon Nov 14, 2024
7dc4911
detekt
samgst-amazon Nov 14, 2024
ea45aaa
Merge branch 'main' into samgst/addTelemetryInfo
samgst-amazon Nov 14, 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
23 changes: 22 additions & 1 deletion ...jetbrains-community/src/software/aws/toolkits/jetbrains/core/credentials/sso/DiskCache.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
import com.fasterxml.jackson.module.kotlin.readValue
import software.aws.toolkits.core.utils.createParentDirectories
import software.aws.toolkits.core.utils.deleteIfExists
import software.aws.toolkits.core.utils.exists
import software.aws.toolkits.core.utils.getLogger
import software.aws.toolkits.core.utils.info
import software.aws.toolkits.core.utils.inputStreamIfExists
Expand All @@ -37,6 +38,7 @@ import software.aws.toolkits.telemetry.AuthTelemetry
import software.aws.toolkits.telemetry.Result
import java.io.InputStream
import java.io.OutputStream
import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.Paths
import java.security.MessageDigest
Expand Down Expand Up @@ -107,12 +109,31 @@ class DiskCache(
if (inputStream == null) {
val stage = LoadCredentialStage.ACCESS_FILE
LOG.info { "Failed to load Client Registration: cache file does not exist" }

val path = clientRegistrationCache(cacheKey)
Copy link
Contributor

@manodnyab manodnyab Nov 14, 2024

Choose a reason for hiding this comment

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

Can we reuse the value above?

val fileExists = path.exists()
Copy link
Contributor

@manodnyab manodnyab Nov 14, 2024

Choose a reason for hiding this comment

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

Dont we already know that the file doesn't exist here

Copy link
Contributor

@manodnyab manodnyab Nov 14, 2024

Choose a reason for hiding this comment

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

Would we get an error other than NoSuchFileException?

Copy link
Contributor Author

@samgst-amazon samgst-amazon Nov 14, 2024

Choose a reason for hiding this comment

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

I think you're right. I think the permissions errors are being handled correctly so this may not be what's needed

val parentExists = path.parent?.exists() ?: false
val parentPerms = tryOrNull { path.parent?.let { Files.getPosixFilePermissions(it).toString() } }
val isOwner = tryOrNull {
path.parent?.let {
val owner = Files.getOwner(it)
val currentUser = System.getProperty("user.name")
owner.name == currentUser
}
} ?: false

AuthTelemetry.modifyConnection(
action = "Load cache file",
source = "loadClientRegistration:$source",
result = Result.Failed,
reason = "Failed to load Client Registration",
reasonDesc = "Load Step:$stage failed. Cache file does not exist"
reasonDesc = buildString {
append("Load Step:$stage failed. ")
append("Cache file exists:$fileExists, ")
append("Parent exists:$parentExists, ")
append("Parent perms:$parentPerms, ")
append("Is owner:$isOwner")
}
)
return null
}
Expand Down
Loading