dev: Unified metric filtering logic across emitters by nozjkoitop · Pull Request #19026 · apache/druid

nozjkoitop · 2026-02-17T15:34:55Z

Extends #19030.

Description

This PR expands metric-name-based filtering beyond LoggingEmitter and introduces shared filtering support that can be reused across multiple emitters.

What changed

Added shared emitter metric-filtering config:
- shouldFilterMetrics
- metricSpecPath
Added common filtering support in processing:
- AbstractFilteringEmitter
- shared allowlist loading/parsing utilities
Updated LoggingEmitter, HttpPostEmitter, ParametrizedUriEmitter and PrometheusEmitter to use the shared filtering mechanism

Behavior

filtering is optional and disabled by default
when filtering is enabled, metric events are emitted only if their metric name is present in the configured allowlist
when no custom spec path is configured, the emitter uses its bundled default metric spec
when a custom spec path is configured, that file is used as the allowlist source
non-metric event handling remains emitter-specific

Release note

Added shared metric allowlist filtering support for multiple emitters. LoggingEmitter, HttpPostEmitter, ParametrizedUriEmitter, and PrometheusEmitter can optionally restrict emitted metric events to an allowlisted set of metric names using the shared filtering configuration.

This PR has:

been self-reviewed.
added documentation for new or modified features or behaviors.
a release note entry in the PR description.
added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
added or updated version, license, or notice information in licenses.yaml
added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
added integration tests.
been tested in a test Druid cluster.

abhishekrb19 · 2026-02-19T01:07:00Z

Thanks @nozjkoitop for the contribution! Cross-linking the comment here on a similar implementation - #19030 (review)

nozjkoitop · 2026-02-24T08:57:07Z

@abhishekrb19 quick ping, could you take a look at the latest updates here when you have a moment? I incorporated the schema + test coverage deltas discussed on #19030 (along with your comment from the review), so we can move this feature forward

abhishekrb19 · 2026-02-26T07:18:50Z

@nozjkoitop I will find time later this week to review these changes alongside #19030. Thanks!

jtuglu1 · 2026-03-05T20:29:01Z

Could we make this functionality generic to all types of emitters?

nozjkoitop · 2026-03-06T09:59:04Z

yea, lemme try it out
perhaps i can add a shared util to the emitters

nozjkoitop · 2026-03-06T17:22:13Z

added couple of emitters as a concept, @jtuglu1 wdyt?

jtuglu1 · 2026-03-06T23:03:45Z

cc @maytasm

abhishekrb19 · 2026-03-07T02:32:04Z

Could we make this functionality generic to all types of emitters?

Agreed, this will be broadly useful as a core concept for all emitters. Can we do this as a separate change though and scope this PR down to only add the functionality for the LoggingEmitter as mentioned in #19021?

