SOLR-17855: Remove Dropwizard dependency from core#4404
SOLR-17855: Remove Dropwizard dependency from core#4404mlbiscoc wants to merge 8 commits intoapache:mainfrom
Conversation
github-actions
Bot
added
documentation
mlbiscoc
left a comment
mlbiscoc
left a comment
There was a problem hiding this comment.
There are a few things to note:
- The
OVERSEERSTATUSapi no longer including rates, percentiles and timing information. Only requests and error info. If we still care about this overseer info, I can properly collect to OTEL and have it exposed via our metrics api and OTLP instead. - Dropwizard
metrics-core:3.2.5is still being pulled in places where we embed ZooKeeper which is Solrs testing framework.
dsmiley
left a comment
dsmiley
left a comment
There was a problem hiding this comment.
Note that Solr has RTimer, which maybe might be an adequate substitute in some places where you removed stuff completely?
I'm okay with losing some metrics relating to the Overseer. Someone can invest in bringing it back if they so please. I'm more okay with this, I admit, because I want folks to disable the Overseer instead ;-)
It'll be very necessary to add to major-changes-in-solr-10.adoc here on metrics that have disappeared.
| scheduledExecutor.shutdown(); | ||
| } else { | ||
| log(meter.getCount() + " docs at " + meter.getMeanRate() + " doc/s"); | ||
| double rate = count / ((System.nanoTime() - meterStartNanos) / 1e9); |
There was a problem hiding this comment.
I'd prefer to see something that requires less trust in the time math as a code reviewer... like some JDK API calls to produce a readable equivalent.
There was a problem hiding this comment.
if we wanted to bring back such timings, I'd rather it be done with annotations similar to tracing @WithSpan. Alas, OTEL has no metric equivalent, which would be for something more light-weight that isn't span-worthy. I think talking to ZK is span-worthy as it's another process/system, albeit it's so fast that someone would typically prefer to disable such. Ideally Spans would have levels like logging but they don't; people have proposed it. I did see this interesting development, which is somewhat related as it allows disabling spans for an entire "scope".
|
@mlbiscoc I think we may be able to elminate JettySolrRunnerWithMetrics as well... I poked around and it appears that it is no longer used in testing, it exists for two classes, but isn't actively used in the asserts, it just sits there... I played around a bit and got this patch file: |
3 participants
https://issues.apache.org/jira/browse/SOLR-17855
After migration to OTEL, dropwizard metrics are not used anymore for metrics collection but there were still a few references to it in solr-core. This removes those dropwizard instruments so we can get rid of the dependency from core but it is still needed in test-framework where we embed zookeeper.