stefan-egli commented on code in PR #30:
URL:
https://github.com/apache/sling-org-apache-sling-event/pull/30#discussion_r1245096454
##########
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:
+1 to what @klcodanr recommends.
This should do a log.debug for the first `n` times, then when it hits the
limit `n` only do a log.error. The way it is done now is removing support for
metric name collisions due to name sanitization (which is one legitimate reason
for this retry)
> I wouldn't think it would have to be large say 5(?)
agree, I thought about `10` initially, but could be somewhere in that range,
yes.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
