spring-cloud / spring-cloud-stream-binder-kafka

Spring Cloud Stream binders for Apache Kafka and Kafka Streams
Apache License 2.0
331 stars 302 forks source link

Allow to disable Micrometer metrics #324

Closed jdubois closed 6 years ago

jdubois commented 6 years ago

With Spring Cloud Stream Elmhurst.RC2, we have Micrometer configured out-of-the-box.

I would like to remove it, as we use Dropwizard Metrics - I don't want to enter a discussion on which one is better, just to be able to disable Micrometer. For the record, I'll also track this on a ticket (with specific details) on https://github.com/jhipster/generator-jhipster and I'll link it to here.

I have two "normal" configurations for this:

  1. Exclude it in the dependencies: that works perfectly well with org.springframework.boot:spring-boot-starter-actuator but if I do it in org.springframework.cloud:spring-cloud-stream I have a
    org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'integrationManagementConfigurer' defined in class path resource [org/springframework/integration/config/IntegrationManagementConfiguration.class]: Post-processing of merged bean definition failed; nested exception is java.lang.IllegalStateException: Failed to introspect Class [org.springframework.integration.support.management.IntegrationManagementConfigurer] from ClassLoader [sun.misc.Launcher$AppClassLoader@18b4aac2]
  2. Disable it in the application.yml file using
    management:
    metrics:
        enabled: false

This works well in Spring Boot, but is ignored by Spring Cloud Stream.

For me, both options should work for Spring Cloud Stream, like they do for Spring Boot.

olegz commented 6 years ago

@jdubois just to clarify, the spring-boot-starter-actuator is excluded by default already. It's one of the enhancement of the 2.0 line. So, I am a bit confused as to why do you have to exclude it in the first place. Could you please clarify?

jdubois commented 6 years ago

I'm a bit confused... So I'm copy/pasting parts of my pom.xml:

        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter-actuator</artifactId>
            <exclusions>
                <exclusion>
                    <groupId>io.micrometer</groupId>
                    <artifactId>micrometer-core</artifactId>
                </exclusion>
            </exclusions>
        </dependency>

...

        <dependency>
            <groupId>org.springframework.cloud</groupId>
            <artifactId>spring-cloud-stream</artifactId>
        </dependency>
        <dependency>
            <groupId>org.springframework.cloud</groupId>
            <artifactId>spring-cloud-stream-binder-kafka</artifactId>
        </dependency>
olegz commented 6 years ago

Ok, I see, so you ARE explicitly adding spring-boot-starter-actuator, but want to exclude the micrometer and when you do you get the above error. Correct?

jdubois commented 6 years ago

Only when I add Spring Cloud Stream, otherwise that works very well.

olegz commented 6 years ago

Ok, let me dig in, thanks for clarifying

garyrussell commented 6 years ago

I think the problem might be that Spring Integration now has a dependency on Micrometer (since 5.0.3).

jdubois commented 6 years ago

Yes @garyrussell - sorry about this, I'm struggling to have everything running as soon as possible with JHipster, and the Micrometer migration is much too big for us.

olegz commented 6 years ago

@jdubois we've identified the issue and few possible solutions so @garyrussell (SI) and @jkschneider (Micrometer) are wondering if we can have a quick off-line chat on this to ensure we can support you. What would be the best way to arrange that? https://jhipster.slack.com/ seems private

jdubois commented 6 years ago

Everything we do on JHipster is public so there is no private chat - I don't know what this Slack channel is and I wouldn't use it, we have lots of scammers trying to abuse JHipster users with false websites, etc... You can contact me by email - Gary probably has it I guess, or we can connect through LinkedIn

olegz commented 6 years ago

Linking to https://github.com/jhipster/generator-jhipster/issues/7214

olegz commented 6 years ago

@jdubois I admit, "private" was the wrong term to use on my end. My apology. I was primarily referring to some chat facility like Slack where we can facilitate faster discussion. But sure we can have it here, so. . . We've identified the issue in SI which currently has hard dependency on Micrometer. There are several solutions (long/short term).

  1. [Short Term] To try what @jkschneider suggested jhipster/generator-jhipster#7214
  2. [Long Term] SI can make Micrometer optional, but that would require a new release of SI which will also not be pulled until new Spring Boot (i.e., 2.0.1) or a manual dependency override in the actual application
  3. [Short Term] Given that SCSt has not been released yet we can include a "dirty trick" for the next Friday's release where using byte code generation libraries we'll essentially generate empty Micrometer stubs to satisfy SI dependency requirements in the event the real Micrometer is not on the classpath.
  4. Last but not least @jkschneider had some other suggestions around Micrometer work with/around Dropwizard Metrics so I'll let him provide the details

Thoughts?

jdubois commented 6 years ago

Oh sorry, so our chat would have been https://gitter.im/jhipster/generator-jhipster - but no worries, it's good by GitHub

Basically we are doing solution 1 - @jkschneider probably has a better solution, but that's typically the same thing, and that works for us.

Solution 2 is cleaner from my point of view, and as we generate the Maven/Gradle config it's easy to do for us -> so for me it's better

Solution 3: don't do a dirty trick for us. Solutions 1 and 2 are good, and for me this is only temporary.

In the long term, anyway, my goal is to migrate to Micrometer - we just need some time & discussion about it, but at the moment we're rushing to support everything else (we're doing a huge work on ReactJS, which obviously has nothing to do with Spring Boot, but that's where the main effort is)

olegz commented 6 years ago

Great, so it appears that at the moment if I understand correctly John's solution worked for you and we can work (long term) in SI to make Micrometer optional even though it appears to be no longer relevant to your issue given that your long term solution is to use Micrometer.

If the above is correct, feel free to close this issue and thank you for being patient with us

jdubois commented 6 years ago

Yes, it's a temporary solution but that's good for the moment - I mostly wanted that you are aware of the issue, and to be sure I wasn't missing something. Thanks a lot!

jkschneider commented 6 years ago

@jdubois Unrelated, but super excited to hear there is work on JHipster + React :)

jdubois commented 6 years ago

Thanks @jkschneider but that's also one of the reasons we can't migrate easily: we would need to re-code the metrics screen for AngularJS, Angular and React.... I think we'll do a controller that sends the same output as the current Dropwizard Metrics Servlet but that's also a lot of work.

jkschneider commented 6 years ago

Yep. I understand. It's probable that a more purpose-driven output will make each implementation far simpler. We are doing the same thing for spring-cloud-dataflow. I added more info on why the actuator/metrics endpoint can't be as simple anymore on https://github.com/jhipster/generator-jhipster/issues/7100.

I'm more than willing to help on this BTW