Http4s MetricsOps

[lhns]

Hi, I implemented the http4s MetricsOps trait for otel4s.

• Use names conforming to otel semconv
• Is the core module the right place for a metrics integration or does it belong into a separate module?
• How should the histogram buckets be configured? (fixed in otel4s 0.5.0-RC1)
• Implement Test and Example
• The PR is ready for review

Related: http4s/http4s#7448

[alexcardell]

Whether they should be separate middleware is an interesting question, as combining them (having a Tracer context bound for the metrics middleware) opens up the possibility of using exemplars

[NthPortal]

honestly, I don't know enough to review this PR well. thoughts, @rossabaker ?

[iRevive]

Whether they should be separate middleware is an interesting question, as combining them (having a Tracer context bound for the metrics middleware) opens up the possibility of using exemplars

I wonder if it will work in the current state.

OpenTelemetry Java uses JContext.current() to attach trace_id/span_id to an exemplar. The context cannot be propagated seamlessly between CE IOLocal and oteljava ThreadLocal. There are more details: typelevel/otel4s#202 and a possible fix typelevel/otel4s#214 (waiting for CE, though).

However, we can propagate the context manually (changes to otel4s required). For example, a Counter instrument:

def add(
  value: A,
  attributes: immutable.Iterable[Attribute[_]]
): F[Unit] =
  Ask[F, Context].ask.flatMap { ctx =>
    Sync[F].delay {
      val scope = ctx.underlying.makeCurrent() // make the current context active
      counter.add(cast(value), attributes.toJavaAttributes) 
      scope.close() // release the context
    }  
  }

[iRevive]

Do you think the metric names are chosen correctly?

Here are the recommended names and dimensions: https://opentelemetry.io/docs/specs/semconv/http/http-metrics/.

I'm unsure whether the spec should be followed 1-to-1 though.

[iRevive]

If we follow OpenTelemetry suggestions (i.e. attribute/dimension names) there would be major differences with the http4s-prometheus.

So it wouldn't work as a drop-in replacement (from the visualization standpoint, e.g. Grafana dashboard).

On the other hand, if we follow the suggestions, metrics provided by the http4s-otel4s should be compatible with any OpenTelemetry http dashboard (e.g. https://grafana.com/grafana/dashboards/18860-http-metrics-opentelemetry/). This is a plus when you monitor various services written in different languages/stacks.

[lhns]

I am pretty sold on otel and I think the most future proof thing would be to rely on their semconv. The naming already differs a bit from the prometheus impl because of the dots in the metric names. If we want to link traces and metrics they should probably follow the same naming conventions.

[lhns]

I now changed the metric names and attributes to use otel semantic naming where applicable. A few exceptions are:

• http.server.abnormal_terminations
• http.phase
• classifier

[lhns]

rebased on #65

[NthPortal]

sorry for renaming and breaking everything >.>

[lhns]

I added the Metrics middleware to Http4sExample and created a small test. GH Actions seem to run fine on my fork.

[lhns]

@rossabaker just wanted to let you know that the PR should be ready for review. :)

[iRevive]

Apologies for the git conflicts. Since otel4s 0.7.0, we can use otel4s-sdk-teskit to run tests across all platforms. Example.

Here is the branch with resolved conflicts: https://github.com/iRevive/http4s-otel4s-middleware/pull/1/files.

[NthPortal]

Since otel4s 0.7.0, we can use otel4s-sdk-teskit to run tests across all platforms.

the latest main (currently released as 0.6.0) has been updated to use otel4s 0.7.0

[iRevive]

Looks good!

[NthPortal]

thanks for all the work you've put into this @lhns (and to @iRevive for the reviews)!

[NthPortal]

thanks again @lhns!

[NthPortal]

release tagged as v0.7.0-rc.1 including this PR; it will be on maven central as soon as the build finishes and things propagate