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

upgrade Kamon to 2.1.12 #101

Merged
merged 8 commits into from
Mar 19, 2021
Merged

upgrade Kamon to 2.1.12 #101

merged 8 commits into from
Mar 19, 2021

Conversation

bcarter97
Copy link
Member

@bcarter97 bcarter97 commented Mar 15, 2021

Description

  • Upgrade Kamon to 2.1.12 to remove the nanohttpd peer dependency

Migration notes

Other changes

@btanyab
Copy link
Contributor

btanyab commented Mar 16, 2021

In the application.conf configuration path need to be changed to :

kamon {
  environment.service = "kafka-message-scheduler"
  instrumentation.akka.filters {
    "actors.track" {
      includes = ["kafka-message-scheduler/user/scheduling-actor", "kafka-message-scheduler/user/publisher-actor"]
      excludes = ["kafka-message-scheduler/system/**"]
    }
  }

as per Kamon docs

@peter-hazell
Copy link

peter-hazell commented Mar 16, 2021

In the application.conf configuration path need to be changed to :

kamon {
  environment.service = "kafka-message-scheduler"
  instrumentation.akka.filters {
    "actors.track" {
      includes = ["kafka-message-scheduler/user/scheduling-actor", "kafka-message-scheduler/user/publisher-actor"]
      excludes = ["kafka-message-scheduler/system/**"]
    }
  }

as per Kamon docs

Good spot Tanya - final paragraph of the migration guide explains as well: https://kamon.io/docs/latest/guides/migration/from-1.x-to-2.0/#filter-changes

@peter-hazell
Copy link

peter-hazell commented Mar 16, 2021

@bcarter97 I think there are couple of parts of the migration guide we should revisit, even though we did see metrics being produced when we tested this, they might not be all the right metrics we expect

https://kamon.io/docs/latest/guides/migration/from-1.x-to-2.0/#there-is-a-new-kamoninit-method - Kamon.init() needs adding in our main method

https://kamon.io/docs/latest/guides/migration/from-1.x-to-2.0/#we-moved-from-aspectj-to-kanela - I think there is an aspectj dependency in build.sbt at the moment, I expect that can be changed to kanela or removed all together.

@btanyab
Copy link
Contributor

btanyab commented Mar 16, 2021

Would it be easier to import just kamon-bundle (instead of each module separately) which bundles all the instrumentation modules available in Kamon and the Kanela agent

scheduler/src/main/resources/application.conf Show resolved Hide resolved
build.sbt Outdated Show resolved Hide resolved
build.sbt Outdated Show resolved Hide resolved
docker/docker-compose.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@paulaylingdev paulaylingdev left a comment

Choose a reason for hiding this comment

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

Minor comments but looks good!

build.sbt Outdated Show resolved Hide resolved
build.sbt Outdated Show resolved Hide resolved
docker/docker-compose.yml Outdated Show resolved Hide resolved
scheduler/src/main/scala/com/sky/kms/Main.scala Outdated Show resolved Hide resolved
@paulaylingdev
Copy link
Contributor

Can we update the monitoring paragraph a little in the README.md. I think the link to 'kamon prometheus reporter' is old.

Might be worth adding a sentence that prometheus is included with the docker compose as an example.

@bcarter97 bcarter97 changed the title upgrade Kamon to latest version upgrade Kamon to 2.1.12 Mar 19, 2021
@bcarter97 bcarter97 merged commit 47ec6e3 into master Mar 19, 2021
@bcarter97 bcarter97 deleted the nanohttpd-upgrade-fix branch March 19, 2021 15:29
paulaylingdev added a commit that referenced this pull request Jul 16, 2021
btanyab pushed a commit that referenced this pull request Jul 20, 2021
* Use jitpack kafka-topic-loader instead of bintray

* Revert kamon-bundle use from #101

* Bump kamon to 2.2.2

* Make kamon-jmx-collector unmanaged
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.

5 participants