micrometer-metrics / micrometer

An application observability facade for the most popular observability tools. Think SLF4J, but for observability.
https://micrometer.io
Apache License 2.0
4.45k stars 980 forks source link

Divergence between TimedAspect and CountedAspect during overwriting extraTags #2461

Open kinddevil opened 3 years ago

kinddevil commented 3 years ago

When overwriting the extraTags in Aspect, the behavior is different - extraTags can be replaced in the TimedAspect but in CountedAspect as below.

I have a function with extra tags like

@Timed(extraTags = {"extra", "A"})
@Counted(extraTags = {"extra", "B"})
public void recordMetrics() {
...
}

When I overwrite the extra tag in TimedAspect

@Bean
  public TimedAspect timedAspect(MeterRegistry registry) {
    return new TimedAspect(registry, p -> Arrays.asList(Tag.of("extra", "AA"));
  }

The extra changed to AA successfully. But when I use some similar code in CountedAspect it failed.

  @Bean
  public CountedAspect countedAspect(MeterRegistry registry) {
    return new CountedAspect(registry,  p -> Arrays.asList(Tag.of("extra", "BB"));
  }

The value is still B.

It is because of the different orders when applying the ProceedingJoinPoint tags https://github.com/micrometer-metrics/micrometer/blob/939e0e4a36cc3bf05785c6436f1f7a72e710c9d9/micrometer-core/src/main/java/io/micrometer/core/aop/CountedAspect.java#L148 and https://github.com/micrometer-metrics/micrometer/blob/939e0e4a36cc3bf05785c6436f1f7a72e710c9d9/micrometer-core/src/main/java/io/micrometer/core/aop/TimedAspect.java#L192.

Can we make them in the same order?

kinddevil commented 3 years ago

There is a fix for the issue https://github.com/micrometer-metrics/micrometer/pull/2462, can someone take a look? Thank you!

kinddevil commented 3 years ago

Hi @shakuzen, do you have any suggestions for this issue?