async client: introduce AsyncClientRequestTracker to keep track of inflight requests and cancel them on destruction

[yskopets]

Description: introduce AsyncClientRequestTracker to keep track of inflight requests and cancel them on destruction
Risk Level: Low
Testing: unit tests
Docs Changes: N/A
Release Notes: N/A

Context:

• related to proposal: make configuration of a tracing provider dynamic #9998

Motivation:

• some tracers implementations, e.g. zipkin and datadog, allow concurrent inflight requests to trace collector
• it's important to track those requests and cancel them in case tracer gets removed
• AsyncClientRequestTracker is meant to be reusable in such cases

[mattklein123]

LGTM with small comment. Thank you!

/wait

[mattklein123]

LGTM w/ 1 small question. Thank you!

/wait-any

[mattklein123]

It doesn't make sense to pass a non-const object above and a const object here. I would just pass a non-const in both places and remove the const_cast. Thank you!

/wait

[yskopets]

AsyncClientRequestTracker::remove() is intended for use in the context of AsyncClient::Callbacks::onSuccess(request, response) and AsyncClient::Callbacks::onFailure(request, reason).

Notice that onSuccess / onFailure methods will now have request as an extra parameter to enable correlation between requests and responses.

The proposed interface for AsyncClient::Callbacks looks like this:

  /**
   * Notifies caller of async HTTP request status.
   *
   * To support a use case where a caller makes multiple requests in parallel,
   * individual callback methods provide request context corresponding to that response.
   */
  class Callbacks {
  public:
    virtual ~Callbacks() = default;

    /**
     * Called when the async HTTP request succeeds.
     * @param request  request handle.
     *                 NOTE: request handle is passed for correlation purposes only, e.g.
     *                 for client code to be able to exclude that handle from a list of
     *                 requests in progress.
     * @param response the HTTP response
     */
    virtual void onSuccess(const Request& request, ResponseMessagePtr&& response) PURE;

    /**
     * Called when the async HTTP request fails.
     * @param request request handle.
     *                NOTE: request handle is passed for correlation purposes only, e.g.
     *                for client code to be able to exclude that handle from a list of
     *                requests in progress.
     * @param reason  failure reason
     */
    virtual void onFailure(const Request& request, FailureReason reason) PURE;
  };

request was made const Request& to prevent any unintended usage, e.g. calling request.cancel().

So, const Request& in Callbacks causes the same in AsyncClientRequestTracker::remove() (otherwise, const_cast will be a part of every call to AsyncClientRequestTracker::remove()).

Do you want me to remove const AsyncClient::Request& from both AsyncClientRequestTracker and Callbacks ?

[mattklein123]

Hmm OK. Can you add a comment above the const_cast as to why it's required? Thank you.

[yskopets]

added comments

[mattklein123]

@yskopets friendly request to please never force push. It makes reviews more difficult. Please just add commits and merge master. Thank you!

[mattklein123]

Thank you!