Hi Haiting, I’ll update the PIP as we discussed:
1. `pulsar_broker_rest_endpoint_failed` Labels: a. path: The HTTP request path b. method: The HTTP request method c. code: The HTTP response code This metric only records HTTP requests which response code >= 400 2. `pulsar_broker_rest_endpoint_latency` Labels: a. path: The HTTP request path b. method: The HTTP request method c. code: The HTTP response code This metric records all the HTTP requests, including failed requests. Thanks, Tao Jiuming Haiting Jiang <jianghait...@gmail.com> 于2022年11月24日周四 16:35写道： > Hi Jiuming, > > > I’m not sure that if observe failed requests latency is meaningful. > > 1. Normally for failed requests, the latency sometimes will help in > debugging. > 2. Add httpcode label to latency, we can analyze the latency between > different successful results too, like 200 and 307. > > As for `pulsar_broker_rest_endpoint_failed`, after a second thought. I > think it would be useful for operators to set up system alarms. So +1 > on this. > > Thanks, > Haiting > > On Thu, Nov 24, 2022 at 12:52 AM Jiuming Tao > <jm...@streamnative.io.invalid> wrote: > > > > 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 > > >>>> > > >>>> > > >> > > >
