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

ObservedAspect should intercept methods in super classes #3809

Open quaff opened 1 year ago

quaff commented 1 year ago

Please describe the feature request. Currently ObservedAspect use @within pointcut, will only intercept methods declared in the class which annotated with @Observed, methods in super classes are not get observed, it's impossible to add @Observed to super class if it from 3rd party.

Rationale Assume we have a class named Parent contains method parentMethod() from 3rd party, and we create our own child class named Child extends Parent and annotated @Observed with new method childMethod(), I'm expecting all method call on child get observed, but child.parentMethod() is not.

Additional context

import static io.micrometer.observation.tck.TestObservationRegistryAssert.assertThat;

import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.actuate.observability.AutoConfigureObservability;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.context.TestConfiguration;
import org.springframework.context.annotation.Bean;

import io.micrometer.observation.ObservationRegistry;
import io.micrometer.observation.annotation.Observed;
import io.micrometer.observation.aop.ObservedAspect;
import io.micrometer.observation.tck.TestObservationRegistry;

@AutoConfigureObservability
@SpringBootTest
public class ApplicationTests {

    @Autowired
    Child child;

    @Autowired
    TestObservationRegistry registry;

    @Test
    void test() {
        child.childMethod();
        assertThat(registry).hasNumberOfObservationsEqualTo(1);
        child.parentMethod();
        // will fail since parentMethod is not intercepted
        assertThat(registry).hasNumberOfObservationsEqualTo(2);
    }

    @TestConfiguration
    static class ObservationTestConfiguration {

        @Bean
        TestObservationRegistry observationRegistry() {
            return TestObservationRegistry.create();
        }

        @Bean
        ObservedAspect observedAspect(ObservationRegistry observationRegistry) {
            return new ObservedAspect(observationRegistry);
        }

        @Bean
        Child child() {
            return new Child();
        }
    }

    @Observed
    static class Child extends Parent {

        public void childMethod() {

        }
    }

    static class Parent {

        public void parentMethod() {

        }
    }

}

minimal spring boot project observed.zip

marcingrzejszczak commented 1 year ago

We would have to add some additional, Spring specific code in Micrometer for that to work (I'm not saying we can't do that). We have such similar code working in Spring Cloud Sleuth e.g.

jonatan-ivanov commented 1 year ago

it's impossible to add @Observed to super class if it from 3rd party.

Right now you either need to override that method in the cild and add @Observed on it or ask the third-party to instrument their codebase.