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

Reply via email to