Prometheus per label ttl by aho135 · Pull Request #18689 · apache/druid

aho135 · 2025-10-23T23:37:16Z

This is an extension of #18598 to enhance the TTL to track metrics at the label level instead of just the metric level.

Description

This PR enhances the current metric TTL tracking in the prometheus-emitter so that all unique label combinations are tracked instead of just the metric name. This is done using a ConcurrentHashMap which maintains a mapping of label values to a Stopwatch. The map needs to handle concurrent updates because there is a possibility for the TTL scheduler to remove a key from a map at the same that the key is updated during metric emission. These are handled through the two atomic operations removeIfExpired and resetLastUpdateTime

Release note

Enhances the prometheus-emitter metric TTL tracking to consider all label value combinations instead of just the metric name.

Key changed/added classes in this PR

PrometheusEmitter
PrometheusEmitterTest
DimensionsAndCollector

This PR has:

abhishekrb19

Thanks @aho135! Did a high level review and left some thoughts

abhishekrb19 · 2025-10-25T00:40:52Z

@@ -33,7 +37,7 @@ public class DimensionsAndCollector
  private final SimpleCollector collector;
  private final double conversionFactor;
  private final double[] histogramBuckets;
-  private final Stopwatch updateTimer;
+  private final ConcurrentHashMap<List<String>, Stopwatch> labelValuesToStopwatch;


Did you observe any memory pressure with this change? Previously there were some concerns on the memory usage with this approach, so it would be nice if you could share any observations

I haven't done any in depth memory profiling for this. I think it largely depends on how many unique label combinations there are. For something like queryId I'd imagine the memory usage would be higher under heavy query load. But for dimensions like taskId the memory usage should be much lower. I think it roughly gets offset anyway because we are clearing the label from the underlying SimpleCollector map now

abhishekrb19 · 2025-10-25T00:51:14Z

-            metric.getMillisSinceLastUpdate()
-        );
-        metric.getCollector().clear();
+      Set<List<String>> labelValuesCopy = new HashSet<>(metric.getLabelValuesToStopwatch().keySet());


Is a copy of the map required here if the underlying map is already concurrent safe from using ConcurrentMap? We're also doing computeIfPresent() on the map entries whihh should be safe

Yeah good point. Felt a little dangerous to be iterating over while removing keys, but I think the consistency guarantees of the ConcurrentMap make this safe

abhishekrb19 · 2025-10-25T01:06:44Z

+        "Metric should be expired",
+        testMetric.removeIfExpired(Arrays.asList("historical", "druid.test.cn", "historical1"))
+    );
+    Assert.assertFalse(


Thanks for these tests!

From a functional PoV, it would be better to verify the end state of the collector, that the metrics are indeed cleared, rather than calling the internal function removeIfExpired(); as it is, I think we don't have coverage for cleanUpStaleMetrics().

What do you think of something like:

emitter.emit(metric1 - labelset1)

emitter.emit(metric1 - labelset2)

emitter.emit(metric1 - labelset1)

verify that metric1 - labelset1 is not cleared from the collector, whereas metric1 - labelset2 is cleared.

I think we could leverage the SimpleCollector's samples for verification. We could also pass BlockingExecutorService from the test to the prometheus emitter to control the executions (a bunch of tests do this to have better control of async executors). Please let me know what you think.

Yeah agreed. Ideally I'd wanted to check SimpleCollector.children and ensure that the labels were removed by cleanUpStaleMetrics(). Unfortunately the SimpleCollector doesn't expose any function to access the children Map. I did verify via the debugger that the entry is removed from the child map after the TTL though

The tests do actually run cleanUpStaleMetrics() because the executor is created in emitter.start() and manual sleep times allow the scheduled executor to run

This test case does roughly model the behavior you suggested:

Except I update metric1 - labelset2 instead

emitter.emit(metric1 - labelset1)
emitter.emit(metric1 - labelset2)
emitter.emit(metric1 - labelset2)

Gotcha, the SimpleCollector exposes a collect() API to get the metrics collected/samples. Perhaps something like this should work?

List<Collector.MetricFamilySamples.Sample> samples = testMetric.getCollector().collect().get(0).samples; // assert the labels & the sample size is 1 during a valid period

After the TTL period, the sample size would be 0? I think that would give us sufficient functionality validation on cleanUpStaleMetrics()

abhishekrb19 · 2025-10-31T20:53:05Z

+        "Metric should be expired",
+        testMetric.removeIfExpired(Arrays.asList("historical", "druid.test.cn", "historical1"))
+    );
+    Assert.assertFalse(


Gotcha, the SimpleCollector exposes a collect() API to get the metrics collected/samples. Perhaps something like this should work?

List<Collector.MetricFamilySamples.Sample> samples = testMetric.getCollector().collect().get(0).samples; // assert the labels & the sample size is 1 during a valid period

After the TTL period, the sample size would be 0? I think that would give us sufficient functionality validation on cleanUpStaleMetrics()

abhishekrb19 · 2025-10-31T20:53:30Z

 | `druid.emitter.prometheus.addServiceAsLabel`  | Flag to include the druid service name (e.g. `druid/broker`, `druid/coordinator`, etc.) as a prometheus label.                                                                                                                         | no        | false                                |
 | `druid.emitter.prometheus.pushGatewayAddress` | Pushgateway address. Required if using `pushgateway` strategy.                                                                                                                                                                         | no        | none                                 |
-| `druid.emitter.prometheus.flushPeriod`        | When using the `pushgateway` strategy metrics are emitted every `flushPeriod` seconds. <br/>When using the `exporter` strategy this configures the metric TTL such that if the metric value is not updated within `flushPeriod` seconds then it will stop being emitted. Note that unique label combinations per metric are currently not subject to TTL expiration. It is recommended to set this to at least 3 * `scrape_interval`. | Required if `pushgateway` strategy is used, optional otherwise. | 15 seconds for `pushgateway` strategy. <br/>None for `exporter` strategy. |
+| `druid.emitter.prometheus.flushPeriod`        | When using the `pushgateway` strategy metrics are emitted every `flushPeriod` seconds. <br/>When using the `exporter` strategy this configures the metric TTL such that if the metric value is not updated within `flushPeriod` seconds then it will stop being emitted. It is recommended to set this to at least 3 * `scrape_interval`. | Required if `pushgateway` strategy is used, optional otherwise. | 15 seconds for `pushgateway` strategy. <br/>None for `exporter` strategy. |


Should we perhaps amend this to say that this reset also applies to unique label combinations?

abhishekrb19

Lgtm, some minor comments. Thanks, @aho135!

abhishekrb19 · 2025-11-04T17:45:42Z

@aho135 when you get a chance, could you please pull the latest master to this branch? That should fix the IT failures due to openjdk version.

abhishekrb19 · 2025-11-04T22:58:08Z

The failing embedded test is unrelated to this PR. It's been flakey on a few other PRs, a retry didn't help:

Error:  Tests run: 5, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 246.174 s <<< FAILURE! - in org.apache.druid.testing.embedded.docker.IngestionDockerTest
Error:  test_runIndexTask_andKillData  Time elapsed: 128.148 s  <<< ERROR!
org.apache.druid.java.util.common.ISE: Timed out waiting for event

cc: @kfaraz

kfaraz · 2025-11-05T08:45:15Z

Thanks, @abhishekrb19 ! I think this test flakes out every now and then.
Will try to debug this soon.

…8718) Follow up to #18689 to not track labels if the TTL isn't set Co-authored-by: Andrew Ho <[email protected]>

