kevin-wu24 commented on code in PR #20021:
URL: https://github.com/apache/kafka/pull/20021#discussion_r2193382168
##########
metadata/src/main/java/org/apache/kafka/image/loader/MetadataLoader.java:
##########
@@ -345,7 +347,23 @@ private void maybePublishMetadata(MetadataDelta delta,
MetadataImage image, Load
}
}
metrics.updateLastAppliedImageProvenance(image.provenance());
-
metrics.setCurrentMetadataVersion(image.features().metadataVersionOrThrow());
+ MetadataVersion metadataVersion =
image.features().metadataVersionOrThrow();
+ metrics.setCurrentMetadataVersion(metadataVersion);
+
+ // Set the metadata version feature level, since it is handled
separately from other features
+ metrics.recordFinalizedFeatureLevel(
+ MetadataVersion.FEATURE_NAME,
+ metadataVersion.featureLevel()
+ );
+
+ // Set all production feature levels from the image, defaulting to
their minimum production values
+ for (var feature : Feature.PRODUCTION_FEATURES) {
Review Comment:
> This also means that if the finalized feature version is "removed" (set to
0), the code needs to remove the associated metric.
I guess this applies to all features besides metadata.version (whose minimum
level is 7), and kraft.version (whose minimum is 0, but 0 does not mean that
KRaft is "disabled" like other features). Since these two features are never
part of the features image, I think their associated metrics should never be
removed. Other features are removed from the image when their level is set to
0, so I think it makes sense to remove their metrics too. I'm going to update
the KIP to document the exceptions for metadata + kraft version.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
