Hi Haiting, I’m not sure that if observe failed requests latency is meaningful, so that I was added `pulsar_broker_rest_endpoint_failed` to record these requests which is failed.
Thanks, Tao Jiuming > 2022年11月23日 下午12:55，Haiting Jiang <jianghait...@gmail.com> 写道： > > Hi Jiuming, > > I am not sure why we need a new metric for failed requests. > Can we just put the `httpcode` tag also in the > `pulsar_broker_rest_endpoint_latency_ms`? > So that user can also see the latency of failed requests (like timeout > requests). > And it can easily cover the case of failed metrics. > > Thanks, > Haiting > > On Wed, Nov 23, 2022 at 12:00 PM Jiuming Tao > <jm...@streamnative.io.invalid> wrote: >> >> Hi Penghui, >> >>> We'd better add an option to enable or disable the endpoint-level metrics. >>> And keep it as disabled as default. >> >> Agreed, I also considered if we need a configuration to enable/disable the >> feature, seems add an option is better >> >>> So that we can >>> get the request path from the handle method >>> `public void handle(String path, Request baseRequest, HttpServletRequest >>> request, HttpServletResponse response)` >> >> I’ve tried this, but the `path` is the request path like `/user/111/query`, >> and we cannot get the path configured by @Path by using `Handler`. >> Jetty is a Servlet container, and we are using Jersey to provide Rest API >> services, I think use Jersey’s mechanism is better >> >> Thanks, >> Tao Jiuming >> >> >> >>> 2022年11月23日 上午9:37，PengHui Li <peng...@apache.org> 写道： >>> >>> Hi, Jiuming >>> >>> Thanks for starting the proposal. >>> >>> We'd better add an option to enable or disable the endpoint-level metrics. >>> And keep it as disabled as default. >>> >>> I noticed the existing jetty metrics are based on >>> `org.eclipse.jetty.server.handler.StatisticsHandler`. >>> Can we just have a new StatisticsHandler? e.g. EndpointStatisticsHandler. >>> So that we can >>> get the request path from the handle method >>> `public void handle(String path, Request baseRequest, HttpServletRequest >>> request, HttpServletResponse response)` >>> >>> Thanks, >>> Penghui >>> >>> On Tue, Nov 22, 2022 at 5:20 PM Jiuming Tao <jm...@streamnative.io.invalid> >>> wrote: >>> >>>> Hi Haiting, >>>> >>>> I'm sorry I didn't explain FAILED, the FAILED means the HTTP response code >>>>> = 400, and I’ll update the PIP later. >>>> >>>> Thanks, >>>> Tao Jiuming >>>> >>>>> 2022年11月22日 下午5:15，Haiting Jiang <jianghait...@gmail.com> 写道： >>>>> >>>>> Hi Jiuming, >>>>> >>>>> Overall, this PIP makes sense to me. >>>>> About the metric "pulsar_broker_rest_endpoint_failed", please provide >>>>> a more clear definition of "fail". Are redirects like 403 included? >>>>> >>>>> Thanks, >>>>> Haiting >>>>> >>>>> On Tue, Nov 22, 2022 at 3:17 AM Jiuming Tao >>>>> <jm...@streamnative.io.invalid> wrote: >>>>>> >>>>>> Hi pulsar community, >>>>>> >>>>>> I’ve opened a PIP to discuss: PIP-223: Add metrics for all Rest >>>> Endpoints >>>>>> >>>>>> The PIP link: https://github.com/apache/pulsar/issues/18560 < >>>> https://github.com/apache/pulsar/issues/18560> >>>>>> >>>>>> Thanks, >>>>>> Tao Jiuming >>>> >>>> >>
