From a090470bf3b1dd644e1d03fda66f7cbb4b2a594e Mon Sep 17 00:00:00 2001 From: Karel Fajkus Date: Mon, 8 Oct 2018 16:42:56 +0200 Subject: [PATCH 1/3] added statsd builder --- .../statsd/StatsDMetricsMonitorBuilder.java | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 statsd/src/main/java/com/avast/metrics/statsd/StatsDMetricsMonitorBuilder.java diff --git a/statsd/src/main/java/com/avast/metrics/statsd/StatsDMetricsMonitorBuilder.java b/statsd/src/main/java/com/avast/metrics/statsd/StatsDMetricsMonitorBuilder.java new file mode 100644 index 0000000..fe916ce --- /dev/null +++ b/statsd/src/main/java/com/avast/metrics/statsd/StatsDMetricsMonitorBuilder.java @@ -0,0 +1,79 @@ +package com.avast.metrics.statsd; + +import com.avast.metrics.api.Naming; +import com.avast.metrics.filter.MetricsFilter; + +import java.time.Duration; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; + +public class StatsDMetricsMonitorBuilder { + private String host; + private int port; + private boolean autoRegisterMetrics = false; + private String prefix; + private Naming naming = Naming.defaultNaming(); + private Duration gaugeSendPeriod = getDefaultGaugeSendPeriod(); + private ScheduledExecutorService scheduler = createScheduler(); + private MetricsFilter metricsFilter = MetricsFilter.ALL_ENABLED; + + public StatsDMetricsMonitorBuilder setHost(String host) { + this.host = host; + return this; + } + + public StatsDMetricsMonitorBuilder setPort(int port) { + this.port = port; + return this; + } + + public StatsDMetricsMonitorBuilder setAutoRegisterMetrics(boolean autoRegisterMetrics) { + this.autoRegisterMetrics = autoRegisterMetrics; + return this; + } + + public StatsDMetricsMonitorBuilder setPrefix(String prefix) { + this.prefix = prefix; + return this; + } + + public StatsDMetricsMonitorBuilder setNaming(Naming naming) { + this.naming = naming; + return this; + } + + public StatsDMetricsMonitorBuilder setGaugeSendPeriod(Duration gaugeSendPeriod) { + this.gaugeSendPeriod = gaugeSendPeriod; + return this; + } + + public StatsDMetricsMonitorBuilder setScheduler(ScheduledExecutorService scheduler) { + this.scheduler = scheduler; + return this; + } + + public StatsDMetricsMonitorBuilder setMetricsFilter(MetricsFilter metricsFilter) { + this.metricsFilter = metricsFilter; + return this; + } + + public StatsDMetricsMonitor createStatsDMetricsMonitor() { + return new StatsDMetricsMonitor(host, port, autoRegisterMetrics, prefix, naming, gaugeSendPeriod, scheduler, metricsFilter); + } + + public StatsDMetricsMonitor createStatsDMetricsMonitor(StatsDMetricsMonitor monitor, String... newNames) { + return new StatsDMetricsMonitor(monitor, gaugeSendPeriod, scheduler, newNames); + } + + public StatsDMetricsMonitor createStatsDMetricsMonitor(StatsDMetricsMonitor monitor, MetricsFilter metricsFilter, String... newNames) { + return new StatsDMetricsMonitor(monitor, gaugeSendPeriod, scheduler, metricsFilter, newNames); + } + + private static Duration getDefaultGaugeSendPeriod() { + return Duration.ofSeconds(1); + } + + private static ScheduledExecutorService createScheduler() { + return Executors.newScheduledThreadPool(2); + } +} \ No newline at end of file From 92718f27a66fda0c44363b5de1d3150e8a4be10c Mon Sep 17 00:00:00 2001 From: Karel Fajkus Date: Tue, 9 Oct 2018 12:48:02 +0200 Subject: [PATCH 2/3] review fixes, added deprecated annotations --- .../metrics/statsd/StatsDMetricsMonitor.java | 16 ++- .../statsd/StatsDMetricsMonitorBuilder.java | 108 +++++++----------- 2 files changed, 54 insertions(+), 70 deletions(-) diff --git a/statsd/src/main/java/com/avast/metrics/statsd/StatsDMetricsMonitor.java b/statsd/src/main/java/com/avast/metrics/statsd/StatsDMetricsMonitor.java index 18aab13..0f25f0f 100755 --- a/statsd/src/main/java/com/avast/metrics/statsd/StatsDMetricsMonitor.java +++ b/statsd/src/main/java/com/avast/metrics/statsd/StatsDMetricsMonitor.java @@ -50,6 +50,10 @@ public StatsDMetricsMonitor(String host, int port, String prefix, final Naming n this(host, port, prefix, naming, gaugeSendPeriod, scheduler, MetricsFilter.ALL_ENABLED); } + /** + * @deprecated As caller should be owner of thread pool. + */ + @Deprecated public StatsDMetricsMonitor(String host, int port, String prefix, final Naming naming) { this(host, port, prefix, naming, getDefaultGaugeSendPeriod(), createScheduler()); } @@ -58,11 +62,20 @@ public StatsDMetricsMonitor(String host, int port, String prefix, final Duration this(host, port, prefix, Naming.defaultNaming(), gaugeSendPeriod, scheduler); } + /** + * @deprecated As caller should be owner of thread pool. + * + */ + @Deprecated public StatsDMetricsMonitor(String host, int port, String prefix) { this(host, port, prefix, Naming.defaultNaming(), getDefaultGaugeSendPeriod(), createScheduler()); } - + /** + * @deprecated As caller should be owner of thread pool. + * + */ + @Deprecated public StatsDMetricsMonitor(String host, int port, String prefix, MetricsFilter metricsFilter) { this(host, port, prefix, Naming.defaultNaming(), getDefaultGaugeSendPeriod(), createScheduler(), metricsFilter); } @@ -218,6 +231,7 @@ public boolean isAutoRegisterMetric() { @Override public void close() { + // TODO: Remove when constructors without scheduler will be removed scheduler.shutdown(); client.stop(); } diff --git a/statsd/src/main/java/com/avast/metrics/statsd/StatsDMetricsMonitorBuilder.java b/statsd/src/main/java/com/avast/metrics/statsd/StatsDMetricsMonitorBuilder.java index fe916ce..be99620 100644 --- a/statsd/src/main/java/com/avast/metrics/statsd/StatsDMetricsMonitorBuilder.java +++ b/statsd/src/main/java/com/avast/metrics/statsd/StatsDMetricsMonitorBuilder.java @@ -4,76 +4,46 @@ import com.avast.metrics.filter.MetricsFilter; import java.time.Duration; -import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; public class StatsDMetricsMonitorBuilder { - private String host; - private int port; - private boolean autoRegisterMetrics = false; - private String prefix; - private Naming naming = Naming.defaultNaming(); - private Duration gaugeSendPeriod = getDefaultGaugeSendPeriod(); - private ScheduledExecutorService scheduler = createScheduler(); - private MetricsFilter metricsFilter = MetricsFilter.ALL_ENABLED; - - public StatsDMetricsMonitorBuilder setHost(String host) { - this.host = host; - return this; - } - - public StatsDMetricsMonitorBuilder setPort(int port) { - this.port = port; - return this; - } - - public StatsDMetricsMonitorBuilder setAutoRegisterMetrics(boolean autoRegisterMetrics) { - this.autoRegisterMetrics = autoRegisterMetrics; - return this; - } - - public StatsDMetricsMonitorBuilder setPrefix(String prefix) { - this.prefix = prefix; - return this; - } - - public StatsDMetricsMonitorBuilder setNaming(Naming naming) { - this.naming = naming; - return this; - } - - public StatsDMetricsMonitorBuilder setGaugeSendPeriod(Duration gaugeSendPeriod) { - this.gaugeSendPeriod = gaugeSendPeriod; - return this; - } - - public StatsDMetricsMonitorBuilder setScheduler(ScheduledExecutorService scheduler) { - this.scheduler = scheduler; - return this; - } - - public StatsDMetricsMonitorBuilder setMetricsFilter(MetricsFilter metricsFilter) { - this.metricsFilter = metricsFilter; - return this; - } - - public StatsDMetricsMonitor createStatsDMetricsMonitor() { - return new StatsDMetricsMonitor(host, port, autoRegisterMetrics, prefix, naming, gaugeSendPeriod, scheduler, metricsFilter); - } - - public StatsDMetricsMonitor createStatsDMetricsMonitor(StatsDMetricsMonitor monitor, String... newNames) { - return new StatsDMetricsMonitor(monitor, gaugeSendPeriod, scheduler, newNames); - } - - public StatsDMetricsMonitor createStatsDMetricsMonitor(StatsDMetricsMonitor monitor, MetricsFilter metricsFilter, String... newNames) { - return new StatsDMetricsMonitor(monitor, gaugeSendPeriod, scheduler, metricsFilter, newNames); - } - - private static Duration getDefaultGaugeSendPeriod() { - return Duration.ofSeconds(1); - } - - private static ScheduledExecutorService createScheduler() { - return Executors.newScheduledThreadPool(2); - } + private final String host; + private final int port; + private final String prefix; + private final ScheduledExecutorService scheduler; + private boolean autoRegisterMetrics = false; + private Naming naming = Naming.defaultNaming(); + private Duration gaugeSendPeriod = Duration.ofSeconds(1); + private MetricsFilter metricsFilter = MetricsFilter.ALL_ENABLED; + + public StatsDMetricsMonitorBuilder(String host, int port, String prefix, final ScheduledExecutorService scheduler) { + this.host = host; + this.port = port; + this.prefix = prefix; + this.scheduler = scheduler; + } + + public StatsDMetricsMonitorBuilder withAutoRegisterMetrics(boolean autoRegisterMetrics) { + this.autoRegisterMetrics = autoRegisterMetrics; + return this; + } + + public StatsDMetricsMonitorBuilder withNaming(Naming naming) { + this.naming = naming; + return this; + } + + public StatsDMetricsMonitorBuilder withGaugeSendPeriod(Duration gaugeSendPeriod) { + this.gaugeSendPeriod = gaugeSendPeriod; + return this; + } + + public StatsDMetricsMonitorBuilder withMetricsFilter(MetricsFilter metricsFilter) { + this.metricsFilter = metricsFilter; + return this; + } + + public StatsDMetricsMonitor build() { + return new StatsDMetricsMonitor(host, port, autoRegisterMetrics, prefix, naming, gaugeSendPeriod, scheduler, metricsFilter); + } } \ No newline at end of file From 3ac264f98de5b9ff4ad84f08668c32e7c09e02e5 Mon Sep 17 00:00:00 2001 From: Karel Fajkus Date: Tue, 9 Oct 2018 13:35:01 +0200 Subject: [PATCH 3/3] changed deprecated message --- .../metrics/statsd/StatsDMetricsMonitor.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/statsd/src/main/java/com/avast/metrics/statsd/StatsDMetricsMonitor.java b/statsd/src/main/java/com/avast/metrics/statsd/StatsDMetricsMonitor.java index 0f25f0f..a52cf0e 100755 --- a/statsd/src/main/java/com/avast/metrics/statsd/StatsDMetricsMonitor.java +++ b/statsd/src/main/java/com/avast/metrics/statsd/StatsDMetricsMonitor.java @@ -51,7 +51,9 @@ public StatsDMetricsMonitor(String host, int port, String prefix, final Naming n } /** - * @deprecated As caller should be owner of thread pool. + * @deprecated This constructor is only for backward compatibility and should be no longer used. + * It internally builds an instance of scheduler that will be never shutted down and causes threads leak. + * Prefer to use builder or other constructors. */ @Deprecated public StatsDMetricsMonitor(String host, int port, String prefix, final Naming naming) { @@ -63,8 +65,9 @@ public StatsDMetricsMonitor(String host, int port, String prefix, final Duration } /** - * @deprecated As caller should be owner of thread pool. - * + * @deprecated This constructor is only for backward compatibility and should be no longer used. + * It internally builds an instance of scheduler that will be never shutted down and causes threads leak. + * Prefer to use builder or other constructors. */ @Deprecated public StatsDMetricsMonitor(String host, int port, String prefix) { @@ -72,8 +75,9 @@ public StatsDMetricsMonitor(String host, int port, String prefix) { } /** - * @deprecated As caller should be owner of thread pool. - * + * @deprecated This constructor is only for backward compatibility and should be no longer used. + * It internally builds an instance of scheduler that will be never shutted down and causes threads leak. + * Prefer to use builder or other constructors. */ @Deprecated public StatsDMetricsMonitor(String host, int port, String prefix, MetricsFilter metricsFilter) { @@ -231,8 +235,6 @@ public boolean isAutoRegisterMetric() { @Override public void close() { - // TODO: Remove when constructors without scheduler will be removed - scheduler.shutdown(); client.stop(); }