Skip to content

[ETCM-49] Replace statsd metrics with prometheus #668

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

Merged
merged 8 commits into from
Sep 18, 2020

Conversation

mirkoAlic
Copy link
Contributor

@mirkoAlic mirkoAlic commented Sep 14, 2020

Description

in order to follow IOHK devops standards we will move our metrics to Prometheus (Pull monitoring model) + Grafana.

Important Changes Introduced

  • New Dependencies for node
  • Setup Prometheus server and client with JVM stats
  • Add logback collector
  • Integrate Block metrics (using micrometer) into Prometheus monitoring (Pull model)
  • Remove StatsD system (Push model)
  • Use Metrics configuration

Testing

In order to run it locally using docker monitoring client please check the README.md#Monitoring

@mirkoAlic mirkoAlic force-pushed the ETCM-49-Replace-statsd-metrics-with-prometheus branch 2 times, most recently from 6271adf to 3e23224 Compare September 14, 2020 18:28
@mirkoAlic mirkoAlic force-pushed the ETCM-49-Replace-statsd-metrics-with-prometheus branch from 3e23224 to 7851d30 Compare September 14, 2020 18:33
@mirkoAlic mirkoAlic marked this pull request as ready for review September 14, 2020 18:48
@mirkoAlic mirkoAlic requested a review from mmrozek September 16, 2020 11:14
@mirkoAlic mirkoAlic force-pushed the ETCM-49-Replace-statsd-metrics-with-prometheus branch from 83ba14b to ebc0d7c Compare September 16, 2020 14:28
@mirkoAlic mirkoAlic force-pushed the ETCM-49-Replace-statsd-metrics-with-prometheus branch from ebc0d7c to cf151db Compare September 16, 2020 16:03
import io.micrometer.jmx.JmxConfig

class AppJmxConfig extends JmxConfig {
def get(key: String): String = null
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need that?

Copy link
Contributor Author

@mirkoAlic mirkoAlic Sep 17, 2020

Choose a reason for hiding this comment

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

yes (override is missing, i will add it)

import scala.util.Try

case class Metrics(metricsPrefix: String, registry: MeterRegistry, serverPort: Int = 0) {
private[this] final val MetricsPrefixDot = metricsPrefix + "."
Copy link
Contributor

Choose a reason for hiding this comment

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

The same logic is used in meterRegistryBuilder maybe it could be reused

case class Metrics(metricsPrefix: String, registry: MeterRegistry, serverPort: Int = 0) {
private[this] final val MetricsPrefixDot = metricsPrefix + "."

private[this] def mkName(name: String): String =
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

… into ETCM-49-Replace-statsd-metrics-with-prometheus

Conflicts:
  build.sbt
  src/main/scala/io/iohk/ethereum/network/p2p/messages/PV62.scala
  src/universal/conf/logback.xml
  src/main/scala/io/iohk/ethereum/domain/BlockBody.scala
TODO:
  Regenerate nix dependencies
…repo.nix files, sbtnix is inyecting local paths)
@mirkoAlic mirkoAlic added the update dependencies notify if is required to update nix dependencies or run sbt clean before compile the code label Sep 17, 2020
Copy link
Contributor

@mmrozek mmrozek left a comment

Choose a reason for hiding this comment

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

LGTM

@mirkoAlic mirkoAlic merged commit 57b23ac into develop Sep 18, 2020
@mirkoAlic mirkoAlic deleted the ETCM-49-Replace-statsd-metrics-with-prometheus branch September 18, 2020 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement update dependencies notify if is required to update nix dependencies or run sbt clean before compile the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants