LG-13123 Log when GpoConfirmationUploader uploads GpoConfirmations#10943
Merged
LG-13123 Log when GpoConfirmationUploader uploads GpoConfirmations#10943
GpoConfirmationUploader uploads GpoConfirmations#10943Conversation
We have observed a number of issues with the `GpoConfirmationUploader`. We are adding more visibility to the job to improve our monitoring and alerting to deal with failures that come up. This commit adds a line to `events.log` when confirmations get uploaded. It includes a count of the confirmations that were uploaded so we can build a metric for that. [skip changelog]
aduth
approved these changes
Jul 16, 2024
app/services/analytics_events.rb
Outdated
|
|
||
| # @params [Boolean] success Whether records were successfully uploaded | ||
| # @params [String] exception The exception that occured if an exception did occur | ||
| # @param [Number] gpo_confirmation_count The number of GPO Confirmation records uplaoded |
Contributor
There was a problem hiding this comment.
Nit: Typo
Suggested change
| # @param [Number] gpo_confirmation_count The number of GPO Confirmation records uplaoded | |
| # @param [Number] gpo_confirmation_count The number of GPO Confirmation records uploaded |
| ) | ||
| rescue StandardError => error | ||
| analytics.gpo_confirmation_upload( | ||
| success: false, exception: error.to_s, gpo_confirmation_count: 0, |
Contributor
There was a problem hiding this comment.
Any chance the stringified error includes sensitive values?
Contributor
Author
There was a problem hiding this comment.
Yeah, there is a risk of that if an unencrypted confirmation record gets inspected. I don't see a way for that to happen but that doesn't mean it won't. I'm not sure what the best way to balance that risk is and this error is already reported to NewRelic.
matthinz
approved these changes
Jul 16, 2024
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
We have observed a number of issues with the
GpoConfirmationUploader. We are adding more visibility to the job to improve our monitoring and alerting to deal with failures.This commit adds a line to
events.logwhen confirmations get uploaded. It includes a count of the confirmations that were uploaded so we can build a metric for that.This new logging is in addition to the records that get written to the
letter_requests_to_usps_ftp_logswhich are used in monthly reports.