FrankChen021 commented on code in PR #18525:
URL: https://github.com/apache/druid/pull/18525#discussion_r3969882995
##########
extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisorIOConfig.java:
##########
@@ -77,6 +78,7 @@ public KafkaSupervisorIOConfig(
@JsonProperty("earlyMessageRejectionPeriod") Period
earlyMessageRejectionPeriod,
@JsonProperty("lateMessageRejectionStartDateTime") DateTime
lateMessageRejectionStartDateTime,
@JsonProperty("configOverrides") KafkaConfigOverrides configOverrides,
+ @JsonProperty("headerBasedFilterConfig") KafkaHeaderBasedFilterConfig
headerBasedFilterConfig,
Review Comment:
[P2] Preserve the existing public constructor
Adding `headerBasedFilterConfig` to this public constructor removes the
previous JVM descriptor, so callers compiled against the existing Kafka
indexing-service API fail with `NoSuchMethodError` (and source callers stop
compiling) even when filtering is unused. Keep the old overload delegating
`null`, as this change already does for `KafkaIndexTaskIOConfig` and
`OrderedPartitionableRecord`.
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/stats/TaskRealtimeMetricsMonitor.java:
##########
@@ -105,6 +105,9 @@ public boolean doMonitor(ServiceEmitter emitter)
emitter.emit(builder.setMetric("ingest/events/processed",
rowIngestionMetersTotals.getProcessed() -
previousRowIngestionMetersTotals.getProcessed()));
+ final long filtered = rowIngestionMetersTotals.getFiltered() -
previousRowIngestionMetersTotals.getFiltered();
+ emitter.emit(builder.setMetric("ingest/events/filtered", filtered));
Review Comment:
[P2] Update the default logging-emitter allowlist
`LoggingEmitter` drops every service metric not present in its bundled
`processing/src/main/resources/loggingEmitterAllowedMetrics.json` when
`shouldFilterMetrics` is enabled without a custom allowlist. This new
`ingest/events/filtered` metric is not in that resource, so the documented
metric silently disappears for that supported configuration even though the
other emitter defaults were updated. Add it to the bundled allowlist and cover
the default-filtered path.
--
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]
