Reactor-netty: Add option for changing the max URI tags

Created on 14 Jan 2020  路  8Comments  路  Source: reactor/reactor-netty

All 8 comments

I'm looking to pick this up but looking for some advice on the design of the solution. Looking through the code, I code see a few ways to implement this.

  • overload the metrics() function on HttpServer to accept a new parameter - maxUriTags
  • add new method on HttpServer specifically for setting the maxUriTags ie. maxUriTags(int maxTags)
  • add a parameter to MicrometerHttpMetricsRecorder constructor
    MicrometerHttpMetricsRecorder(String name, String protocol, int maxUriTags)
    which is passed up from both MicrometerHttpServerMetricsRecorder and MicrometerHttpClientMetricsRecorder. Then metrics(boolean metricsEnabled, HttpServerMetricsRecorder recorder) can be used to override the default max uri tags value.

However, I think for the third option both MicrometerHttpClientMetricsRecorder and MicrometerHttpServerMetricsRecorder classes would have to change scope from default to public.

I was thinking for something like this:

metrics(boolean metricsEnabled, int maxUriTags)
BootstrapHandlers#updateMetricsSupport(io.netty.bootstrap.ServerBootstrap, reactor.netty.channel.ChannelMetricsRecorder, int maxUriTags)
MetricsSupportConsumer(ChannelMetricsRecorder recorder, int maxUriTags, boolean onServer)

so when we add NettyPipeline.ChannelMetricsHandler to the pipeline if the recorder is MicrometerHttpMetricsRecorder we can add this filter

wdyt?

Hi.
I would like to join your project. Can I start with this task?

@parviz-93 @michaelmcfadyen already stated that he is willing to take this issue.
@michaelmcfadyen Are you still going to work on this?

@violetagg got it.
Maybe you have some simple task for me? I would be happy to help 馃槉

@parviz-93 let's move the discussion potential contributions here https://gitter.im/reactor/reactor-netty
this issue is not the correct channel ;)

@violetagg I've been out of the country this week so not started on this issue but yes, I'm still willing to pick it up. Can you assign me to the issue?

@michaelmcfadyen we do not use assign, a comment here that you'll be working on this is enough.

Was this page helpful?
0 / 5 - 0 ratings