[GitHub] [samza] rmatharu commented on a change in pull request #1406: SAMZA-2561: Add config map to DiagnosticsManager

[GitBox <gi...@apache.org>]

rmatharu commented on a change in pull request #1406:
URL: https://github.com/apache/samza/pull/1406#discussion_r465383565



##########
File path: samza-core/src/main/scala/org/apache/samza/diagnostics/DiagnosticsStreamMessage.java
##########
@@ -155,6 +158,14 @@ public void addProcessorStopEvents(List<ProcessorStopEvent> stopEventList) {
     }
   }
 
+  /**
+   * Add the job's config to the message.
+   * @param config the config to add.
+   */
+  public void addConfig(Map<String, String> config) {

Review comment:
       Nit: The method definition could be 
   `public void addConfig(Config config) {
   addToMetricsMessage(GROUP_NAME_FOR_DIAGNOSTICS_MANAGER, CONFIG_METRIC_NAME, (Map<String, String>) config);
   }`
   
   So that the cast-to-map logic is encapsulated in this class; and not visible to callers.
   
   




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org