Hi Lincong, Thanks for the KIP.
As Colin pointed out, would it better to expose certain specific pieces of information from the request/response like api key, request headers, record counts, client ID instead of the entire request/response objects ? This enables us to change the request response apis independently of this pluggable public API, in future, unless you think we have a strong reason that we need to expose the request, response objects. Also, it would be great if you can expand on : "Add code to the broker (in KafkaApis) to allow Kafka servers to invoke any observers defined. More specifically, change KafkaApis code to invoke all defined observers, in the order in which they were defined, for every request-response pair." probably with an example of how you visualize it. It would help the KIP to be more concrete and easier to understand the end to end workflow. Thanks, Mayuresh On Thu, Nov 8, 2018 at 7:44 PM Ismael Juma <ism...@juma.me.uk> wrote: > I agree, the current KIP doesn't discuss the public API that we would be > exposing and it's extensive if the normal usage would allow for casting > AbstractRequest into the various subclasses and potentially even accessing > Records and related for produce request. > > There are many use cases where this could be useful, but it requires quite > a bit of thinking around the APIs that we expose and the expected usage. > > Ismael > > On Thu, Nov 8, 2018, 6:09 PM Colin McCabe <cmcc...@apache.org wrote: > > > Hi Lincong Li, > > > > I agree that server-side instrumentation is helpful. However, I don't > > think this is the right approach. > > > > The problem is that RequestChannel.Request and AbstractResponse are > > internal classes that should not be exposed. These are implementation > > details that we may change in the future. Freezing these into a public > API > > would really hold back the project. For example, for really large > > responses, we might eventually want to avoid materializing the whole > > response all at once. It would make more sense to return it in a > streaming > > fashion. But if we need to support this API forever, we can't do that. > > > > I think it's fair to say that this is, at best, half a solution to the > > problem of tracing requests. Users still need to write the plugin code > and > > arrange for it to be on their classpath to make this work. I think the > > alternative here is not client-side instrumentation, but simply making > the > > change to the broker without using a plugin interface. > > > > If a public interface is absolutely necessary here we should expose only > > things like the API key, client ID, time, etc. that don't constrain the > > implementation a lot in the future. I think we should also use java here > > to avoid the compatibility issues we have had with Scala APIs in the > past. > > > > best, > > Colin > > > > > > On Thu, Nov 8, 2018, at 11:34, radai wrote: > > > another downside to client instrumentation (beyond the number of > > > client codebases one would need to cover) is that in a large > > > environments you'll have a very long tail of applications using older > > > clients to upgrade - it would be a long and disruptive process (as > > > opposed to updating broker-side instrumentation) > > > On Thu, Nov 8, 2018 at 11:04 AM Peter M. Elias <petermel...@gmail.com> > > wrote: > > > > > > > > I know we have a lot of use cases for this type of functionality at > my > > > > enterprise deployment. I think it's helpful for maintaining > > reliability of > > > > the cluster especially and identifying clients that are not properly > > tuned > > > > and therefore applying excessive load to the brokers. Additionally, > > there > > > > is a bit of a dark spot without something like as currently. For > > example, > > > > if a client is not using a consumer group, there is no direct way to > > query > > > > the state of the consumer without looking at raw network connections > to > > > > determine the extent of the traffic generated by that particular > > consumer. > > > > > > > > While client instrumentation can certainly help with this currently, > > given > > > > that Kafka is intended to be a shared service across a potentially > very > > > > large surface area of clients, central observation of client activity > > is in > > > > my opinion an essential feature. > > > > > > > > Peter > > > > > > > > On Thu, Nov 8, 2018 at 12:13 PM radai <radai.rosenbl...@gmail.com> > > wrote: > > > > > > > > > bump. > > > > > > > > > > I think the proposed API (Observer) is useful for any sort of > > > > > multi-tenant environment for chargeback and reporting purposes. > > > > > > > > > > if no one wants to comment, can we initiate a vote? > > > > > On Mon, Nov 5, 2018 at 6:31 PM Lincong Li <andrewlinc...@gmail.com > > > > wrote: > > > > > > > > > > > > Hi everyone. Here > > > > > > < > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-388%3A+Add+observer+interface+to+record+request+and+response > > > > > > > > > > > > is > > > > > > my KIP. Any feedback is appreciated. > > > > > > > > > > > > Thanks, > > > > > > Lincong Li > > > > > > > > -- -Regards, Mayuresh R. Gharat (862) 250-7125
