Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for native image usage, motivated by Quarkus #1055

Merged
merged 11 commits into from
Oct 4, 2021

Conversation

ivantopo
Copy link
Contributor

A few weeks ago I started looking into monitoring Quarkus apps with Kamon and noticed that Kamon doesn't play very well with the native image builder.

The biggest issue is Kamon starting a few threads as soon as the Kamon companion object is loaded and the chain of classes that get loaded because of that. I tried marking those classes to be loaded at runtime but it wasn't good enough to make it work out of the box without users having to mark classes that use Kamon to also be loaded at runtime. It was all kind of a small rabbit hole with no ending.

At the end I decided to delay usages of Kamon's scheduler until Kamon.init is called, and queue up everything that gets registered until that point happens. I still need to clean up a few things, test, and ensure that I didn't introduce any new issues. But so far I can say that this works with a tiny test Quarkus app! 😄

Also, even though I'm doing this particularly with Quarkus, I think any native-built app should work just fine out of the box.

This PR is also going to introduce a few extra changes:

  • Introducing ScheduledCollector as a new type of Module. This is necessary because we had a few places where we would use Kamon's scheduler to execute tasks, but that scheduler is not available anymore. With ScheduledCollector instances we can do that, and we can also move things from the system metrics module to this mechanism as well.
  • Introducing metric-filter for metrics reporters. Something that we get asked kind of often is how to prevent certain metrics from being reported. We do have way to do this manually by wrapping a reporter with this, but it is straight forward. Since I was already touching the module registry I went ahead and added that as well.

Also, I'm under the impression that this work should make it easier for our users to ignore Kamon in their tests (although, we will have to go through almost every test to add init/stop calls). I'll try to wrap this up this week 🙏

Copy link
Contributor

@dpsoft dpsoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ivantopo awesome man!!!

core/kamon-core/src/main/scala/kamon/Kamon.scala Outdated Show resolved Hide resolved
Copy link
Contributor Author

@ivantopo ivantopo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a quick pass over the PR and found a few minor things that I fixed right away (pushing after this comment) and two other to verify (see the two comments). I'll get to them in a bit.

@ihostage
Copy link
Contributor

ihostage commented Sep 29, 2021

It will be very great if Kamon instrumentations worked only once on compile-time. But, as I understand, this opportunity was blocked by oracle/graal#1065 😞

@ivantopo
Copy link
Contributor Author

Yeap. I also would like to see that happen! For now we are only going to support the core libraries so people can at least do manual instrumentation, and then see how things develop from there 😄

@ivantopo ivantopo marked this pull request as ready for review September 29, 2021 12:17
@ivantopo
Copy link
Contributor Author

ivantopo commented Oct 4, 2021

I think this PR is ready to be merged. The currently known limitations and issues:

  • There is no integration at all with Quarkus. Users will still need to find their own ways to init/stop Kamon, and attach the automatic instrumentation if they want to run on the JVM. I'm going to address this on a separate pull request for a new kamon-quarkus module.
  • kamon-system-metrics won't work with the native image. This will be addressed in a separate PR since I think we will end up separating the process-specific collectors (JVM and Process) from the host metrics.
  • It is necessary to exclude the grpc-netty dependencies from kamon-opentelemetry reporter and explicitly add quarkus-grpc for it to work with the native image:
    <!-- This adds the quarkus-grpc-dependency -->
    <dependency>
      <groupId>io.quarkus</groupId>
      <artifactId>quarkus-grpc</artifactId>
    </dependency>

    <dependency>
      <groupId>io.kamon</groupId>
      <artifactId>kamon-opentelemetry_2.12</artifactId>
      <version>2.2.3+11-f78b64bb</version>

      <!-- This prevents kamon-opentelemetry from pulling grpc-related dependencies by itself -->
      <exclusions>
        <exclusion>
          <groupId>io.grpc</groupId>
          <artifactId>grpc-netty</artifactId>
        </exclusion>
      </exclusions>
    </dependency>

@ivantopo ivantopo merged commit cd3bb14 into kamon-io:master Oct 4, 2021
@dpsoft
Copy link
Contributor

dpsoft commented Oct 4, 2021

@ivantopo awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants