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
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> >
>
