mlbiscoc commented on code in PR #3430: URL: https://github.com/apache/solr/pull/3430#discussion_r2216397680
########## solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java: ########## @@ -49,6 +50,10 @@ import org.junit.Before; import org.junit.Test; +// NOCOMMIT: Test fails because of the @After assert on index path. Was going to migrate to just +// check the registry for the core is deleted but this test does a rename operation which otel has +// not addressed yet. Need to migrate the rename operation to otel first. +@LuceneTestCase.BadApple(bugUrl = "https://issues.apache.org/jira/browse/SOLR-17458";) public class SolrMetricsIntegrationTest extends SolrTestCaseJ4 { Review Comment: Is Solr 10 not going to support renaming or swapping core names anymore? ########## solr/core/src/java/org/apache/solr/cluster/placement/impl/CollectionMetricsBuilder.java: ########## @@ -29,6 +29,7 @@ import org.slf4j.LoggerFactory; /** Builder class for constructing instances of {@link CollectionMetrics}. */ +// NOCOMMIT: Need to migrate this to an OTEL collection Metrics builder public class CollectionMetricsBuilder { Review Comment: This seems tied to some cluster plugins. Not sure how but would need to look on how something like this would be supported from otel and its created `MetricsMap` ########## solr/core/src/java/org/apache/solr/metrics/reporters/SolrJmxReporter.java: ########## @@ -37,6 +37,8 @@ * <p>NOTE: {@link com.codahale.metrics.jmx.JmxReporter} that this class uses exports only newly * added metrics (it doesn't process already existing metrics in a registry) */ +// NOCOMMIT: This JMX reporter looks to be wrapped over a Dropwizard registry. We need to migrate +// this to OTEL. Maybe we can look at available otel shims for JMX? public class SolrJmxReporter extends FilteringSolrMetricReporter { Review Comment: Dropwizard was wrapped into this JMX reporter. OTEL doesn't have a way to just create a JXM reporter but what if we bridge [OTEL -> Micrometer shim](https://opentelemetry.io/docs/languages/java/instrumentation/#shims) and use [micrometers JMX reporter](https://docs.micrometer.io/micrometer/reference/implementations/jmx.html) for the metrics to still support JMX? ########## solr/core/src/java/org/apache/solr/metrics/reporters/SolrSlf4jReporter.java: ########## @@ -49,6 +49,8 @@ * <code>solr.jvm</code>, <code>solr.core</code>, etc * </ul> */ +// NOCOMMIT: Will we still be supporting this? Need to understand how Slf4j works and if it is even +// possible to wrap OTEL into this like dropwizard. public class SolrSlf4jReporter extends FilteringSolrMetricReporter { Review Comment: Not familiar with this reporter but somehow dropwizard is wrapped into this. -- 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: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