Existing emitters like prometheus-emitter, dropwizard-emitter, opentsdb-emitter, etc. already have a notion of an allow list, so we’ll need to be careful not to break compatibility with them. IMO it will be better to come up with a common interface and enable it for all the emitters in a backwards-compatible manner to existing emitters in a separate change (or alternatively define the interface in this patch, but only integrate it with the LoggingEmitter, but I'd prefer that we do this separately too). Please let me know what you think @nozjkoitop @jtuglu1.

jtuglu1 · 2026-03-07T02:37:21Z

Existing emitters like prometheus-emitter, dropwizard-emitter, opentsdb-emitter, etc. already have a notion of an allow list, so we’ll need to be careful not to break compatibility with them.

Yes, and, ideally this can be extended so any new/existing emitter can "plugin" to the existing mechanism.

or alternatively define the interface in this patch, but only integrate it with the LoggingEmitter

I'd prefer this if possible, since we'd just be deferring the unavoidable unification of all these emitters' filtering mechanisms to down-the-line and I don't want to keep adding more risk that these various different emitter filtering mechanisms become too disparate and a "clean" interface is hard to do.

If possible, I'd like to define it in this PR (and let users of other emitters "opt-in" as necessary via changes down-the-line).

abhishekrb19

@nozjkoitop, thanks for the changes! I left some high-level comments on the common config that would apply to all emitters.

Since the scope of this PR has expanded and we’ve been porting some of the logging-emitter changes from #19030, I’ll go ahead and merge 19030 to keep the drift minimal and make things easier to review. Then we can repurpose this PR to extend the filtering functionality broadly across all emitter types, as @jtuglu1 suggested. Please let me know what you think.

abhishekrb19 · 2026-03-07T02:36:22Z

+public interface MetricAllowlistParser
+{
+  Set<String> parse(JsonNode metricConfig, String source);
+}


Please add javadocs for common interfaces.

abhishekrb19 · 2026-03-07T02:39:10Z

+public class GlobalEmitterConfig
+{
+  @JsonProperty
+  boolean filterMetrics;
+


Please add javadocs

abhishekrb19 · 2026-03-07T02:39:22Z

+  @JsonProperty
+  String metricAllowlistPath;
+


These properties can be private-scoped

abhishekrb19 · 2026-03-07T02:40:49Z

+    catch (IOException e) {
+      throw new ISE(e, "Failed to parse metric allowlist file [%s]", allowlistPath);


Please use DruidException - style guide ref: https://github.com/apache/druid/blob/master/dev/style-conventions.md#message-formatting-for-logs-and-exceptions

abhishekrb19 · 2026-03-07T02:44:38Z

+      throw new IAE("Metric allowlist file path is empty");
+    }
+    try {
+      return new FileInputStream(allowlistPath);
+    }
+    catch (FileNotFoundException e) {
+      final InputStream classpathInputStream = MetricAllowlistLoader.class.getClassLoader().getResourceAsStream(allowlistPath);
+      if (classpathInputStream != null) {
+        return classpathInputStream;
+      }
+      throw new IAE(e, "Metric allowlist file [%s] not found", allowlistPath);
+    }


DruidExceptions here as well similar to #19030:
https://github.com/apache/druid/pull/19030/changes#diff-e6a24d753a6939a848cd7909791a6ceaf358cb5976e7b3ed084912204778cb63R137-R145

abhishekrb19 · 2026-03-07T02:47:20Z

+    if (!metricConfig.isObject()) {
+      throw new ISE("Metric allowlist file [%s] must be a JSON object of metric names", source);
+    }
+    final ImmutableSet.Builder<String> metricNames = ImmutableSet.builder();


Does this work with the different defaultMetrics.json formats that emitters support (they look close enough but have slight differences). Could you also update some of the existing emitters like prometheus-emitter to exercise these code paths?

It does work with the json objects, as long as the metric names are the field names
In case of other structure there is an abstract getMetricAllowlistParser where custom parser can be provided

abhishekrb19 · 2026-03-07T02:58:29Z

+    if (shouldFilterOutMetric(name)) {
+      return;
+    }


Hmm, instead of doing this for every single emitter, what do you think about adding an abstract class that implements MetricFilteringEmitter implements Emitter and overrides emit() ? This class can have awareness about global configs instantiated from the specific sub-class Emitters like PrometheusEmitter, LoggingEmitter, etc and delegate the filtering to it if the config is enabled or just fall back to current behavior? I will think about this more. @jtuglu1 - do you have suggestions here?

added an AbstractFilteringEmitter with shouldFilterMetrics and allowedMetricNames

maytasm · 2026-03-09T00:00:36Z

Since @abhishekrb19 merged #19030, this PR will have to be rebased. There are also duplicate configs between the two PRs too. @nozjkoitop please comment here when the PR is ready for review. Thanks!

  {
    String namespace = config.getNamespace();
-    String path = config.getDimensionMapPath();
+    String path = config.getMetricSpecPath().orElse(config.getDimensionMapPath());


+        config.isShouldFilterMetrics(),
+        config.isShouldFilterMetrics()
+        ? config.getMetricSpecPath()
+                .or(() -> Optional.ofNullable(config.getDimensionMapPath()))


…gging-filtering

nozjkoitop · 2026-03-11T13:41:04Z

@maytasm latest master is merged, please take a look when you have some time

nozjkoitop · 2026-03-26T09:13:26Z

Hi @abhishekrb19, following up on this PR. It has been rebased and updated based on the latest review comments, so I’d appreciate another look when you get a chance.

metrics filtering in LoggingEmitter

d3768ad

github-actions Bot added Area - Documentation Area - Metrics/Event Emitting Area - Dependencies labels Feb 17, 2026

static checks fix

618a540

abhishekrb19 mentioned this pull request Feb 19, 2026

add defaultMetrics for logging emitter #19030

Merged

9 tasks

nozjkoitop added 3 commits February 19, 2026 08:28

fix review comments

716d337

fix test

db8a01f

imports cleanup

1e000d0

WIP unified emitter metric allowlist filtering

e4ac542

jtuglu1 self-requested a review March 6, 2026 20:28

abhishekrb19 reviewed Mar 7, 2026

View reviewed changes

maytasm self-requested a review March 8, 2026 22:59

nozjkoitop added 4 commits March 10, 2026 10:55

master merged

6c4ec3a

post-merge alignment

7da8cd9

address review comments

2a549fd

cleanup

91567b7

github-advanced-security AI found potential problems Mar 10, 2026

View reviewed changes

nozjkoitop added 2 commits March 11, 2026 11:09

removed redundand event param from preEmit

9a75e8d

fix test

c41f754

nozjkoitop added 2 commits March 11, 2026 11:32

Merge remote-tracking branch 'upstream/master' into feature-metric-lo…

0db84d3

…gging-filtering

fix resources

0631aba

nozjkoitop requested a review from abhishekrb19 March 16, 2026 13:03

merged upstream

01cee9c

nozjkoitop changed the title ~~Enabled filtering of metrics in the LoggingEmitter~~ dev: Enabled filtering of metrics in the LoggingEmitter Mar 26, 2026

nozjkoitop changed the title ~~dev: Enabled filtering of metrics in the LoggingEmitter~~ dev: Unified metric filtering logic for emitters Mar 26, 2026

nozjkoitop changed the title ~~dev: Unified metric filtering logic for emitters~~ dev: Unified metric filtering logic across emitters Mar 26, 2026

		catch (IOException e) {
		throw new ISE(e, "Failed to parse metric allowlist file [%s]", allowlistPath);

Conversation

nozjkoitop commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

What changed

Behavior

Release note

Uh oh!

abhishekrb19 commented Feb 19, 2026

Uh oh!

nozjkoitop commented Feb 24, 2026

Uh oh!

abhishekrb19 commented Feb 26, 2026

Uh oh!

jtuglu1 commented Mar 5, 2026

Uh oh!

nozjkoitop commented Mar 6, 2026

Uh oh!

nozjkoitop commented Mar 6, 2026

Uh oh!

jtuglu1 commented Mar 6, 2026

Uh oh!

abhishekrb19 commented Mar 7, 2026

Uh oh!

jtuglu1 commented Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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 Mar 7, 2026 • 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!

maytasm commented Mar 9, 2026

Uh oh!

Check notice

Uh oh!

Check notice

Uh oh!

Uh oh!

nozjkoitop commented Mar 11, 2026

Uh oh!

nozjkoitop commented Mar 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nozjkoitop commented Feb 17, 2026 •

edited

Loading

jtuglu1 commented Mar 7, 2026 •

edited

Loading

abhishekrb19 Mar 7, 2026 •

edited

Loading