Skip to content

Internal Bugfix - add telemetry guards for string overflow #4538

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open
2 changes: 1 addition & 1 deletion .idea/copyright/apache.xml
Copy link
Contributor

Choose a reason for hiding this comment

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

don't need to fix, but in future avoid unnecessary diffs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion .idea/copyright/profiles_settings.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import software.aws.toolkits.jetbrains.services.codemodernizer.model.CodeModerni
import software.aws.toolkits.jetbrains.services.codemodernizer.model.CodeModernizerSessionContext
import software.aws.toolkits.jetbrains.services.codemodernizer.model.CodeModernizerStartJobResult
import software.aws.toolkits.jetbrains.services.codemodernizer.model.CodeTransformHilDownloadArtifact
import software.aws.toolkits.jetbrains.services.codemodernizer.model.CodeTransformTelemetryService
import software.aws.toolkits.jetbrains.services.codemodernizer.model.CustomerSelection
import software.aws.toolkits.jetbrains.services.codemodernizer.model.Dependency
import software.aws.toolkits.jetbrains.services.codemodernizer.model.InvalidTelemetryReason
Expand Down Expand Up @@ -120,6 +121,7 @@ class CodeModernizerManager(private val project: Project) : PersistentStateCompo
)
init {
CodeModernizerSessionState.getInstance(project).setDefaults()
CodeTransformTelemetryService.getInstance().resetDefaults()
}

fun validate(project: Project): ValidationResult {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ package software.aws.toolkits.jetbrains.services.codemodernizer
import com.intellij.openapi.components.Service
import com.intellij.openapi.components.service
import com.intellij.openapi.project.Project
import kotlinx.serialization.encodeToString
import org.apache.commons.codec.digest.DigestUtils
import software.amazon.awssdk.services.codewhispererruntime.model.TransformationStatus
import software.aws.toolkits.jetbrains.core.gettingstarted.editor.ActiveConnection
import software.aws.toolkits.jetbrains.core.gettingstarted.editor.ActiveConnectionType
import software.aws.toolkits.jetbrains.core.gettingstarted.editor.BearerTokenFeatureSet
import software.aws.toolkits.jetbrains.core.gettingstarted.editor.checkBearerConnectionValidity
import software.aws.toolkits.jetbrains.services.codemodernizer.model.CodeTransformTelemetryMetadata
import software.aws.toolkits.jetbrains.services.codemodernizer.model.CustomerSelection
import software.aws.toolkits.jetbrains.services.codemodernizer.model.JobId
import software.aws.toolkits.jetbrains.services.codemodernizer.model.ValidationResult
Expand Down Expand Up @@ -196,11 +198,11 @@ class CodeTransformTelemetryManager(private val project: Project) {
)
}

