Handle recoverable logging errors. Report log delivery exception metrics #179
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
brianterry
approved these changes
May 16, 2022
|
this has been approved from a while back @kddejong do you think you could approve + merge this? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
n/a
Description of changes:
Handle recoverable logging errors
CreateLogGroup and CreateLogStream "ResourceAlreadyExists" exceptions are not exceptional and logging can continue. I couldn't find any documentation on TPS for CreateLogGroup. DescribeLogGroup has a TPS of 5 so it seems like something we can do away with. I've observed CreateLogStream be called twice and return ResourceAlreadyExists, presumably due to a temporary network error.
I've observed PutLogEvents returning "DataAlreadyAcceptedException" and "InvalidSequenceTokenException". Both errors provide the sequence token that should be used next. Without reading that token, the logger cannot send any more logs. I suspect "DataAlreadyAcceptedException" happens with temporary network errors that are retried. I'm not sure why "InvalidSequenceTokenException" happens.
Report log delivery exception metrics
Today, when a go resource provider runs into logging errors, you have no indication of why -- just missing logs. This change follows the java plugin by reporting LogDeliveryExceptions for each AWS call that can fail in the logging provider.
Report metric time as UTC
Metrics were rejected because time wasn't UTC. I've been running the UTC submission code for weeks now with no problem.
Upgrade go SDK
PutLogEvents errors weren't available in the sdk version previously used. I upgraded the aws sdk to latest.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.