Thanks for your feedback, Colin, see response below.
Den tors 17 juni 2021 kl 00:28 skrev Colin McCabe <cmcc...@apache.org>:
> On Sun, Jun 13, 2021, at 21:51, Travis Bischel wrote:
>
...
> > Another downside is that by dictating the important metrics, this KIP
> either
>
> has two choices: try to choose what is important to every org, and
> inevitably
> > leave out something important to somebody else, or just add everything
> and let
> > the orgs filter. This KIP mostly looks to go with the latter approach,
> meaning
> > orgs will be shipping & filtering. With hooks, an org would be able to
> gather
> > exactly what they want.
>
> I actually do agree with this criticism to some extent. It would be good
> if the broker could specify what metrics it wants, and the clients would
> send only those metrics.
>
The metrics to collect are indeed controlled by the cluster operator (or
whoever has access),
this is done by setting up metrics subscriptions (a new Admin ConfigEntry)
that are propagated to the client through the
PushTelemetryResponse, telling the client exactly what metrics to push and
at what interval.
> More generally, I'd like to see this split up into several RPCs rather
> than one mega-RPC.
>
> Maybe something like
> 1. RegisterClient{Request,Response}
> 2. ClientMetricsReport{Request,Response}
> 3. UnregisterClient{Request,Response}
>
> Then the broker can communicate which metrics it wants in
> RegisterClientResponse. It can also assign a client instance ID (which I
> think should be a UUID, not another string).
>
All this functionality is covered by the single PushTelemetryRequest which
is used both
for pushing metrics to the broker (in the request) and propagating metrics
subscriptions
to the client (in the response). Using a single request type for both these
operations allows
piggy-backing either metrics or subscriptions (depending on direction) in a
request that
is sent at regular intervals, sort of like a recurring poll.
I think something like RegisterClientRequest makes sense for deconfliction
and fencing,
such as with InitProducerIdRequest, but we don't have any need for that so
I don't think the
added complexity gives us much.
/Magnus
