You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "mlbiscoc (via GitHub)" <gi...@apache.org> on 2025/11/04 20:13:20 UTC
[PR] SOLR-17458: OTEL configurable disable metrics with NOOP [solr]
mlbiscoc opened a new pull request, #3835:
URL: https://github.com/apache/solr/pull/3835
https://issues.apache.org/jira/browse/SOLR-17458
Bringing back the solr metrics config that I removed to use OpenTelemetry NOOP implementations. Added tests back that were originally deleted as well as updating documentation for disabling and re-enabling metrics
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17458: OTEL configurable disable metrics with NOOP [solr]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3835:
URL: https://github.com/apache/solr/pull/3835#issuecomment-4435980997
This PR has had no activity for 60 days and is now labeled as stale. Any new activity will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the dev@solr.apache.org mailing list. To exempt this PR from being marked as stale, make it a draft PR or add the label "exempt-stale". If left unattended, this PR will be closed after another 60 days of inactivity. Thank you for your contribution!
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17458: OTEL configurable disable metrics with NOOP [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3835:
URL: https://github.com/apache/solr/pull/3835#discussion_r2512337560
##########
solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java:
##########
@@ -122,6 +122,11 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
+ format);
}
+ if (!isEnabled()) {
+ rsp.add("metrics", MetricSnapshots.builder().build());
Review Comment:
No, I mean write to the client for a user to see. The output would still be valid exposition, I assume
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17458: OTEL configurable disable metrics with NOOP [solr]
Posted by "mlbiscoc (via GitHub)" <gi...@apache.org>.
mlbiscoc commented on code in PR #3835:
URL: https://github.com/apache/solr/pull/3835#discussion_r2511038545
##########
solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java:
##########
@@ -122,6 +122,11 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
+ format);
}
+ if (!isEnabled()) {
+ rsp.add("metrics", MetricSnapshots.builder().build());
Review Comment:
It shouldn't because above this the format looks for Prometheus/OpenMetrics and appends `wt=prometheus` if neither is passed in to default to PrometheusResponseWriter. It will also throw an exception of a Bad Request if any other format of wt is passed in. Also when you say `#` comment, you want a log line like `### Metrics disabled` or something?
##########
solr/core/src/java/org/apache/solr/core/MetricsConfig.java:
##########
@@ -16,133 +16,24 @@
*/
package org.apache.solr.core;
-import java.util.Collections;
-
-/** */
+/**
+ * Configuration for metrics collection in Solr. Currently only supports both enabled and disabled
+ * states. TODO: Extend to support more configuration options in the future.
+ */
public class MetricsConfig {
- private final PluginInfo[] metricReporters;
- private final PluginInfo counterSupplier;
- private final PluginInfo meterSupplier;
- private final PluginInfo timerSupplier;
- private final PluginInfo histogramSupplier;
- private final Object nullNumber;
- private final Object notANumber;
- private final Object nullString;
- private final Object nullObject;
private final boolean enabled;
- private final CacheConfig cacheConfig;
- private MetricsConfig(
- boolean enabled,
- PluginInfo[] metricReporters,
- PluginInfo counterSupplier,
- PluginInfo meterSupplier,
- PluginInfo timerSupplier,
- PluginInfo histogramSupplier,
- Object nullNumber,
- Object notANumber,
- Object nullString,
- Object nullObject,
- CacheConfig cacheConfig) {
+ private MetricsConfig(boolean enabled) {
this.enabled = enabled;
Review Comment:
I moved to check the `PluginInfo.isEnabled` in the latest commit but are you saying remove enabled from the MetricsConfig class here? The enabled flag here is used as a check around Solr unless you want to remove this and on `SolrXmlConfig` just return null?
##########
solr/solr-ref-guide/modules/deployment-guide/pages/metrics-reporting.adoc:
##########
@@ -130,6 +130,23 @@ Metrics collection for index merges can be configured in the `<metrics>` section
</config>
----
+== Metrics Configuration
+
+The metrics available in your system can be customized by modifying the `<metrics>` element in `solr.xml`.
+
+TIP: See also the section xref:configuration-guide:configuring-solr-xml.adoc[] for more information about the `solr.xml` file, where to find it, and how to edit it.
+
+=== Disabling the Metrics Collection
+The `<metrics>` element in `solr.xml` supports one attribute `metricsEnabled`, which takes a boolean value,
+for example `<metricsEnabled="true">`.
Review Comment:
Fixed with plugin info so now it is just `enable`
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17458: OTEL configurable disable metrics with NOOP [solr]
Posted by "mlbiscoc (via GitHub)" <gi...@apache.org>.
mlbiscoc commented on code in PR #3835:
URL: https://github.com/apache/solr/pull/3835#discussion_r2491928518
##########
solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java:
##########
@@ -122,6 +122,11 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
+ format);
}
+ if (!isEnabled()) {
+ rsp.add("metrics", MetricSnapshots.builder().build());
Review Comment:
I am just returning an empty response instead of throwing an error on metrics being disabled like before. My change here is debatable so any thoughts welcome.
##########
solr/core/src/java/org/apache/solr/core/MetricsConfig.java:
##########
@@ -16,133 +16,24 @@
*/
package org.apache.solr.core;
-import java.util.Collections;
-
-/** */
+/**
+ * Configuration for metrics collection in Solr. Currently only supports both enabled and disabled
+ * states. TODO: Extend to support more configuration options in the future.
+ */
public class MetricsConfig {
Review Comment:
This doesn't do much right now except enable/disable metrics but I would expect this to grow in the future.
##########
solr/core/src/test/org/apache/solr/cloud/CloudExitableDirectoryReaderTest.java:
##########
@@ -73,6 +73,8 @@ public class CloudExitableDirectoryReaderTest extends SolrCloudTestCase {
@BeforeClass
public static void setupCluster() throws Exception {
+ System.setProperty("metricsEnabled", "true");
Review Comment:
Metrics are disabled by default for tests using `MiniSolrCloudCluster` using this [config here](https://github.com/apache/solr/blob/19ec326349d1f31d6735f3b8c11b793503c77f43/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java#L147). Do we want to also disable by default in `SolrTestCaseJ4`?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17458: OTEL configurable disable metrics with NOOP [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #3835:
URL: https://github.com/apache/solr/pull/3835#issuecomment-3499162976
BTW about disabling metrics... there are some basic/essential metrics that SolrCloud core placement uses (your friend AttributeFetcher etc.) at a node level, like core count, disk space. That needn't be addressed in this PR if you don't want, but _ultimately_ we'd like to disable/configure metrics categorically by registry IMO. Like just need the "node" metrics; not JVM or cores.
I wish both metrics and tracing could be configured like java loggers can -- via a named hierarchy and at a "level". It'd be a hack but I could imagine abusing log4j2 config for metrics & tracing enable/disable.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17458: OTEL configurable disable metrics with NOOP [solr]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3835:
URL: https://github.com/apache/solr/pull/3835#issuecomment-3733732094
This PR has had no activity for 60 days and is now labeled as stale. Any new activity will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the dev@solr.apache.org mailing list. To exempt this PR from being marked as stale, make it a draft PR or add the label "exempt-stale". If left unattended, this PR will be closed after another 60 days of inactivity. Thank you for your contribution!
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17458: OTEL configurable disable metrics with NOOP [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #3835:
URL: https://github.com/apache/solr/pull/3835#issuecomment-3501098712
Yes, should be possible to easily filter metrics we don’t use or don’t have a dashboard for, to conserve ingestion cost and do size in APM systems. A file to control this would mean we could ship Solr with examples such as metrics-all.yaml, metrics-dashboard.yaml, metrics-essential.yaml etc. as starting points.
While such filtering is possible in OTEL collector, it would still make sense with filtering at the source.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17458: OTEL configurable disable metrics with NOOP [solr]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3835:
URL: https://github.com/apache/solr/pull/3835#issuecomment-4953409127
This PR is now closed due to 60 days of inactivity after being marked as stale. Re-opening this PR is still possible, in which case it will be marked as active again.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17458: OTEL configurable disable metrics with NOOP [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3835:
URL: https://github.com/apache/solr/pull/3835#discussion_r2492101814
##########
solr/core/src/java/org/apache/solr/core/MetricsConfig.java:
##########
@@ -16,133 +16,24 @@
*/
package org.apache.solr.core;
-import java.util.Collections;
-
-/** */
+/**
+ * Configuration for metrics collection in Solr. Currently only supports both enabled and disabled
+ * states. TODO: Extend to support more configuration options in the future.
+ */
public class MetricsConfig {
- private final PluginInfo[] metricReporters;
- private final PluginInfo counterSupplier;
- private final PluginInfo meterSupplier;
- private final PluginInfo timerSupplier;
- private final PluginInfo histogramSupplier;
- private final Object nullNumber;
- private final Object notANumber;
- private final Object nullString;
- private final Object nullObject;
private final boolean enabled;
- private final CacheConfig cacheConfig;
- private MetricsConfig(
- boolean enabled,
- PluginInfo[] metricReporters,
- PluginInfo counterSupplier,
- PluginInfo meterSupplier,
- PluginInfo timerSupplier,
- PluginInfo histogramSupplier,
- Object nullNumber,
- Object notANumber,
- Object nullString,
- Object nullObject,
- CacheConfig cacheConfig) {
+ private MetricsConfig(boolean enabled) {
this.enabled = enabled;
Review Comment:
There's no need for this; *all* PluginInfo innately support an `enable` attribute. See `PluginInfo.isEnabled`
BTW I wish it was changed from "enable" to "enabled".... I sometimes get that mixed up.
##########
solr/solr-ref-guide/modules/deployment-guide/pages/metrics-reporting.adoc:
##########
@@ -130,6 +130,23 @@ Metrics collection for index merges can be configured in the `<metrics>` section
</config>
----
+== Metrics Configuration
+
+The metrics available in your system can be customized by modifying the `<metrics>` element in `solr.xml`.
+
+TIP: See also the section xref:configuration-guide:configuring-solr-xml.adoc[] for more information about the `solr.xml` file, where to find it, and how to edit it.
+
+=== Disabling the Metrics Collection
+The `<metrics>` element in `solr.xml` supports one attribute `metricsEnabled`, which takes a boolean value,
+for example `<metricsEnabled="true">`.
Review Comment:
typo/wrong
##########
solr/core/src/test/org/apache/solr/cloud/CloudExitableDirectoryReaderTest.java:
##########
@@ -73,6 +73,8 @@ public class CloudExitableDirectoryReaderTest extends SolrCloudTestCase {
@BeforeClass
public static void setupCluster() throws Exception {
+ System.setProperty("metricsEnabled", "true");
Review Comment:
I think it should be disabled by default in all tests IMO. A test can opt-in easily enough. If we were to debate and agree that some tests categorically should be enable-by-default, the SolrCloud ones would be ones to enable as they are kind of integration in nature. Ironically, that's the inverse of how things are now :-). Okay to defer this. We use `jetty.testMode` (poor name) to universally mean that we're running in test mode (not necessarily with Jetty).
##########
solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java:
##########
@@ -440,12 +444,17 @@ public boolean hasRegistry(String name) {
}
/**
- * Get (or create if not present) a named {@link SdkMeterProvider}.
+ * Get (or create if not present) a named {@link SdkMeterProvider}. If metrics are disabled,
+ * returns a NOOP implementation from OpenTelemetry.
*
* @param providerName name of the meter provider and prometheus metric reader
- * @return existing or newly created meter provider
+ * @return existing or newly created meter provider, or NOOP if metrics disabled
*/
- public SdkMeterProvider meterProvider(String providerName) {
+ public MeterProvider meterProvider(String providerName) {
+ if (!metricsEnabled) {
+ return OpenTelemetry.noop().getMeterProvider();
Review Comment:
nice
##########
solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java:
##########
@@ -122,6 +122,11 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
+ format);
}
+ if (!isEnabled()) {
+ rsp.add("metrics", MetricSnapshots.builder().build());
Review Comment:
Hard to tell from the PR view but is this causing the handler to return Solr-XML/JSON/JavaBin? We don't want that. I think a line with a "#" comment saying it's disabled would be useful
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17458: OTEL configurable disable metrics with NOOP [solr]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3835:
URL: https://github.com/apache/solr/pull/3835#issuecomment-4043036112
This PR is now closed due to 60 days of inactivity after being marked as stale. Re-opening this PR is still possible, in which case it will be marked as active again.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17458: OTEL configurable disable metrics with NOOP [solr]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #3835: SOLR-17458: OTEL configurable disable metrics with NOOP
URL: https://github.com/apache/solr/pull/3835
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17458: OTEL configurable disable metrics with NOOP [solr]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #3835: SOLR-17458: OTEL configurable disable metrics with NOOP
URL: https://github.com/apache/solr/pull/3835
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org