kfaraz commented on code in PR #18598:
URL: https://github.com/apache/druid/pull/18598#discussion_r2406955922
##########
extensions-contrib/prometheus-emitter/src/main/java/org/apache/druid/emitter/prometheus/DimensionsAndCollector.java:
##########
@@ -20,20 +20,23 @@
package org.apache.druid.emitter.prometheus;
import io.prometheus.client.SimpleCollector;
+import java.util.concurrent.atomic.AtomicLong;
public class DimensionsAndCollector
{
private final String[] dimensions;
private final SimpleCollector collector;
private final double conversionFactor;
private final double[] histogramBuckets;
+ private final AtomicLong lastUpdateTime;
Review Comment:
You can use a `org.apache.druid.java.util.common.Stopwatch` instead.
In `updateLastUpdateTime`, you can `restart` the stopwatch.
In `isExpired`, you can invoke `stopwatch.hasElapsed()`.
##########
extensions-contrib/prometheus-emitter/src/main/java/org/apache/druid/emitter/prometheus/PrometheusEmitter.java:
##########
@@ -140,6 +153,7 @@ private void emitMetric(ServiceMetricEvent metricEvent)
DimensionsAndCollector metric = metrics.getByName(name, service);
if (metric != null) {
+ metric.updateLastUpdateTime();
Review Comment:
Update the time after the respective collector has been actually updated.
##########
extensions-contrib/prometheus-emitter/src/main/java/org/apache/druid/emitter/prometheus/DimensionsAndCollector.java:
##########
@@ -55,4 +58,20 @@ public double[] getHistogramBuckets()
{
return histogramBuckets;
}
+
+ public void updateLastUpdateTime()
Review Comment:
```suggestion
public void resetLastUpdateTime()
```
##########
extensions-contrib/prometheus-emitter/src/main/java/org/apache/druid/emitter/prometheus/PrometheusEmitter.java:
##########
@@ -256,4 +278,21 @@ public void setPushGateway(PushGateway pushGateway)
{
this.pushGateway = pushGateway;
}
+
+ private void checkMetricTtl()
Review Comment:
```suggestion
private void cleanUpStaleMetrics()
```
##########
extensions-contrib/prometheus-emitter/src/main/java/org/apache/druid/emitter/prometheus/PrometheusEmitter.java:
##########
@@ -93,6 +94,18 @@ public void start()
} else {
log.error("HTTPServer is already started");
}
+ // Start TTL scheduler if TTL is configured
+ if (config.getMetricTtlMs() != null) {
+ ttlExecutor = ScheduledExecutors.fixed(1, "PrometheusTTLExecutor-%s");
+ // Check TTL every minute
+ ttlExecutor.scheduleAtFixedRate(
Review Comment:
We should reuse the existing `exec`.
With `exporter` strategy, it will be used to clean up stale metrics.
With `pushgateway` strategy, it will be used to push metrics.
##########
extensions-contrib/prometheus-emitter/src/main/java/org/apache/druid/emitter/prometheus/PrometheusEmitterConfig.java:
##########
@@ -77,6 +77,10 @@ public class PrometheusEmitterConfig
@JsonProperty
private final Duration waitForShutdownDelay;
+ @JsonProperty
+ @Nullable
+ private final Long metricTtlMs;
Review Comment:
We should use the existing `flushPeriod` config.
In `exporter` mode, it will represent the TTL.
In `pushgateway` mode, it will represent the push period.
We can update the docs to reflect this detail.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
