Fix logging emitter resource packaging and rename to avoid classpath collision#19358
Closed
sarangv wants to merge 5 commits intoapache:masterfrom
Closed
Fix logging emitter resource packaging and rename to avoid classpath collision#19358sarangv wants to merge 5 commits intoapache:masterfrom
sarangv wants to merge 5 commits intoapache:masterfrom
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #19030 (follow-up).
Description
Follow-up fix for PR #19030 (
add defaultMetrics for logging emitter), which introduced metric filtering for theLoggingEmitter. Two issues were discovered during deployment testing in a real Druid cluster:Fixed resource packaging in processing/pom.xml
The
processing/pom.xmlalready had a<resources>block that only included the Sigar native library directory. When Maven sees any explicit<resources>section, it stops automatically including the defaultsrc/main/resourcesdirectory. This meantdefaultMetrics.jsonwas never packaged into the processing JAR, causing aDruidException(NOT_FOUND) at runtime whenLoggingEmitterattempted to load it viagetResourceAsStream().The fix adds
<resource><directory>src/main/resources</directory></resource>explicitly alongside the existing Sigar entry.Renamed defaultMetrics.json to loggingEmitterAllowedMetrics.json
Both the
processingmodule and theextensions-contrib/prometheus-emittermodule ship a classpath resource calleddefaultMetrics.json, but they have completely different schemas:dimensions,type,conversionFactor,helpfields[]valuesDepending 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 tologgingEmitterAllowedMetrics.jsongives 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.jsontologgingEmitterAllowedMetrics.json. No configuration changes are needed -- the emitter automatically loads the correct resource. Thedruid.emitter.logging.allowedMetricsPathproperty continues to work as documented for custom allowlist files.Key changed/added classes in this PR
LoggingEmitter-- updated resource constant and Javadoc to referenceloggingEmitterAllowedMetrics.jsonLoggingEmitterConfig-- updated Javadoc to referenceloggingEmitterAllowedMetrics.jsonprocessing/pom.xml-- added explicitsrc/main/resourcesto<resources>blockprocessing/src/main/resources/loggingEmitterAllowedMetrics.json-- renamed fromdefaultMetrics.jsonprocessing/src/test/resources/loggingEmitterAllowedMetrics.json-- renamed fromdefaultMetrics.jsonThis PR has: