klcodanr commented on code in PR #30:
URL: 
https://github.com/apache/sling-org-apache-sling-event/pull/30#discussion_r1244188308


##########
src/main/java/org/apache/sling/event/impl/jobs/stats/GaugeSupport.java:
##########
@@ -150,9 +154,10 @@ private void registerWithSuffix(String suffix, int count, 
Gauge<Long> value) {
             metricRegistry.register(metricName, value);
             gaugeMetricNames.add(metricName);
         } catch (IllegalArgumentException e) {
-            if (queueName != null) {
-                registerWithSuffix(suffix, count + 1, value);
-            }
+            logger.error("Failed to register mbean with suffix " + suffix, e);
+            // if (queueName != null) {
+            //     registerWithSuffix(suffix, count + 1, value);
+            // }

Review Comment:
   Rather than just failing, this should add a recursion limit. I wouldn't 
_think_ it would have to be large say 5(?), but it should retry x times 
incrementing the count and then log the error and quit.



##########
src/main/java/org/apache/sling/event/impl/jobs/stats/GaugeSupport.java:
##########
@@ -48,6 +50,8 @@ class GaugeSupport {
     private final Set<String> gaugeMetricNames = new HashSet<>();
     private final String queueName;
 
+    Logger logger = LoggerFactory.getLogger(this.getClass());

Review Comment:
   This should be private (and probably static final) most Sling projects use 
log or LOG as name the convention.



-- 
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: dev-unsubscr...@sling.apache.org

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

Reply via email to