Fixes apache#14638 This is an extension of apache#18598 to enhance the TTL to track metrics at the label level instead of just the metric level. This should allow operators to configure medium cardinality dimensions on the prometheus-emitter without putting pressure on the Prometheus collector/sub-system.

…ache#18718) Follow up to apache#18689 to not track labels if the TTL isn't set Co-authored-by: Andrew Ho <[email protected]>

Andrew Ho added 7 commits October 20, 2025 19:35

Update prometheus-emitter to track TTL for each label combination

7d1e647

Remove unused import

bb6f26a

Null check for isExpired

ad11f41

Log out label values

bbd9b4d

Update doc to reflect new behavior

8085685

Throw DruidException.defensive for invalid usage of isExpired

db67dac

Concurrency updates

e458d33

github-actions Bot added Area - Documentation Area - Metrics/Event Emitting labels Oct 23, 2025

Add javadoc

33e8cbe

kfaraz requested review from abhishekrb19 and kfaraz October 24, 2025 07:52

Fix function name

794e6c9

abhishekrb19 reviewed Oct 25, 2025

View reviewed changes

Remove copy of labelValues, rename function

535d140

abhishekrb19 reviewed Oct 31, 2025

View reviewed changes

Andrew Ho and others added 4 commits October 31, 2025 17:25

Unit test improvements to check that the Collector is properly cleared

5fb81f2

Unit test fixes

f1f42c6

Switch from execute to submit

f277ddc

Constructor cleanup

544f68c

abhishekrb19 approved these changes Nov 3, 2025

View reviewed changes

Doc and test cleanup

08463bf

Merge branch 'apache:master' into prometheus-per-label-ttl

d05b368

abhishekrb19 merged commit 5f70c83 into apache:master Nov 4, 2025
88 of 90 checks passed

aho135 mentioned this pull request Nov 5, 2025

prometheus-emitter: disable label tracking when the TTL isn't set #18718

Merged

9 tasks

abhishekrb19 pushed a commit that referenced this pull request Nov 5, 2025

prometheus-emitter: disable label tracking when the TTL isn't set (#1…

33a9c29

…8718) Follow up to #18689 to not track labels if the TTL isn't set Co-authored-by: Andrew Ho <[email protected]>

kgyrtkirk added this to the 36.0.0 milestone Jan 19, 2026

Uh oh!

Conversation

aho135 commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Release note

Key changed/added classes in this PR

Uh oh!

abhishekrb19 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aho135 Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abhishekrb19 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abhishekrb19 commented Nov 4, 2025

Uh oh!

abhishekrb19 commented Nov 4, 2025

Uh oh!

Uh oh!

kfaraz commented Nov 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aho135 commented Oct 23, 2025 •

edited

Loading

aho135 Oct 27, 2025 •

edited

Loading