Re: [DISCUSS] KIP-388 Add observer interface to record request and response

2018-12-21 Thread Colin McCabe
d probably break the existing > >>> observers. > >>>In this case, I think Dong's suggestion is the right way to go. > >>> 2. If we know (for sure) that the information that can be exposed > >>>to the observer will never change, it is

Re: [DISCUSS] KIP-388 Add observer interface to record request and response

2018-11-29 Thread Lincong Li
;interface and then implement the adaptors because there will be only one >>> implementation for the interfaces provided internally to extract the info >>>from the requests. We also don't need to separate RequestInfo and >>>ResponseInfo. What we need is jus

Re: [DISCUSS] KIP-388 Add observer interface to record request and response

2018-11-18 Thread Dong Lin
rs for the pre-defined info and pass it to the observer (e.g. use >> public void record(ObserverdInfo observedInfo)). Also, >> getProduceToTopicSizeInBytes/getProduceToTopicRecordCount/ >> >> getFetchFromTopicPartitionSizeInBytes/getFetchFromTopicPartitionRecordCount >&g

Re: [DISCUSS] KIP-388 Add observer interface to record request and response

2018-11-18 Thread Lincong Li
s/getFetchFromTopicPartitionRecordCount >are very specific to one type of request and we should make the >pre-define info more generic if we choose to go for this route. > > > > Best, > Zhanxiang (Patrick) Huang > > -- > *From:* Dong Lin > *Sent:* Satu

Re: [DISCUSS] KIP-388 Add observer interface to record request and response

2018-11-17 Thread Patrick Huang
Count are very specific to one type of request and we should make the pre-define info more generic if we choose to go for this route. Best, Zhanxiang (Patrick) Huang From: Dong Lin Sent: Saturday, November 17, 2018 18:37 To: dev Subject: Re: [DISCUSS] KIP-388 Add obs

Re: [DISCUSS] KIP-388 Add observer interface to record request and response

2018-11-17 Thread Dong Lin
Hey Lincong, The main concern for the original version of the KIP is that, if we expose internal classes such as RequestChannel.Request and AbstractResponse, it will be difficult to make change to those internal classes in the future. In the latest version of the KIP, it seems that user is going

Re: [DISCUSS] KIP-388 Add observer interface to record request and response

2018-11-14 Thread Lincong Li
Hi Wesley, Thank you very much for your feedback. The concern on memory pressure is definitely valid. However it should be the user's job to keep this concern in mind and implement the observer in the most reasonable way for their use case. In other words, implement it at their own risks. The alt

Re: [DISCUSS] KIP-388 Add observer interface to record request and response

2018-11-13 Thread xiongqi wu
Lincong, Thanks for the KIP. I have a question about the lifecycle of request and response. With the current (requestAdapter, responseAdapter) implementation, the observer can potentially extend the lifecycle of request and response through adapter. Anyone can implement own observer, and some obse

Re: [DISCUSS] KIP-388 Add observer interface to record request and response

2018-11-12 Thread Lincong Li
Thanks Mayuresh, Ismael and Colin for your feedback! I updated the KIP basing on your feedback. The change is basically that two interfaces are introduced to prevent the internal classes from being exposed. These two interfaces contain getters that allow user to extract information from request an

Re: [DISCUSS] KIP-388 Add observer interface to record request and response

2018-11-09 Thread Mayuresh Gharat
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

Re: [DISCUSS] KIP-388 Add observer interface to record request and response

2018-11-08 Thread Ismael Juma
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 c

Re: [DISCUSS] KIP-388 Add observer interface to record request and response

2018-11-08 Thread Colin McCabe
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 f

Re: [DISCUSS] KIP-388 Add observer interface to record request and response

2018-11-08 Thread radai
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 inst

Re: [DISCUSS] KIP-388 Add observer interface to record request and response

2018-11-08 Thread Peter M. Elias
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

Re: [DISCUSS] KIP-388 Add observer interface to record request and response

2018-11-08 Thread radai
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 wrote: > > Hi everyone. Here >

[DISCUSS] KIP-388 Add observer interface to record request and response

2018-11-05 Thread Lincong Li
Hi everyone. Here is my KIP. Any feedback is appreciated. Thanks, Lincong Li