OpenFeign / feign

Feign makes writing java http clients easier
Apache License 2.0
9.43k stars 1.92k forks source link

Provide histogram informations for base Feign metric #1630

Open sesigl opened 2 years ago

sesigl commented 2 years ago

In my current project, we use a filter to change the micrometer Meter to expose histogram information to be able to calculate percentiles easily. Usually, this is very helpful to get a better understanding of latency.

Currently, we "update" the meter like this using a custom MeterFitler:

        val sloBoundariesInNanos = Arrays.stream(sloBoundaries)
            .mapToDouble { obj -> obj.toNanos().toDouble() }
            .toArray()

        val builder = DistributionStatisticConfig.builder()
            .percentilesHistogram(true)
            .serviceLevelObjectives(*sloBoundariesInNanos)

        return builder.build()
            .merge(config)

Unfortunately, this leads to code duplication in projects and teams. Ideally, this should be either activated by default, or it should be easy to configure.

My proposal would be to expose it by default. There is not a huge performance impact, and the metrics will give engineers more insights into latencies.

I looked already into the current open-source implementation and located the timer of interest created in MeteredInvocationHandleFactory.

I am not sure how this should impact DropWizard metrics. There, maybe the histogram type can be useful.

Once this is done, this might be also interesting for other metrics like client metrics.

Looking forward to some feedback. I am also happy to contribute in case of no one started yet, but want to collect feedback before investing time.

kdavisk6 commented 1 year ago

Feel free to submit an API proposal and we'll work with you to get it implemented.