Alfredops 715 fix ass 2.0.0 #74
Conversation
…more specifically the FTS counts. Use dynamic invocation to call the appopriate method.
…ometer-related functionality. Note: I tried to use config overlays (REST calls via the micrometer init script), but it breaks the alfresco-specific solr (although resources are correctly added). Alfresco-solr explicitelly dissalows this by setting -Ddisable.configEdit=true.
|
|
||
| public class SolrMetrics implements MeterBinder { | ||
|
|
||
| AlfrescoCoreAdminHandler coreAdminHandler; |
There was a problem hiding this comment.
Why? It is used outside package.
There was a problem hiding this comment.
I don't see any package-usages ? 🤷
There was a problem hiding this comment.
It's used by eu.xenit.alfred.telemetry.solr.monitoring.handler.MicrometerHandler, outside the package.
There was a problem hiding this comment.
Will merge now and come back to this later if needed
There was a problem hiding this comment.
If used elsewhere, please create and use an accessor.
No comment about high-jacking solr's insight.xml file? |
| @@ -18,7 +18,7 @@ | |||
| AlfrescoCoreAdminHandler coreAdminHandler; | |||
| MeterRegistry registry; | |||
|
|
|||
| Logger logger = LoggerFactory.getLogger(SolrMetrics.class); | |||
| Logger logger = LoggerFactory.getLogger(SolrTrackerMetrics.class); | |||
There was a problem hiding this comment.
Logger should be at least private, preferably also static final
private- so that no other class can hijack your loggerstatic- so there is only one logger instance per class, also avoiding attempts to serialize loggersfinal- no need to change the logger over the lifetime of the class
|
|
||
| public class SolrMetrics implements MeterBinder { | ||
|
|
||
| AlfrescoCoreAdminHandler coreAdminHandler; |
There was a problem hiding this comment.
If used elsewhere, please create and use an accessor.
|
|
||
| @Override | ||
| public void bindTo(MeterRegistry registry) { | ||
| if(Util.isEnabled("METRICS_SOLR_CORESTATS_ENABLED")) |
There was a problem hiding this comment.
Use braces in if statements
https://google.github.io/styleguide/javaguide.html#s4.1.1-braces-always-used
| Thread.currentThread().sleep(10_000); | ||
| trackerRegistry = coreAdminHandler.getTrackerRegistry(); | ||
| } catch (InterruptedException e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
Use a logger to log catched exceptions.
|
Oh, seems I was to late here 🙃 . Can we please cleanup this code anyway? According to Xenit code style conventions and best practices... |
Starting with ASS 2.0 some methods changed in SolrInformationServer, more specifically the FTS counts.
Use dynamic invocation to call the appopriate method, so that alfred-telemetry-solr6 can be packaged as a single jar.
Use the already imported file solrconfig_insights.xml to add the micrometer-related functionality.
Note: I tried to use config overlays (REST calls via the micrometer init script), but it breaks the alfresco-specific solr, although resources are correctly added.