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