More precise metrics & logging #4943
Conversation
adamw
changed the title
…y to be called for individual endpoints
adamw
changed the title
| ) | ||
| } | ||
| .onInterceptorResponse { res => | ||
| m.eval(counter.labelValues(labels.valuesForRequest(req) ++ labels.valuesForResponse(res): _*).inc()) |
There was a problem hiding this comment.
I have a test that validates that when an invalid Method is used, the server responds with MethodNotAllowed
When I update to use this tapir 1.13.0, instead I get an InternalServerError and an exception in the logs
java.lang.IllegalArgumentException: Expected 3 label values, but got 2. at io.prometheus.metrics.core.metrics.StatefulMetric.labelValues(StatefulMetric.java:112) at sttp.tapir.server.metrics.prometheus.PrometheusMetrics$.requestTotal$$anonfun$1$$anonfun$3$$anonfun$1(PrometheusMetrics.scala:140) at sttp.tapir.server.metrics.prometheus.PrometheusMetrics$.requestTotal$$anonfun$1$$anonfun$3$$anonfun$adapted$1(PrometheusMetrics.scala:140) at delay @ sttp.tapir.integ.cats.effect.CatsMonadError.eval(CatsMonadError.scala:12) at flatMap @ sttp.tapir.integ.cats.effect.CatsMonadError.flatMap(CatsMonadError.scala:9) at delay @ sttp.tapir.integ.cats.effect.CatsMonadError.eval(CatsMonadError.scala:12) at flatMap @ sttp.tapir.integ.cats.effect.CatsMonadError.flatMap(CatsMonadError.scala:9) at map @ sttp.tapir.integ.cats.effect.CatsMonadError.map(CatsMonadError.scala:8) at delay @ org.typelevel.log4cats.slf4j.internal.Slf4jLoggerInternal$Slf4jLogger.contextLog(Slf4jLoggerInternal.scala:155) at map @ sttp.tapir.integ.cats.effect.CatsMonadError.map(CatsMonadError.scala:8) at map @ sttp.tapir.integ.cats.effect.CatsMonadError.map(CatsMonadError.scala:8) at flatMap @ sttp.tapir.integ.cats.effect.CatsMonadError.flatMap(CatsMonadError.scala:9) at map @ sttp.tapir.integ.cats.effect.CatsMonadError.map(CatsMonadError.scala:8) at map @ sttp.tapir.integ.cats.effect.CatsMonadError.map(CatsMonadError.scala:8) at map @ sttp.tapir.integ.cats.effect.CatsMonadError.map(CatsMonadError.scala:8) at flatMap @ sttp.tapir.integ.cats.effect.CatsMonadError.flatMap(CatsMonadError.scala:9) at adaptError$extension @ org.http4s.blaze.client.Http1Connection.executeRequest(Http1Connection.scala:255) at flatMap @ sttp.tapir.integ.cats.effect.CatsMonadError.flatMap(CatsMonadError.scala:9) at flatMap @ sttp.tapir.integ.cats.effect.CatsMonadError.flatMap(CatsMonadError.scala:9)
| * If the response was created by an [[EndpointHandler]], or a [[RequestHandler]]. This might impact the actions taken by interceptors, | ||
| * to ensure that certain actions are only taken exactly once for each response. | ||
| */ | ||
| case class Response[B](response: ServerResponse[B], source: ResponseSource) extends RequestResult[B] |
There was a problem hiding this comment.
hi, @adamw
Doesn't it look like incompatible change which requires major version bump?
There was a problem hiding this comment.
No, currently only core is covered by bin-compat requirements: https://tapir.softwaremill.com/en/latest/other/stability.html
server is stabilising, which means: stabilising: the API is mostly stable, with rare binary-incompatible changes possible in minor releases (only if necessary)
There was a problem hiding this comment.
Ok, got it, thank you
In some cases, responses are generated by interceptors: 404 rejections using
RejectInterceptor, CORS preflight requests, not-acceptable interceptor, exception interceptor. In most of these cases, there are noEndpoints that are associated with the requests.Whatever the source of the response (endpoint invocation, decode failure, 404 rejection), such responses should be handled by interceptors such as logging & metrics.
To properly log such interceptor-generated (or more precisely:
RequestHandler-generated, without invoking anEndpointHandler) responses, the log interceptor must know if the response was generated by an endpoint, or request handler. This is achieved by adding an additional field toRequestResult.Response.The logging interceptor is enhanced by calling a new
ServerLog.requestHandledByInterceptormethod, which should log the interceptor-generated response. Moreover, the exception & logging interceptors are combined, so that upon an exception, first an exception is logged, then the exception is turned into a 500 response, and then that response is logged as well.The metrics interceptor is enhanced so that when a request is received, there's always some callback on the
EndpointMetric(returned by theMetric) that is called - including interceptor-generated responses.Finally, active-requests gauge/up-down counter is changed so that it counts all requests. This also means that there's no endpoint-template (path) label associated with the metric.
The ordering of interceptors is changed. Now, the metrics & logging+exception interceptors come first, followed by CORS, reject & not-acceptable.