Skip to content
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

Add SNS request Id to log message when message is published #757

Merged

Conversation

maurofranchi
Copy link
Contributor

We are currently experiencing some SNS issues in production and AWS always ask us to provide them with some SNS Request Ids related to the problematic publish operations. Unfortunately JustSaying is not returning it.

This PR is to simply add the returned request Id (if present) to the message logged when a message is published.
I know V6 is currently in maintenance mode but this could help us investigate a bit better on any issue we might have on AWS.

@maurofranchi maurofranchi requested a review from a team as a code owner August 14, 2020 08:54
@ghost ghost requested review from brainmurphy and removed request for a team August 14, 2020 08:54
@slang25
Copy link
Member

slang25 commented Aug 14, 2020

If the publish is fails with an exception then today you can use HandleException to then log the request id. @josephwoodward added this functionality for this reason (AWS wanted request ids to be able to provide support)
#350

@martincostello
Copy link
Member

Ah - for some reason I thought that capability only existed in v7. In that case, we can just use that and not bother changing the logs.

@martincostello
Copy link
Member

Just checked the tags of the merge commit (ef8334a) and it's in all the v6 releases.

@maurofranchi
Copy link
Contributor Author

If the publish is fails with an exception then today you can use HandleException to then log the request id. @josephwoodward added this functionality for this reason (AWS wanted request ids to be able to provide support)
#350

I saw the messageId is returned, but lately AWS started asking for RequestId which is not currently added to the MessageResponse object from that PR you linked. Do you want me to add it to that object?

@maurofranchi
Copy link
Contributor Author

I saw the messageId is returned, but lately AWS started asking for RequestId which is not currently added to the MessageResponse object from that PR you linked. Do you want me to add it to that object?

Also I forgot to say that in our case the request doesn't fail, so we would not have the handleException. It just takes a bit longer to publish the message.

@slang25
Copy link
Member

slang25 commented Aug 14, 2020

Ah yeah, that doesn't help

@slang25
Copy link
Member

slang25 commented Aug 14, 2020

I don't think adding it to the existing object makes sense, cause it relies on an exception occuring. I think this change makes most sense 👍

@martincostello martincostello added this to the v6.0.4 milestone Aug 14, 2020
@martincostello martincostello merged commit 6a6b469 into justeattakeaway:rel/v6 Aug 14, 2020
slang25 referenced this pull request in slang25/JustSaying-oh-no-not-again Sep 11, 2020
Co-authored-by: maurofranchi <mauro.franchi@just-eat.com>
slang25 referenced this pull request in slang25/JustSaying-oh-no-not-again Sep 11, 2020
Co-authored-by: maurofranchi <mauro.franchi@just-eat.com>
slang25 added a commit that referenced this pull request Sep 11, 2020
Added SNS request Id to log message when message is published (#757)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants