madrob commented on code in PR #972:
URL: https://github.com/apache/solr/pull/972#discussion_r945916672


##########
solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java:
##########
@@ -216,4 +223,65 @@ public void testCoreContainerMetrics() {
     Gauge<?> g = (Gauge<?>) metrics.get("CONTAINER.fs.path");
     assertEquals(g.getValue(), cc.getSolrHome());
   }
+
+  @Test
+  public void testZkMetrics() throws Exception {
+    System.setProperty("metricsEnabled", "true");
+    MiniSolrCloudCluster cluster =
+        new MiniSolrCloudCluster.Builder(3, createTempDir())
+            .withJettyConfig(jetty -> jetty.enableV2(true))

Review Comment:
   isn't this enabled by default?



##########
solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java:
##########
@@ -632,11 +685,17 @@ public Stat setData(String path, Path data, boolean 
retryOnConnLoss)
 
   public List<OpResult> multi(final Iterable<Op> ops, boolean retryOnConnLoss)
       throws InterruptedException, KeeperException {
+    List<OpResult> result = null;
     if (retryOnConnLoss) {
-      return zkCmdExecutor.retryOperation(() -> keeper.multi(ops));
+      result = zkCmdExecutor.retryOperation(() -> keeper.multi(ops));
     } else {
-      return keeper.multi(ops);
+      result = keeper.multi(ops);
+    }
+    metrics.multiOps.increment();

Review Comment:
   will this also record the individual operations as reads/writes? or do we 
only get a blanket multi-op counter, and no insight into what they were doing.



##########
solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java:
##########
@@ -216,4 +223,65 @@ public void testCoreContainerMetrics() {
     Gauge<?> g = (Gauge<?>) metrics.get("CONTAINER.fs.path");
     assertEquals(g.getValue(), cc.getSolrHome());
   }
+
+  @Test
+  public void testZkMetrics() throws Exception {
+    System.setProperty("metricsEnabled", "true");
+    MiniSolrCloudCluster cluster =
+        new MiniSolrCloudCluster.Builder(3, createTempDir())
+            .withJettyConfig(jetty -> jetty.enableV2(true))
+            .addConfig("conf", configset("conf2"))
+            .configure();
+    System.clearProperty("metricsEnabled");
+    try {
+      JettySolrRunner j = cluster.getRandomJetty(random());
+      String url = j.getBaseUrl() + 
"/admin/metrics?key=solr.node:CONTAINER.zkClient&wt=json";
+      HttpClient httpClient = ((HttpSolrClient) j.newClient()).getHttpClient();
+      @SuppressWarnings("unchecked")
+      Map<String, Object> zkMmetrics =
+          (Map<String, Object>)
+              Utils.getObjectByPath(
+                  Utils.executeGET(httpClient, url, Utils.JSONCONSUMER),
+                  false,
+                  List.of("metrics", "solr.node:CONTAINER.zkClient"));
+      System.out.println(Utils.toJSONString(zkMmetrics));

Review Comment:
   should use logger if needed



-- 
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: issues-unsubscr...@solr.apache.org

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


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

Reply via email to