Who can help, who knows SpotBugs? On Wed, Mar 15, 2023 at 1:14 AM Kai Levy <kl...@toasttab.com> wrote:
> Thanks for the suggestion Asaf, unfortunately that hasn't resolved the > issue with SpotBugs. You can see the commit here > < > https://github.com/apache/pulsar/commit/033087134e5f5417af5d8fa5e0cace41b597a2a6 > >. > When I try to compile, I receive the same error message. > > Best, > Kai > > On Tue, Mar 14, 2023 at 2:02 AM Asaf Mesika <asaf.mes...@gmail.com> wrote: > > > Good catch by SpotBugs. > > > > What if in this method > > > > @Override > > public ConsumerStatsRecorder getStats() { > > return stats; > > } > > > > You change return type to ConsumerStats so it's immutable ? > > > > > > On Sat, Mar 11, 2023 at 4:41 AM Kai Levy <kl...@toasttab.com> wrote: > > > > > I made a start on the implementation with `setRetryLetterProducer`. > > > Unfortunately, the spotbugs plugin has issues with it, because now > > > ConsumerImpl is returning a mutable object from getStats. You can see > my > > > progress here > > > < > > > > > > https://github.com/apache/pulsar/compare/master...klevy-toast:pulsar:PIP-253-expose-deadLetter-retryLetter-stats?expand=1 > > > >. > > > The spotbugs errors are below: > > > > > > [ERROR] Medium: org.apache.pulsar.client.impl.ConsumerImpl.getStats() > may > > > expose internal representation by returning ConsumerImpl.stats > > > [org.apache.pulsar.client.impl.ConsumerImpl] At ConsumerImpl.java:[line > > > 2518] EI_EXPOSE_REP > > > [ERROR] Medium: > > > org.apache.pulsar.client.impl.MultiTopicsConsumerImpl.getStats() may > > expose > > > internal representation by returning MultiTopicsConsumerImpl.stats > > > [org.apache.pulsar.client.impl.MultiTopicsConsumerImpl] At > > > MultiTopicsConsumerImpl.java:[line 851] EI_EXPOSE_REP > > > > > > > > > I would appreciate input on the best path forward. > > > > > > Thanks, > > > Kai > > > > > > On Tue, Mar 7, 2023 at 9:49 AM Kai Levy <kl...@toasttab.com> wrote: > > > > > > > Yes, that would work. > > > > > > > > Kai > > > > > > > > On Tue, Mar 7, 2023 at 12:41 AM Asaf Mesika <asaf.mes...@gmail.com> > > > wrote: > > > > > > > >> On Mon, Mar 6, 2023 at 6:24 PM Kai Levy <kl...@toasttab.com> wrote: > > > >> > > > >> > I agree, adding it to the ConsumerStats interface makes more > logical > > > >> sense, > > > >> > but I believe the implementation will be harder that way, since > the > > > >> > producers are lazily initialized. They won't be available when > > > >> > ConsumerStats is created, and there isn't currently a way to > access > > > them > > > >> > directly from the consumer. > > > >> > > > > >> > > > > >> In `ConsumerImp` you have > > > >> > > > >> private volatile Producer<byte[]> retryLetterProducer; > > > >> > > > >> You can just add setRetryLetterProducer on `ConsumerStatsRecorder` > > > >> > > > >> > > > >> > > > >> Kai > > > >> > > > > >> > On Sun, Mar 5, 2023 at 5:19 AM Asaf Mesika <asaf.mes...@gmail.com > > > > > >> wrote: > > > >> > > > > >> > > I would rather see them as attributes of ConsumerStats . > > > >> > > Add > > > >> > > > > > >> > > ProducerStats deadLetterProducerStats; > > > >> > > > > > >> > > ProducerStats retryLetterProducerStats(); > > > >> > > > > > >> > > > > > >> > > On Fri, Mar 3, 2023 at 2:54 AM Kai Levy <kl...@toasttab.com> > > wrote: > > > >> > > > > > >> > > > Hello! > > > >> > > > > > > >> > > > I created a new PIP because I discovered there's no way for a > > user > > > >> to > > > >> > > > access the metrics for a consumer's deadLetterProducer / > > > >> > > > retryLetterProducer, since it is private to > ConsumerImpl.java. I > > > >> would > > > >> > > like > > > >> > > > to propose an API change that would expose those statistics. > > More > > > >> > details > > > >> > > > on the github issue: > > > >> > > > https://github.com/apache/pulsar/issues/19698 > > > >> > > > > > > >> > > > Thanks! > > > >> > > > Kai > > > >> > > > > > > >> > > > > > >> > > > > >> > > > > > > > > > >
