-
Notifications
You must be signed in to change notification settings - Fork 49
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
misc: increase business metrics checks #1474
Conversation
A new generated diff is ready to view.
|
This comment has been minimized.
This comment has been minimized.
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 changes look good but let's also add a check that all strings we're concatenating into the m/
section are actually 2 characters or less. If they are, great. If they're not, we should probably log an error/warning and skip sending the business metrics.
A new generated diff is ready to view.
|
val allowedMetrics = metrics.filter { | ||
if (it.identifier.length > 2) { | ||
logger.warn { "Business metric '${it.identifier}' will be skipped due to length being > 2" } | ||
false | ||
} else { | ||
true | ||
} | ||
} |
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.
Suggestion: I really hope we can discover where these are coming from. Let's add to the log message something like "This is likely a bug. Please raise an issue at <url>".
A new generated diff is ready to view.
|
This comment has been minimized.
This comment has been minimized.
A new generated diff is ready to view.
|
This comment has been minimized.
This comment has been minimized.
Quality Gate passedIssues Measures |
A new generated diff is ready to view.
|
Affected ArtifactsChanged in size
|
These changes are related to malformed business metrics we are seeing in the
User-Agent
header.Also see: smithy-lang/smithy-kotlin#1188
Issue #
Description of changes
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.