Support @TimedSet in TimedAspect

[jebeaudet]

Please describe the feature request.
Multiple @Timed annotations should be supported by TimedAspect. This is to support custom scenarios where you'd want a dedicated metric for the histogram, or some secondary metric with alternate tags. For example :

@Timed("test.two.timed.first")
@Timed(value = "test.two.timed.second", histogram = true)
@Timed(value = "test.two.timed.third", extraTags = {"somethingHere"} )
@RestController
public class TwoTimedController {

    @RequestMapping("/two")
    public String getTwo() {
        return "Two";
    }
}

Rationale
The @Timed annotation is marked as @Repeatable with @TimedSet, but the provided TimedAspect by the library does not provide a pointcut for it, making it unususable. Moreoever, if your class has 2 or more @Timed annotations, it will silently get ignored.

Additional context
It was asked in the past here : #1032
And it could be seen as a regression from Spring Boot 2.7 where it was handled : spring-projects/spring-boot#44236

Thanks!

[jonatan-ivanov]

Because of the presence of @TimedSet, I think we should support it but on the other hand, @Observed might make this unnecessary so could you please tell us more about your use-case?

What you are doing in the example can be done using one annotation:

@Timed(value = "test", extraTags = {"somethingHere"}, histogram = true)

This way you can query

• the count/sum/max (with or without the histogram or the extra tags) like you would using test.two.timed.first
• the histogram (with or without the extra tags) like you would using test.two.timed.second
• the count/sum/max (with or without the histogram or the extra tags) like you would using test.two.timed.third

These are just duplicates so I don't understand why/how you need/use them.
The one use-case where multiple @Timed annotations can be useful that comes to my mind is this:

@Timed(value = "test")
@Timed(value = "test.active", longTask = true)

But this is what @Observe does for you out of the box so multiple annotations might not be needed I think.

Also, as a workaround, you can create an ObservationHandler and create as many Timers in it as you want (you need to use @Observed in that case), check out DefaultMeterObservationHandler for the reference.

[agillesCoveo]

Because of the presence of @TimedSet, I think we should support it but on the other hand, @Observed might make this unnecessary so could you please tell us more about your use-case?

Hi Jonatan, I'm working with Jacques.

Our use case is to be able to provide:

• a Metric to cover fine detail latency measurements, using default histogram bucket algorithm with 70+ buckets
• a Metric to give coarse grain latency where we can add multiple high cardinality tags that depends on the request we receive

The idea being to avoid generating the 70+ buckets for every value of the high cardinality tags we want to use.

[jonatan-ivanov]

I'm still not sure I understand what you are doing:

• With @Timed, you cannot really add high cardinality tags since Java does not let you to do dynamic things in annotations (unless you have a lot of annotations but I guess that's not the case)
• Metrics should not have high cardinality tags, see: https://develotters.com/posts/high-cardinality/ but you can use other signals for high cardinality data

We might have a different definition of high cardinality, can you give me an example of what you are trying to do? The example in Jacques' description, that's not really high cardinality.
Do you want to save costs by having less time series by not having a histogram for all the tag combination? Something like this?

@Timed(value = "test", extraTags = {"operation", "one"}, histogram = true)
@GetMapping("/one")
String one() {
    return "one";
}

@Timed(value = "test", extraTags = {"operation", "two"}, histogram = true)
@GetMapping("/two")
String two() {
    return "two";
}

@Timed(value = "test", extraTags = {"operation", "three"}, histogram = true)
@GetMapping("/three")
String three() {
    return "three";
}

This ends up in 3*69 (N*69) time series for the histogram buckets (+ 9 (N*3) for count/sum/max).

So you want something like this instead?

@Timed(value = "test.fine", histogram = true)
@Timed(value = "test.coarse", extraTags = {"operation", "one"})
@GetMapping("/one")
String one() {
    return "one";
}

@Timed(value = "test.fine", histogram = true)
@Timed(value = "test.coarse", extraTags = {"operation", "two"})
@GetMapping("/two")
String two() {
    return "two";
}

@Timed(value = "test.fine", histogram = true)
@Timed(value = "test.coarse", extraTags = {"operation", "three"})
@GetMapping("/three")
String three() {
    return "three";
}

This ends up in 69 (1*69) time series for the histogram buckets (+ 12 (1*3+N*3) for count/sum/max).

If so, this is not really high cardinality but I understand the cost-cutting part. If not, please give me an example (with annotated methods and/or time-series output).

[agillesCoveo]

We add a tag depending of one of the parameter on the path of the request (something like customerName) by implementing MeterRegistryCustomizer<MeterRegistry> buildMeterRegistry, that's what I meant by dynamic. This will scale with the number of customer we have, that's why we wanted to be careful with pairing it with the histogram option.

We knew it wasn't a long term solution as the cardinality was always going to explode. We could go from 10 customer to 100 in a few months (if we're successful hehehe) and the N in your formula is suddenly a lot higher.

