Skip to content

fix: package logging emitter resource and rename to avoid classpath issues#19359

Merged
abhishekrb19 merged 2 commits intoapache:masterfrom
sarangv:logging-emitter-fix
Apr 22, 2026
Merged

fix: package logging emitter resource and rename to avoid classpath issues#19359
abhishekrb19 merged 2 commits intoapache:masterfrom
sarangv:logging-emitter-fix

Conversation

@sarangv
Copy link
Copy Markdown
Contributor

@sarangv sarangv commented Apr 21, 2026

Fixes #19030 (follow-up).

Description

Follow-up fix for PR #19030 (add defaultMetrics for logging emitter), which introduced metric filtering for the LoggingEmitter. Two issues were discovered during deployment testing in a real Druid cluster:

Fixed resource packaging in processing/pom.xml

The processing/pom.xml already had a <resources> block that only included the Sigar native library directory. When Maven sees any explicit <resources> section, it stops automatically including the default src/main/resources directory. This meant defaultMetrics.json was never packaged into the processing JAR, causing a DruidException (NOT_FOUND) at runtime when LoggingEmitter attempted to load it via getResourceAsStream().

The fix adds <resource><directory>src/main/resources</directory></resource> explicitly alongside the existing Sigar entry.

Renamed defaultMetrics.json to loggingEmitterAllowedMetrics.json

Both the processing module and the extensions-contrib/prometheus-emitter module ship a classpath resource called defaultMetrics.json, but they have completely different schemas:

  • Prometheus emitter: objects with dimensions, type, conversionFactor, help fields
  • Logging emitter: flat object with metric name keys and empty [] values

Depending on classloader ordering at runtime, LoggingEmitter.class.getClassLoader().getResourceAsStream("defaultMetrics.json") could resolve to the Prometheus emitter's file instead, causing a silent misconfiguration or parse failure. Renaming to loggingEmitterAllowedMetrics.json gives the logging emitter a unique resource name with no collision risk.

Release note

Operators using the logging emitter metric filtering feature introduced in #19030 should note that the bundled default allowlist resource has been renamed from defaultMetrics.json to loggingEmitterAllowedMetrics.json. No configuration changes are needed -- the emitter automatically loads the correct resource. The druid.emitter.logging.allowedMetricsPath property continues to work as documented for custom allowlist files.


Key changed/added classes in this PR
  • LoggingEmitter -- updated resource constant and Javadoc to reference loggingEmitterAllowedMetrics.json
  • LoggingEmitterConfig -- updated Javadoc to reference loggingEmitterAllowedMetrics.json
  • processing/pom.xml -- added explicit src/main/resources to <resources> block
  • processing/src/main/resources/loggingEmitterAllowedMetrics.json -- renamed from defaultMetrics.json
  • processing/src/test/resources/loggingEmitterAllowedMetrics.json -- renamed from defaultMetrics.json

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 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.
  • been tested in a test Druid cluster.

@abhishekrb19 abhishekrb19 changed the title Fix logging emitter resource packaging and rename to avoid classpath issues fix: package logging emitter resource and rename to avoid classpath issues Apr 21, 2026
@abhishekrb19 abhishekrb19 added this to the 37.0.0 milestone Apr 21, 2026
Copy link
Copy Markdown
Contributor

@abhishekrb19 abhishekrb19 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix, @sarangv.
I’ve tagged the PR with the 37.0.0 milestone so we can include this in the upcoming release.
cc: @cecemei

…rics.json

The previous fix broadly added src/main/resources as a resource directory,
which inadvertently packaged log4j2.xml, log4j2.debug.xml, and the
services/javax.annotation.processing.Processor file into druid-processing.jar.

The packaged log4j2.xml (console-only) ends up on the classpath of dependent
modules like indexing-service and shadows processing's test log4j2.xml that
contains the RoutingAppender used to route task logs to per-task files. This
caused ThreadingTaskRunnerTest#test_streamTaskLogs_ofRunningTask_readsFromTaskLogFile
to fail because the per-task log file was never written.

Narrow the include to only loggingEmitterAllowedMetrics.json so behavior for
every other file in processing/src/main/resources matches pre-PR state.
@abhishekrb19 abhishekrb19 merged commit 2de75ab into apache:master Apr 22, 2026
113 of 116 checks passed
@github-actions github-actions Bot modified the milestones: 37.0.0, 38.0.0 Apr 22, 2026
@cecemei cecemei mentioned this pull request Apr 23, 2026
10 tasks
cecemei pushed a commit that referenced this pull request Apr 24, 2026
…ssues (#19359) (#19371)

The previous fix broadly added src/main/resources as a resource directory,
which inadvertently packaged log4j2.xml, log4j2.debug.xml, and the
services/javax.annotation.processing.Processor file into druid-processing.jar.

The packaged log4j2.xml (console-only) ends up on the classpath of dependent
modules like indexing-service and shadows processing's test log4j2.xml that
contains the RoutingAppender used to route task logs to per-task files. This
caused ThreadingTaskRunnerTest#test_streamTaskLogs_ofRunningTask_readsFromTaskLogFile
to fail because the per-task log file was never written.

Narrow the include to only loggingEmitterAllowedMetrics.json so behavior for
every other file in processing/src/main/resources matches pre-PR state.


(cherry picked from commit 2de75ab)

Co-authored-by: sarangv <sarang.vadali@gmail.com>
Co-authored-by: Sarang Vadali <sarangvadali@salesforce.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants