[GitHub] [samza] prateekm commented on a change in pull request #1223: SAMZA-2397: Updating gauge-val function on newGauge on same metric name

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

prateekm commented on a change in pull request #1223: SAMZA-2397: Updating gauge-val function on newGauge on same metric name
URL: https://github.com/apache/samza/pull/1223#discussion_r353353131
 
 

 ##########
 File path: samza-api/src/main/java/org/apache/samza/metrics/MetricsRegistry.java
 ##########
 @@ -64,6 +64,17 @@
    */
   <T> Gauge<T> newGauge(String group, Gauge<T> value);
 
+  /**
+   * Register an existing {@link org.apache.samza.metrics.Gauge}
+   * @param group Group for this Gauge
+   * @param value Initial value for the Gauge
+   * @param <T> Type the Gauge will be wrapping
+   * @param overrideExistingGauge Overwrite any existing gauges present for the same group and gauge
+   * @return Gauge was registered
+   */
+  <T> Gauge<T> newGauge(String group, Gauge<T> value, Boolean overrideExistingGauge);
 
 Review comment:
   I'd prefer to not add a new variant here and update the existing behavior instead. It seems to fall into "fixing undefined behavior" territory, in that I don't think a reasonable user expectation would be to retain the old metric. We can fix this here and call it out in the "backwards incompatible / API changes" section of the PR description.
   
   @cameronlee314 What do you think?

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


With regards,
Apache Git Services