Hey Chesnay, I think I got what that method was designed for now. Basically the motivation is to let the SourceOutput to report the eventTimeFetchLag for users. At this point, the SourceOutput only has the EventTime, so this method provides a way for the users to pass the FetchTime to the SourceOutput. This is essentially a context associated with each record emitted to the SourceOutput.
It might be slightly better if we let the method accept a Supplier in this case. However, it seems to introduce a parallel channel or a sidepath between the user implementation and SourceOutput. I am not sure if this is the right way to go. Would it be more intuitive if we just add a new method to the SourceOutput, to allow the FetchTime to be passed in explicitly? This would work well with the change I suggested above, which adds a generic metadata type <T> to the RecordsWithSplits and passes that to the RecordEmitter.emitRecord() as an argument. What do you think? Thanks, Jiangjie (Becket) Qin On Tue, Jul 20, 2021 at 2:50 PM Chesnay Schepler <ches...@apache.org> wrote: > Would it be easier to understand if the method would accept a Supplier > instead? > > On 20/07/2021 05:36, Becket Qin wrote: > > In that case, do we still need the metric here? It seems we are creating > a > > "global variable" which users may potentially use. I am wondering how > much > > additional convenience it provides because it seems easy for people to > > simply pass the fetch time by themselves if they have decided to not use > > SourceReaderBase. Also, it looks like we do not have an API pattern that > > lets users get the value of a metric and derive another metric. So I > think > > it is easier for people to understand if LastFetchTimeGauge() is just an > > independent metric by itself, instead of being a part of the > > eventTimeFetchLag computation. > > >
