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.48k stars 991 forks source link

TimedAspect LongTask Missing MeterTag Support #4578

Open razilevin opened 10 months ago

razilevin commented 10 months ago

Describe the bug Seems that a TimedAspect that is a long task is missing code to process MeterTag annotations.

Environment

To Reproduce Define a class with a function that uses MeterTag annotations and is a long task you will see your meter tags not being processed.

Expected behavior I expect a timed aspect that is defined as a long task to process MeterTag annotations correctly.

Additional context The statement below is in the context of a TimedAspect.

The function recordBuilder is processing the meterTagAnnotationHandler. A long task timer calls buildLongTaskTimer which does not use the meterTagAnnotationHandler. Not sure if this is in error or by design.

marcingrzejszczak commented 10 months ago

Would it be possible to create a test that reproduces the problem?

razilevin commented 9 months ago

Here is a test - Failing

package com.example.demo.micrometer;

import io.micrometer.core.annotation.Timed;
import io.micrometer.core.aop.MeterTag;
import io.micrometer.core.aop.MeterTagAnnotationHandler;
import io.micrometer.core.aop.TimedAspect;
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.TestConfiguration;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.EnableAspectJAutoProxy;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import org.springframework.test.context.junit.jupiter.SpringExtension;

import java.util.Objects;
import java.util.Optional;
import java.util.function.BiFunction;

import static org.assertj.core.api.Assertions.assertThat;

@ExtendWith(SpringExtension.class)
public class TimedAspectTest {
    @Autowired
    private ClassUnderTest cut;

    @Autowired
    private MeterRegistry mr;

    public static class ClassUnderTest {
        @Timed(value = "op", longTask = true)
        public void method(@MeterTag("some-key") int someField) {
            System.out.println("***");
        }
    }

    @TestConfiguration
    @EnableAspectJAutoProxy
    public static class Configuration {
        @Bean
        public MeterRegistry meterRegistry() {
            return new SimpleMeterRegistry();
        }

        @Bean
        public ClassUnderTest timedInterface() {
            return new ClassUnderTest();
        }

        @Bean
        public TimedAspect timedAspect(MeterRegistry mr) {
            var t = new TimedAspect(mr);
            var ep = new SpelExpressionParser();

            BiFunction<String, Object, String> evaluate = (e, v) -> {
                var ctx = new StandardEvaluationContext(v);
                var exp = ep.parseExpression(e);

                return Optional.ofNullable(exp.getValue(ctx))
                               .map(Object::toString)
                               .orElse("");
            };

            t.setMeterTagAnnotationHandler(new MeterTagAnnotationHandler(aClass -> Objects::toString, aClass -> evaluate::apply));

            return t;
        }
    }

    @Test
    public void givenTimedAspectMeterTagsShouldApply() {
        cut.method(100);

        var cnt = mr.get("op")
                  .tag("some-key", "100")
                  .timer()
                  .count();

        assertThat(cnt).isEqualTo(1);
    }
}