[GitHub] [lucene-solr] noblepaul commented on a change in pull request #825: SOLR-13677 All Metrics Gauges should be unregistered by the objects that registered them

2019-08-13 Thread GitBox
noblepaul commented on a change in pull request #825: SOLR-13677 All Metrics 
Gauges should be unregistered by the objects that registered them
URL: https://github.com/apache/lucene-solr/pull/825#discussion_r313628882
 
 

 ##
 File path: solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java
 ##
 @@ -145,8 +145,14 @@ public void init(NamedList args) {
 
   }
 
+  protected MetricsInfo metricsInfo;
+
+
   @Override
   public void initializeMetrics(SolrMetricManager manager, String 
registryName, String tag, final String scope) {
+metricsInfo = new MetricsInfo(manager, registryName, 
getUniqueMetricTag(tag)) ;
+tag = metricsInfo.getTag();
 
 Review comment:
   Yeah, we should avoid passing this zillion different parameters . But the 
problem is it is too much of a change. However I'll try to do it


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[GitHub] [lucene-solr] noblepaul commented on a change in pull request #825: SOLR-13677 All Metrics Gauges should be unregistered by the objects that registered them

2019-08-13 Thread GitBox
noblepaul commented on a change in pull request #825: SOLR-13677 All Metrics 
Gauges should be unregistered by the objects that registered them
URL: https://github.com/apache/lucene-solr/pull/825#discussion_r313621446
 
 

 ##
 File path: solr/core/src/java/org/apache/solr/metrics/SolrMetricProducer.java
 ##
 @@ -19,17 +19,58 @@
 /**
  * Used by objects that expose metrics through {@link SolrCoreMetricManager}.
  */
-public interface SolrMetricProducer {
+public interface SolrMetricProducer extends AutoCloseable {
+
+  /**
+   * Unique metric name is in the format of A/B/C
+   * A is the parent of B is the parent of C and so on.
+   * If object "B" is unregistered , C also must get unregistered.
+   * If object "A" is unregistered ,  B , C also must get unregistered.
+   *
+   * @param parentName
+   */
+  default String getUniqueMetricTag(String parentName) {
+String name = getClass().getSimpleName() + "@" + 
Integer.toHexString(hashCode());
+return parentName == null ?
+name :
+parentName + "/" + name;
+  }
+
+  default void close() throws Exception{
+
+  }
+
 
   /**
* Initializes metrics specific to this producer
-   * @param manager an instance of {@link SolrMetricManager}
+   *
+   * @param manager  an instance of {@link SolrMetricManager}
* @param registry registry name where metrics are registered
-   * @param tag a symbolic tag that represents this instance of the producer,
-   * or a group of related instances that have the same life-cycle. This tag is
-   * used when managing life-cycle of some metrics and is set when
-   * {@link #initializeMetrics(SolrMetricManager, String, String, String)} is 
called.
-   * @param scope scope of the metrics (eg. handler name) to separate metrics 
of
+   * @param tag  a symbolic tag that represents this instance of the 
producer,
+   * or a group of related instances that have the same 
life-cycle. This tag is
+   * used when managing life-cycle of some metrics and is set 
when
+   * {@link #initializeMetrics(SolrMetricManager, String, 
String, String)} is called.
+   * @param scopescope of the metrics (eg. handler name) to separate 
metrics of
*/
   void initializeMetrics(SolrMetricManager manager, String registry, String 
tag, String scope);
+
+  class MetricsInfo {
 
 Review comment:
   Well, it contains more information than just a tag. That's why I had to 
choose a name other than MetricsTag. The problem we have with Metrics framework 
is that every component that is a MetricsProducer is forced to be aware of all 
internals of MetricsFramework. We should minimize the no:of Objects one need to 
keep track of (ideally a single Object)


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org