fun logHil(jobId: String, metaData: HilTelemetryMetaData, success: Boolean, reason: String) {
fun logHil(jobId: String, metaData: CodeTransformTelemetryMetadata, success: Boolean, reason: String) {
CodetransformTelemetry.humanInTheLoop(
project,
jobId,
metaData.toString(),
metaData.toJsonString(),
sessionId,
reason,
success,
Expand All @@ -223,8 +225,3 @@ class CodeTransformTelemetryManager(private val project: Project) {
fun getInstance(project: Project): CodeTransformTelemetryManager = project.service()
}
}

data class HilTelemetryMetaData(
val dependencyVersionSelected: String? = null,
val cancelledFromChat: Boolean = false,
)
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import software.aws.toolkits.jetbrains.services.amazonq.auth.AuthController
import software.aws.toolkits.jetbrains.services.codemodernizer.ArtifactHandler
import software.aws.toolkits.jetbrains.services.codemodernizer.CodeModernizerManager
import software.aws.toolkits.jetbrains.services.codemodernizer.CodeTransformTelemetryManager
import software.aws.toolkits.jetbrains.services.codemodernizer.HilTelemetryMetaData
import software.aws.toolkits.jetbrains.services.codemodernizer.InboundAppMessagesHandler
import software.aws.toolkits.jetbrains.services.codemodernizer.client.GumbyClient
import software.aws.toolkits.jetbrains.services.codemodernizer.commands.CodeTransformActionMessage
Expand Down Expand Up @@ -59,6 +58,7 @@ import software.aws.toolkits.jetbrains.services.codemodernizer.messages.CodeTran
import software.aws.toolkits.jetbrains.services.codemodernizer.messages.IncomingCodeTransformMessage
import software.aws.toolkits.jetbrains.services.codemodernizer.model.CodeModernizerJobCompletedResult
import software.aws.toolkits.jetbrains.services.codemodernizer.model.CodeTransformHilDownloadArtifact
import software.aws.toolkits.jetbrains.services.codemodernizer.model.CodeTransformTelemetryService
import software.aws.toolkits.jetbrains.services.codemodernizer.model.CustomerSelection
import software.aws.toolkits.jetbrains.services.codemodernizer.model.DownloadFailureReason
import software.aws.toolkits.jetbrains.services.codemodernizer.model.JobId
Expand Down Expand Up @@ -418,11 +418,10 @@ class CodeTransformChatController(

codeModernizerManager.resumePollingFromHil()
} catch (e: Exception) {
CodeTransformTelemetryService.setCancelledFromChat(false)
telemetry.logHil(
CodeModernizerSessionState.getInstance(context.project).currentJobId?.id.orEmpty(),
HilTelemetryMetaData(
cancelledFromChat = false,
),
CodeTransformTelemetryService.getInstance(),
success = false,
reason = "Runtime Error when trying to resume transformation from HIL",
)
Expand Down Expand Up @@ -511,12 +510,10 @@ class CodeTransformChatController(

try {
codeModernizerManager.tryResumeWithAlternativeVersion(selectedVersion)

CodeTransformTelemetryService.setDependencyVersionSelected(selectedVersion)
telemetry.logHil(
CodeModernizerSessionState.getInstance(context.project).currentJobId?.id as String,
HilTelemetryMetaData(
dependencyVersionSelected = selectedVersion,
),
CodeTransformTelemetryService.getInstance(),
success = true,
reason = "User selected version"
)
Expand All @@ -543,12 +540,10 @@ class CodeTransformChatController(

try {
codeModernizerManager.rejectHil()

CodeTransformTelemetryService.setCancelledFromChat(true)
telemetry.logHil(
CodeModernizerSessionState.getInstance(context.project).currentJobId?.id.orEmpty(),
HilTelemetryMetaData(
cancelledFromChat = true,
),
CodeTransformTelemetryService.getInstance(),
success = false,
reason = "User cancelled"
)
Expand All @@ -558,11 +553,10 @@ class CodeTransformChatController(
}
codeModernizerManager.resumePollingFromHil()
} catch (e: Exception) {
CodeTransformTelemetryService.setCancelledFromChat(false)
telemetry.logHil(
CodeModernizerSessionState.getInstance(context.project).currentJobId?.id.orEmpty(),
HilTelemetryMetaData(
cancelledFromChat = false,
),
CodeTransformTelemetryService.getInstance(),
success = false,
reason = "Runtime Error when trying to resume transformation from HIL",
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package software.aws.toolkits.jetbrains.services.codemodernizer.model

import com.fasterxml.jackson.databind.module.SimpleModule
import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper

const val CODETRANSFORM_METADATA_MAX_STRINGIFIED_LENGTH = 65536

data class CodeTransformTelemetryMetadata(
var dependencyVersionSelected: String? = null,
var cancelledFromChat: Boolean = false,
) {
private val propertyValues = listOf(
"dependencyVersionSelected" to dependencyVersionSelected,
"cancelledFromChat" to cancelledFromChat
)

operator fun iterator(): Iterator<Pair<String, Any?>> = propertyValues.iterator()

fun toJsonString(): String {
var trimmedJsonString = trimJsonString(CODETRANSFORM_METADATA_MAX_STRINGIFIED_LENGTH)
return trimmedJsonString
}

fun resetDefaults() {
dependencyVersionSelected = null
cancelledFromChat = false
}

/**
* @description We have a truncation function for all fields to be less than 1000 characters.
* If this fails, we try to completely remove fields to limit the size sent to backend to prevent
* an overflow when submitting data.
*/
private fun trimJsonString(maxLength: Int): String {
val objectMapper = jacksonObjectMapper()
objectMapper.registerModule(
SimpleModule().addSerializer(String::class.java, MaxLengthTelemetryStringSerializer())
)
val jsonString = objectMapper.writeValueAsString(this)
if (jsonString.length <= maxLength) {
return jsonString
}

val trimmedPropertyValues = mutableListOf<Pair<String, Any?>>()
var currentLength = 0
for ((key, value) in propertyValues) {
val elementLength = key.length + value.toString().length + 5 // add 5 for quotes and comma around key-value pairs
if (currentLength + elementLength <= maxLength) {
trimmedPropertyValues.add(Pair(key, value))
currentLength += elementLength
}
// else we omit the key/value pair as a way of "trimming" the object that is too large
}

return objectMapper.writeValueAsString(trimmedPropertyValues.toMap())
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package software.aws.toolkits.jetbrains.services.codemodernizer.model

import com.intellij.openapi.components.Service

@Service(Service.Level.APP)
public final class CodeTransformTelemetryService {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rli Should I drop the the instance and getInstance and just have my metadata a member of my telemetry class?

It looks like the service docs are implying that it will find the service level singelton if existed when calling my class like

service<CodeTransformTelemetryService>()
Screenshot 2024-06-11 at 4 05 26 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

instance for sure
getInstance is just a convenience to save needing to import two things every time. you are the only consumer of the service so it can go either way

companion object {
private val instance = CodeTransformTelemetryMetadata()

fun getInstance() = instance

fun setDependencyVersionSelected(version: String?) {
instance.dependencyVersionSelected = version
}

fun setCancelledFromChat(cancelled: Boolean) {
instance.cancelledFromChat = cancelled
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package software.aws.toolkits.jetbrains.services.codemodernizer.model

import com.fasterxml.jackson.core.JsonGenerator
import com.fasterxml.jackson.databind.JsonSerializer
import com.fasterxml.jackson.databind.SerializerProvider

const val MAX_SERIALIZABLE_STRING_LENGTH = 1000
class MaxLengthTelemetryStringSerializer : JsonSerializer<String>() {
override fun serialize(value: String, gen: JsonGenerator, provider: SerializerProvider) {
val truncatedValue = if (value.length > MAX_SERIALIZABLE_STRING_LENGTH) {
value.substring(0, MAX_SERIALIZABLE_STRING_LENGTH)
} else {
value
}
gen.writeString(truncatedValue)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package software.aws.toolkits.jetbrains.services.codemodernizer.model

import com.intellij.testFramework.ApplicationRule
import junit.framework.TestCase.assertEquals
import org.junit.Rule
import org.junit.Test

open class CodeTransformTelemetryServiceTest {
@Rule
@JvmField
val applicationRule = ApplicationRule()

@Test
fun `CodeTransformTelemetryMetadata will set values and resets defaults properly`() {
CodeTransformTelemetryService.setDependencyVersionSelected("1.2.3")
CodeTransformTelemetryService.setCancelledFromChat(true)
assertEquals(CodeTransformTelemetryService.getInstance().dependencyVersionSelected, "1.2.3")
assertEquals(CodeTransformTelemetryService.getInstance().cancelledFromChat, true)

// check reset defaults works
CodeTransformTelemetryService.getInstance().resetDefaults()
assertEquals(CodeTransformTelemetryService.getInstance().dependencyVersionSelected, null)
assertEquals(CodeTransformTelemetryService.getInstance().cancelledFromChat, false)
}

@Test
fun `CodeTransformTelemetryMetadataSingletonTest toJsonString() will serialize object correctly`() {
CodeTransformTelemetryService.setDependencyVersionSelected("1.2.3")
CodeTransformTelemetryService.setCancelledFromChat(true)
val expectedJsonString = """{"dependencyVersionSelected":"1.2.3","cancelledFromChat":true}"""
assertEquals(expectedJsonString, CodeTransformTelemetryService.getInstance().toJsonString())
}

@Test
fun `CodeTransformTelemetryMetadataSingletonTest trimJsonString() trims single field JSON string to specified length`() {
val longString = "a".repeat(CODETRANSFORM_METADATA_MAX_STRINGIFIED_LENGTH)
CodeTransformTelemetryService.setDependencyVersionSelected(longString)
CodeTransformTelemetryService.setCancelledFromChat(true)

val expectedTrimmedJsonString = """{"dependencyVersionSelected":"${"a".repeat(MAX_SERIALIZABLE_STRING_LENGTH)}","cancelledFromChat":true}"""
assertEquals(expectedTrimmedJsonString, CodeTransformTelemetryService.getInstance().toJsonString())
}
}
Loading