Skip to content

SOLR-18400: Plugins screen 500s when metrics are disabled - #4803

Open
janhoy wants to merge 4 commits into
apache:mainfrom
janhoy:SOLR-18400-plugins-metrics-disabled
Open

SOLR-18400: Plugins screen 500s when metrics are disabled#4803
janhoy wants to merge 4 commits into
apache:mainfrom
janhoy:SOLR-18400-plugins-metrics-disabled

Conversation

@janhoy

@janhoy janhoy commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

https://issues.apache.org/jira/browse/SOLR-18400

When metrics collection is disabled, MetricsHandler adds an error entry instead of metrics, and PrometheusResponseWriter threw IOException("No metrics found in response") → HTTP 500 and a blank Plugins screen.

  • PrometheusResponseWriter now writes the error as a Prometheus comment (# metrics collection is disabled) with HTTP 200 — matching the old JSON-era behavior and keeping the response a valid exposition
  • Plugins screen shows an explanatory message instead of a blank page
  • Regression test added to TestPrometheusResponseWriter (pure unit test, no cluster needed)

Verified with the Selenium Admin UI tests (SmokeTest + CollectionScreensTest, 11 tests) from the admin-ui-selenium-tests branch merged locally, and gradlew precommit.

/admin/metrics now returns a valid Prometheus response with an explanatory
comment instead of a 500 when metrics collection is disabled, and the
Plugins screen shows a message instead of a blank page.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes the Plugins screen failure when Solr metrics collection is disabled by returning an explanatory response and UI message.

Changes:

  • Handles disabled metrics in Prometheus responses.
  • Displays an explanatory Plugins screen message.
  • Adds regression coverage and a changelog entry.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Summary
solr/webapp/web/partials/plugins.html Displays the disabled-metrics message; configuration guidance needs correction.
solr/webapp/web/js/angular/controllers/plugins.js Detects the disabled-metrics response.
solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriter.java Adds regression coverage for the explanatory response.
solr/core/src/java/org/apache/solr/response/PrometheusResponseWriter.java Serializes disabled metrics as comments; OpenMetrics responses also need # EOF.
changelog/unreleased/SOLR-18400-plugins-metrics-disabled.yml Documents the fix.
Suppressed comments (1)

solr/webapp/web/js/angular/controllers/plugins.js:38

  • This searches the entire exposition for the sentinel, so a valid metric HELP/comment/label containing this phrase will make the Plugins screen incorrectly report that metrics are disabled. Compare the complete error comment instead of using a substring match.
                $scope.metricsDisabled = (response.data || '').indexOf('metrics collection is disabled') !== -1;

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread solr/webapp/web/partials/plugins.html Outdated
@janhoy

janhoy commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

Note: the v2 metrics API (GetMetrics) still throws a 510 INVALID_STATE error when metrics collection is disabled, so v1 and v2 now behave differently (v1 returns 200 with a # metrics collection is disabled comment). I left v2 as-is since this PR targets the v1 endpoint the Admin UI uses, but we may want to align v2 with the same graceful behavior in a follow-up.

- Always append the OpenMetrics EOF marker to the disabled-metrics
  comment response (harmless in Prometheus format)
- Correct the Plugins screen message to reference the <metrics enabled>
  setting in solr.xml; metricsEnabled is not a real production property
@janhoy
janhoy marked this pull request as ready for review August 23, 2026 19:32
@epugh

epugh commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Note: the v2 metrics API (GetMetrics) still throws a 510 INVALID_STATE error when metrics collection is disabled, so v1 and v2 now behave differently (v1 returns 200 with a # metrics collection is disabled comment). I left v2 as-is since this PR targets the v1 endpoint the Admin UI uses, but we may want to align v2 with the same graceful behavior in a follow-up.

It would make life easier when we move if V2 did the same as V1... One reason I'm axinous to get us to V2 everywhere we can is that we've seen this pattern of fixes making it to V1 when we also have V2, and then it falls behind...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants