Hooks

[endgame]

Add a system of request/response hooks, to allow fine-grained customisation of the request lifecycle. The main module that you should read is Amazonka.Env.Hooks (which is so big that GitHub "helpfully" collapses it), and I have reproduced its synopsis below:

-- Hooks carried within an 'Env', allowing ad-hoc injection of
-- different behaviour during Amazonka's request/response cycle.
-- Hooks are currently experimental, but Amazonka uses the 'Hooks' API
-- to implement its default logging, and you can add your own
-- behaviour here as well. Some examples of things hooks can do:
--
-- * Log all requests Amazonka makes to a separate log, in order to
--   audit which IAM permissions your program actually needs
--   (see 'requestHook');
--
-- * Inject a [Trace ID for AWS X-Ray](https://docs.aws.amazon.com/xray/latest/devguide/xray-concepts.html#xray-concepts-tracingheader),
--   into each request before it is signed and sent
--   (see 'configuredRequestHook'); and
--
-- * Selectively silence certain expected errors, such as DynamoDB's
--   @ConditionalCheckFailedException@ ('errorHook' and 'silenceError')
--
-- Most functions with names ending in @Hook@ ('requestHook', etc.)
-- are intended for use with lenses: partially apply them to get a
-- function @'Hook' a -> 'Hook' a@ that can go on the RHS of @(%~)@
-- (the lens modify function). You then use functions like
-- 'hookMatchingType' to selectively extend the hooks used at any
-- particular time.
--
-- Names ending in @_@ ('Hook_', 'hookMatchingType_', etc.) do not
-- feed an updated value back into Amazonka because it is either
-- unsafe to do so, or difficult to do anything useful with the
-- updated value.

Pretty much everything else is plumbing. Shout out to @axman6 for design help with this.
Ping @Fuuzetsu who raised the original issue about silencing exceptions.

Fixes #584
Fixes #501
Closes #737

[endgame]

Logger is still passed all the way down into amazonka-core, so that response deserialisation can log the raw response body (it cannot happen sooner for laziness reasons). If people like the hooks interface, I will replace the Logger parameter with a ByteString -> IO () parameter so that we can have a hook for the raw response body too.

[endgame]

#501 notes that retries are noisy. Should we have a finalError hook so that we spam the logs a little less?

[axman6]

Looks good to me, I don't think any of my comments are required changes.

[divarvel]

It's a clever way to handle the different types of requests with a single function. It took me a bit of time to put all pieces together, so maybe an explanation or a complete example in the module header might be nice.

Also, as usual for hooks, a diagram might be a good addition for documenting stages in a visual way.

[endgame]

I intend to merge this in the next couple of days - last call for comments. I'm interested to hear:

• @brendanhay are you happy with the lensy stuff?
• Do we want another logging hook for final errors, to fix Should Network.AWS.Internal.HTTP.retrier log an error each time a request fails? #501?
  • Move the error logging hook to debug or warning level
  • Add a finalError logging hook which logs at error level

[kokobd]

• Do we want another logging hook for final errors

@endgame For error and finalError, we may merge them into one and provide some context information to the hook, including:

• how many times have we retried
• is this the final retry?
• how long time has passed since the first request?
• what's the current retry policy

Then the user may define some fine-grained logic to control errors in retry, such as increasing the alarm level when retry times increase or timing out at some point

[endgame]

Yeah I agree that the error/finalError distinction is a bit rough. Probably better to merge the two and add more information to the Hook_ itself. At the moment, the final error gets logged twice and it might be possible to avoid that with the right additional information.

[endgame]

@endgame For error and finalError, we may merge them into one and provide some context information to the hook, including: -snip-

I agree that we should consolidate error hooks, and have done so. (This is also necessary to make a single use of silenceError actually silence an error.)

I think we avoid pushing too many additional parameters into the error/retry hooks at this stage. Instead, let's wait and see what people actually want, and work out the best way to give that to them. Otherwise, I fear we will add things for use-cases that nobody wants, and bind ourselves to maintaining them.

[mbj]

BTW, once this one is merged I'll setup our XRay tracing to cover AWS service calls via that hooks. Quite happy about the new feature. Before I was doing a wierd "custom send" function based tracing of these that never was scalable on the developer overhead axis.

[endgame]

@mbj Is there anything here that's missing for your use case? While I'm planning to ship the hooks as "officially experimental", it would be good to at least be able to support known use cases.

[mbj]

@endgame on early inspection its what I need, but I cannot know for sure till I ported my ugly wrapper around send.

[endgame]

@mbj Normally I'd ask people to test a PR branch so I have some confidence that merging it is going to be fine, but in this case I think merging now and testing its features is probably the way to go. Can you please let me know (via issue) whether or not the hooks are letting you do the things you want to do?