[GitHub] [sling-org-apache-sling-event] klcodanr commented on a diff in pull request #30: Removing infinite loop

["klcodanr (via GitHub)" <gi...@apache.org>]

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-unsubscribe@sling.apache.org

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