We are looking at another solution at the moment, because making this works looks like as much (if not more) effort than migrating to a more sustainable way of collecting those metrics.

Thanks a lot for your time and your support, you can close this for us if you think this is not a legitimate use case.

[jonatan-ivanov]

We add a tag depending of one of the parameter on the path of the request (something like customerName) by implementing MeterRegistryCustomizer buildMeterRegistry

I don't really understand how do you do that, do you configure a MeterFilter from your MeterRegistryCustomizer? If so, you should not do this, see the warning in the docs: https://docs.micrometer.io/micrometer/reference/concepts/meter-filters.html#_transforming_metrics

This will scale with the number of customer we have

That sounds like high cardinality which you should not do, please see the link in my previous message why: https://develotters.com/posts/high-cardinality/

We knew it wasn't a long term solution as the cardinality was always going to explode. We could go from 10 customer to 100 in a few months (if we're successful hehehe) and the N in your formula is suddenly a lot higher.

We are looking at another solution at the moment, because making this works looks like as much (if not more) effort than migrating to a more sustainable way of collecting those metrics.

Thanks a lot for your time and your support, you can close this for us if you think this is not a legitimate use case.

I don't think your use-case is valid for this particular change but let's keep this open since there might be other users with a different use-case (e.g.: see the LongTaskTimer case above). I think there are multiple ways to solve this for you and neither of them needs a change in Micrometer so you can do them as of today.

1. Having 10 to 100 unique values is not really high cardinality so you might be able to proceed with metrics for this but if my assumption is correct and you use a MeterFilter to attach dynamic data, that will not work since newer releases are caching the result of MeterFilter, see the warning in the docs: https://docs.micrometer.io/micrometer/reference/concepts/meter-filters.html#_transforming_metrics You can do two things to get rid of the dynamic MeterFilter:
   1.1 Create Timers from code instead of using an annotation, check the MeterProvider, it can make this a one-liner just like the annotation but you can attach tags dynamically in a proper way.
   1.2 You can use the Observation API (starting from Spring Boot 3), create a new Observation and attach this data to it (.lowCardinalityKeyValue(...)), in Spring Boot you can configure a histogram for the "coarse" metric from a property (and the "fine" one will not have a histogram by default).
2. Once this becomes a problem, you can attach this information either directly to the Observation (observationRegistry.currentObservation().highCardinalityKeyValue(...)) or you can use an ObservationConvention or an ObservationFilter. This means that you can output this data to somewhere that can handle high cardinality data (tracing, logs, etc.) and your metrics will only contain low cardinality data.

Does any of this help?

[agillesCoveo]

I don't really understand how do you do that, do you configure a MeterFilter from your MeterRegistryCustomizer?

That's exactly what we do. Thanks for the warning, another sign we need to refactor this part of our service.

Does any of this help?

Yes greatly, we understand a lot more how this is suppose to be used. We'll probably play with Observation API and see how it goes.