Re: [PR] SOLR-17853: Migrate Plugins / Stats UI tab with Prometheus metrics [solr]

2025-10-18 Thread via GitHub 


mlbiscoc commented on PR #3623:
URL: https://github.com/apache/solr/pull/3623#issuecomment-3340070592

   Ah thanks for digging into that! Yes I forgot about that. OTEL JVM metrics 
come from an OTEL instrumentation library that uses JFR streaming. I added it 
into security.policy in 
[ce6d0c1](https://github.com/apache/solr/pull/3623/commits/ce6d0c17da4c7c997bf37ffd366569ec6a9e6bac).


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] SOLR-17853: Migrate Plugins / Stats UI tab with Prometheus metrics [solr]

2025-10-18 Thread via GitHub 


malliaridis commented on PR #3623:
URL: https://github.com/apache/solr/pull/3623#issuecomment-3340015829

   So after investigating, it was actually the a SecurityManager issue (missing 
permission). This lead to the follow-up Nullpointer exception. Specifically the 
root cause is:
   ```
at 
java.base/java.security.AccessControlContext.checkPermission(AccessControlContext.java:488)
   java.security.AccessControlException: access denied 
("jdk.jfr.FlightRecorderPermission" "accessFlightRecorder")
at 
java.base/java.security.AccessControlContext.checkPermission(AccessControlContext.java:488)
 ~[?:?]
at 
java.base/java.security.AccessController.checkPermission(AccessController.java:1071)
 ~[?:?]
at 
java.base/java.lang.SecurityManager.checkPermission(SecurityManager.java:411) 
~[?:?]
at 
[email protected]/jdk.jfr.internal.Utils.checkAccessFlightRecorder(Utils.java:100) 
~[?:?]
at 
[email protected]/jdk.jfr.consumer.RecordingStream.(RecordingStream.java:105)
 ~[?:?]
at 
[email protected]/jdk.jfr.consumer.RecordingStream.(RecordingStream.java:101)
 ~[?:?]
at 
io.opentelemetry.instrumentation.runtimemetrics.java17.RuntimeMetrics$JfrRuntimeMetrics.(RuntimeMetrics.java:95)
 ~[?:?]
at 
io.opentelemetry.instrumentation.runtimemetrics.java17.RuntimeMetrics$JfrRuntimeMetrics.build(RuntimeMetrics.java:114)
 ~[?:?]
at 
io.opentelemetry.instrumentation.runtimemetrics.java17.RuntimeMetricsBuilder.buildJfrMetrics(RuntimeMetricsBuilder.java:129)
 ~[?:?]
at 
io.opentelemetry.instrumentation.runtimemetrics.java17.RuntimeMetricsBuilder.build(RuntimeMetricsBuilder.java:96)
 ~[?:?]
at 
org.apache.solr.metrics.OtelRuntimeJvmMetrics.initialize(OtelRuntimeJvmMetrics.java:49)
 ~[?:?]
at 
org.apache.solr.servlet.CoreContainerProvider.setupJvmMetrics(CoreContainerProvider.java:380)
 ~[?:?]
at 
org.apache.solr.servlet.CoreContainerProvider.init(CoreContainerProvider.java:197)
 ~[?:?]
at 
org.apache.solr.servlet.CoreContainerProvider.contextInitialized(CoreContainerProvider.java:95)
 ~[?:?]
at 
org.eclipse.jetty.ee10.servlet.ServletContextHandler.callContextInitialized(ServletContextHandler.java:1591)
 ~[jetty-ee10-servlet-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.ee10.servlet.ServletContextHandler.contextInitialized(ServletContextHandler.java:500)
 ~[jetty-ee10-servlet-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.ee10.servlet.ServletHandler.initialize(ServletHandler.java:676)
 ~[jetty-ee10-servlet-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.ee10.servlet.ServletContextHandler.startContext(ServletContextHandler.java:1325)
 ~[jetty-ee10-servlet-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.ee10.webapp.WebAppContext.startWebapp(WebAppContext.java:1350)
 ~[jetty-ee10-webapp-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.ee10.webapp.WebAppContext.startContext(WebAppContext.java:1308)
 ~[jetty-ee10-webapp-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.ee10.servlet.ServletContextHandler.lambda$doStart$0(ServletContextHandler.java:1051)
 ~[jetty-ee10-servlet-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.server.handler.ContextHandler$ScopedContext.call(ContextHandler.java:1456)
 ~[jetty-server-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.ee10.servlet.ServletContextHandler.doStart(ServletContextHandler.java:1048)
 ~[jetty-ee10-servlet-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.ee10.webapp.WebAppContext.doStart(WebAppContext.java:504) 
~[jetty-ee10-webapp-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:93)
 ~[jetty-util-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.util.component.ContainerLifeCycle.start(ContainerLifeCycle.java:169)
 ~[jetty-util-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.util.component.ContainerLifeCycle.doStart(ContainerLifeCycle.java:120)
 ~[jetty-util-12.0.19.jar:12.0.19]
at org.eclipse.jetty.server.Handler$Abstract.doStart(Handler.java:491) 
~[jetty-server-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:93)
 ~[jetty-util-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.util.component.ContainerLifeCycle.start(ContainerLifeCycle.java:169)
 ~[jetty-util-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.util.component.ContainerLifeCycle.doStart(ContainerLifeCycle.java:120)
 ~[jetty-util-12.0.19.jar:12.0.19]
at org.eclipse.jetty.server.Handler$Abstract.doStart(Handler.java:491) 
~[jetty-server-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.server.handler.ConditionalHandler.doStart(ConditionalHandler.java:372)
 ~[jetty-server-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:93)
 ~[jetty-util-12.0.19.jar:12.0.19]
at 
org.eclipse.jetty.util.component.ContainerLifeCycle.start(ContainerLifeCycle.java:169)
 ~[jetty-util-12.0.19.ja

Re: [PR] SOLR-17853: Migrate Plugins / Stats UI tab with Prometheus metrics [solr]

2025-10-18 Thread via GitHub 


mlbiscoc commented on PR #3623:
URL: https://github.com/apache/solr/pull/3623#issuecomment-3339864425

   Anything else here? I know there was a build issue on your end Christos but 
I am unable to replicate it on my end. I haven't got to a windows machine but 
since this is a feature branch, I am think will do those last Windows test and 
checks later. I am starting to do the finishing touches on OTEL to completion 
and this is a blocker.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] SOLR-17853: Migrate Plugins / Stats UI tab with Prometheus metrics [solr]

2025-10-17 Thread via GitHub 


malliaridis commented on PR #3623:
URL: https://github.com/apache/solr/pull/3623#issuecomment-3339925161

   Sorry for the late reply, I didn't have the time to troubleshoot. I am 
working on it to provide more information. I am on Mac, to answer your 
question, so it is not a Windows-specific issue.
   
   It may be related to the feature branch in general, not only the changes 
from this PR, but I will try to provide more information soon. :)


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] SOLR-17853: Migrate Plugins / Stats UI tab with Prometheus metrics [solr]

2025-10-17 Thread via GitHub 


mlbiscoc merged PR #3623:
URL: https://github.com/apache/solr/pull/3623


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] SOLR-17853: Migrate Plugins / Stats UI tab with Prometheus metrics [solr]

2025-10-05 Thread via GitHub 


mlbiscoc commented on PR #3623:
URL: https://github.com/apache/solr/pull/3623#issuecomment-3369215341

   Will merge this tomorrow if nothing else


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] SOLR-17853: Migrate Plugins / Stats UI tab with Prometheus metrics [solr]

2025-09-22 Thread via GitHub 


mlbiscoc commented on PR #3623:
URL: https://github.com/apache/solr/pull/3623#issuecomment-3320374897

   > Not sure if or how this is related. Perhaps something from the base branch 
[apache:feature/SOLR-17458](https://github.com/apache/solr/tree/feature/SOLR-17458)?
   
   I am not running into this issue and can't reproduce it on other peoples 
machines. Are you on Windows? If so, I might need to get a windows machine to 
see what the issue with this branch is.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] SOLR-17853: Migrate Plugins / Stats UI tab with Prometheus metrics [solr]

2025-09-09 Thread via GitHub 


malliaridis commented on PR #3623:
URL: https://github.com/apache/solr/pull/3623#issuecomment-3272180949

   > What null pointer exception are you seeing? I haven't run into something 
like that. Could you tell me the steps to reproduce?
   
   The only thing I do is run `./gradlew clean dev` and then start solr with 
the techpoducts example via `bin/solr start -e techproducts`. Once I try to 
access `http://localhost:8983` the UI is not loaded anymore because of the 500 
error.
   
   Not sure if or how this is related. Perhaps something from the base branch 
`[apache:feature/SOLR-17458](https://github.com/apache/solr/tree/feature/SOLR-17458)`?
 This is the exception:
   
   ```console
   java.lang.NullPointerException: Cannot invoke 
"org.apache.solr.servlet.RateLimitManager.handleRequest(jakarta.servlet.http.HttpServletRequest)"
 because "rateLimitManager" is null
at 
org.apache.solr.servlet.ServletUtils.rateLimitRequest(ServletUtils.java:187)
at 
org.apache.solr.servlet.SolrDispatchFilter.doFilterRetry(SolrDispatchFilter.java:180)
at 
org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:170)
at jakarta.servlet.http.HttpFilter.doFilter(HttpFilter.java:97)
at 
org.eclipse.jetty.ee10.servlet.FilterHolder.doFilter(FilterHolder.java:208)
at 
org.eclipse.jetty.ee10.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1594)
at 
org.eclipse.jetty.ee10.servlet.ServletHandler$MappedServlet.handle(ServletHandler.java:1555)
at 
org.eclipse.jetty.ee10.servlet.ServletChannel.dispatch(ServletChannel.java:819)
at 
org.eclipse.jetty.ee10.servlet.ServletChannel.handle(ServletChannel.java:436)
at 
org.eclipse.jetty.ee10.servlet.ServletHandler.handle(ServletHandler.java:470)
at 
org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:575)
at 
org.eclipse.jetty.ee10.servlet.SessionHandler.handle(SessionHandler.java:717)
at 
org.eclipse.jetty.server.handler.ContextHandler.handle(ContextHandler.java:1064)
at 
org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:151)
at org.eclipse.jetty.server.Handler$Wrapper.handle(Handler.java:740)
at 
org.eclipse.jetty.server.handler.ConditionalHandler.nextHandler(ConditionalHandler.java:421)
at 
org.eclipse.jetty.server.handler.InetAccessHandler.onConditionsMet(InetAccessHandler.java:50)
at 
org.eclipse.jetty.server.handler.ConditionalHandler.handle(ConditionalHandler.java:378)
at org.eclipse.jetty.server.Handler$Sequence.handle(Handler.java:805)
at 
org.eclipse.jetty.rewrite.handler.RewriteHandler$LastRuleHandler.handle(RewriteHandler.java:159)
at org.eclipse.jetty.rewrite.handler.Rule$Handler.handle(Rule.java:108)
at 
org.eclipse.jetty.rewrite.handler.HeaderPatternRule$1.handle(HeaderPatternRule.java:89)
at org.eclipse.jetty.rewrite.handler.Rule$Handler.handle(Rule.java:108)
at 
org.eclipse.jetty.rewrite.handler.HeaderPatternRule$1.handle(HeaderPatternRule.java:89)
at org.eclipse.jetty.rewrite.handler.Rule$Handler.handle(Rule.java:108)
at 
org.eclipse.jetty.rewrite.handler.HeaderPatternRule$1.handle(HeaderPatternRule.java:89)
at org.eclipse.jetty.rewrite.handler.Rule$Handler.handle(Rule.java:108)
at 
org.eclipse.jetty.rewrite.handler.HeaderPatternRule$1.handle(HeaderPatternRule.java:89)
at org.eclipse.jetty.rewrite.handler.Rule$Handler.handle(Rule.java:108)
at 
org.eclipse.jetty.rewrite.handler.RewriteHandler.handle(RewriteHandler.java:143)
at 
org.eclipse.jetty.server.handler.gzip.GzipHandler.handle(GzipHandler.java:611)
at org.eclipse.jetty.server.Server.handle(Server.java:182)
at 
org.eclipse.jetty.server.internal.HttpChannelState$HandlerInvoker.run(HttpChannelState.java:662)
at 
org.eclipse.jetty.server.internal.HttpConnection.onFillable(HttpConnection.java:416)
at 
org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:322)
at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:99)
at 
org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53)
at 
org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:979)
at 
org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.doRunJob(QueuedThreadPool.java:1209)
at 
org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1164)
at java.base/java.lang.Thread.run(Thread.java:1583)
   ```


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-

Re: [PR] SOLR-17853: Migrate Plugins / Stats UI tab with Prometheus metrics [solr]

2025-09-09 Thread via GitHub 


mlbiscoc commented on code in PR #3623:
URL: https://github.com/apache/solr/pull/3623#discussion_r2333934398


##
solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java:
##
@@ -311,16 +311,24 @@ public void testAddDocs() throws Exception {
   waitForNumDocsInAllReplicas(numDocs, pullReplicas);
 
   for (Replica r : pullReplicas) {

Review Comment:
   Done!



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] SOLR-17853: Migrate Plugins / Stats UI tab with Prometheus metrics [solr]

2025-09-09 Thread via GitHub 


malliaridis commented on code in PR #3623:
URL: https://github.com/apache/solr/pull/3623#discussion_r2334599088


##
solr/webapp/web/js/angular/controllers/plugins.js:
##
@@ -66,23 +70,156 @@ solrAdminApp.controller('PluginsController',
 
 $scope.startRecording = function() {
 $scope.isRecording = true;
-Mbeans.reference({core: $routeParams.core}, function(data) {
-$scope.reference = data.reference;
-console.log($scope.reference);
-})
+$scope.refresh();
 }
 
 $scope.stopRecording = function() {
 $scope.isRecording = false;
-console.log($scope.reference);
-Mbeans.delta({core: $routeParams.core}, $scope.reference, 
function(data) {
-parseDelta($scope.types, data);
-});
+$scope.refresh();

Review Comment:
   I would love to have a call for the metrics covered in the new UI. If you 
are familiar with this topic I would appreciate all input you can provide. 😄 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] SOLR-17853: Migrate Plugins / Stats UI tab with Prometheus metrics [solr]

2025-09-09 Thread via GitHub 


mlbiscoc commented on code in PR #3623:
URL: https://github.com/apache/solr/pull/3623#discussion_r2333943465


##
solr/webapp/web/js/angular/controllers/plugins.js:
##
@@ -66,23 +70,156 @@ solrAdminApp.controller('PluginsController',
 
 $scope.startRecording = function() {
 $scope.isRecording = true;
-Mbeans.reference({core: $routeParams.core}, function(data) {
-$scope.reference = data.reference;
-console.log($scope.reference);
-})
+$scope.refresh();
 }
 
 $scope.stopRecording = function() {
 $scope.isRecording = false;
-console.log($scope.reference);
-Mbeans.delta({core: $routeParams.core}, $scope.reference, 
function(data) {
-parseDelta($scope.types, data);
-});
+$scope.refresh();

Review Comment:
   Correct, its supposed to store the differences between timestamps and it had 
something to do with the mbean handler I removed. I migrated it to just 
refresh. IMO I don't think Solr should do this difference calculation. I think 
Solr generating telemetry and showing it's current/last seen state is enough. 
If someone really cared about difference, collect the metrics in your 
timeseries backend and aggregate it in a way that is actually useful. Also the 
difference could be so large it might not even be useful to try and parse 
through the difference. Unless someone feels strongly about it, I just removed 
it and again in the new UI we should talk about better ways of displaying 
metrics.



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


Review Comment:
   Removed



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] SOLR-17853: Migrate Plugins / Stats UI tab with Prometheus metrics [solr]

2025-09-07 Thread via GitHub 


malliaridis commented on code in PR #3623:
URL: https://github.com/apache/solr/pull/3623#discussion_r2328597071


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


Review Comment:
   You may remove the following function variables in this file (usages 
removed):
   - `getPluginTypes`
   - `getPlugins`
   - `parseDelta`



##
solr/webapp/web/js/angular/controllers/plugins.js:
##
@@ -15,10 +15,8 @@
  limitations under the License.
 */
 
-//NOCOMMIT: This plugin seems tied to the Admin UIs plugin management but is 
tied to dropwizard metrics failing some tests.
-// This needs to change how it gets these metrics or we need to make a shim to 
the /admin/plugins handler for this to support it
 solrAdminApp.controller('PluginsController',
-function($scope, $rootScope, $routeParams, $location, Mbeans, Constants) {
+function($scope, $rootScope, $routeParams, $location, Mbeans, Metrics, 
Constants) {

Review Comment:
   You may remove Mbeans reference if no longer used. And the factory in 
`services.js` as well.



##
solr/webapp/web/js/angular/controllers/plugins.js:
##
@@ -66,23 +70,156 @@ solrAdminApp.controller('PluginsController',
 
 $scope.startRecording = function() {
 $scope.isRecording = true;
-Mbeans.reference({core: $routeParams.core}, function(data) {
-$scope.reference = data.reference;
-console.log($scope.reference);
-})
+$scope.refresh();
 }
 
 $scope.stopRecording = function() {
 $scope.isRecording = false;
-console.log($scope.reference);
-Mbeans.delta({core: $routeParams.core}, $scope.reference, 
function(data) {
-parseDelta($scope.types, data);
-});
+$scope.refresh();

Review Comment:
   Not sure what this recording behavior is used for, but judging by the 
previous functions called, I believe it was displaying only metric differences 
between two timestamps? Does calling the refresh function in both start and 
stop actions here make sense?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] SOLR-17853: Migrate Plugins / Stats UI tab with Prometheus metrics [solr]

2025-09-07 Thread via GitHub 


malliaridis commented on code in PR #3623:
URL: https://github.com/apache/solr/pull/3623#discussion_r2328590737


##
solr/core/src/java/org/apache/solr/handler/admin/SolrInfoMBeanHandler.java:
##
@@ -1,292 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.solr.handler.admin;
-
-import java.io.StringReader;
-import java.text.NumberFormat;
-import java.util.HashSet;
-import java.util.Locale;
-import java.util.Map;
-import java.util.Map.Entry;
-import java.util.Set;
-import org.apache.solr.client.solrj.impl.XMLResponseParser;
-import org.apache.solr.common.SolrException;
-import org.apache.solr.common.SolrException.ErrorCode;
-import org.apache.solr.common.util.ContentStream;
-import org.apache.solr.common.util.NamedList;
-import org.apache.solr.common.util.SimpleOrderedMap;
-import org.apache.solr.common.util.StrUtils;
-import org.apache.solr.core.SolrInfoBean;
-import org.apache.solr.handler.RequestHandlerBase;
-import org.apache.solr.request.SolrQueryRequest;
-import org.apache.solr.response.JavaBinResponseWriter;
-import org.apache.solr.response.SolrQueryResponse;
-import org.apache.solr.security.AuthorizationContext;
-
-/** A request handler that provides info about all registered SolrInfoMBeans. 
*/
-public class SolrInfoMBeanHandler extends RequestHandlerBase {

Review Comment:
   Anything specific to the Admin UI should be decoupled and if no use-cases 
exist or duplication occurs, simplify it by refactoring the code accordingly. 
+1 to have one endpoint only for the same thing and that can be used by all 
consumers dynamically.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] SOLR-17853: Migrate Plugins / Stats UI tab with Prometheus metrics [solr]

2025-09-06 Thread via GitHub 


dsmiley commented on code in PR #3623:
URL: https://github.com/apache/solr/pull/3623#discussion_r2327904358


##
solr/core/src/resources/ImplicitPlugins.json:
##


Review Comment:
   We'll need to remember to document the loss of these endpoints/handlers



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


Review Comment:
   Great job nonetheless!
   
   I do see lots of repetition of the labels.  I wonder if we should just 
remove uninteresting labels, or put at the top of the screen the labels we know 
that are going to be common if we're looking at a specific core.



##
solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java:
##
@@ -311,16 +311,24 @@ public void testAddDocs() throws Exception {
   waitForNumDocsInAllReplicas(numDocs, pullReplicas);
 
   for (Replica r : pullReplicas) {

Review Comment:
   this test seems a bit brittle... since it asserts null when maybe the 
condition/filter changes and is always null for other reasons.  I think it'd be 
better to simply iterate over all jetty's, iterate all cores, extract the 
metric of interest, then assert that it's either null or not null depending on 
the type of replica.



##
solr/core/src/test/org/apache/solr/cloud/TestPullReplicaWithAuth.java:
##
@@ -105,18 +103,35 @@ public void testPKIAuthWorksForPullReplication() throws 
Exception {
   numDocs, pullReplicas, "*:*", SecurityJson.USER, SecurityJson.PASS);
 
   for (Replica r : pullReplicas) {
-try (SolrClient pullReplicaClient = getHttpSolrClient(r)) {
-  QueryResponse statsResponse =
-  queryWithBasicAuth(
-  pullReplicaClient, new SolrQuery("qt", "/admin/plugins", 
"stats", "true"));
+JettySolrRunner jetty =
+cluster.getJettySolrRunners().stream()
+.filter(j -> j.getBaseUrl().toString().equals(r.getBaseUrl()))
+.findFirst()
+.orElse(null);
+assertNotNull("Could not find jetty for replica " + r, jetty);
+
+try (SolrCore core = 
jetty.getCoreContainer().getCore(r.getCoreName())) {
+  // Check that adds gauge metric is null/0 for pull replicas
+  var addOpsDatapoint =
+  SolrMetricTestUtils.getCounterDatapoint(
+  core,
+  "solr_core_update_committed_ops",
+  SolrMetricTestUtils.newCloudLabelsBuilder(core)
+  .label("category", "UPDATE")
+  .label("ops", "adds")
+  .build());
   // the 'adds' metric is a gauge, which is null for PULL replicas
+  assertNull("Replicas shouldn't process the add document request", 
addOpsDatapoint);
+  var cumulativeAddsDatapoint =
+  SolrMetricTestUtils.getGaugeDatapoint(
+  core,
+  "solr_core_update_cumulative_ops",
+  SolrMetricTestUtils.newCloudLabelsBuilder(core)
+  .label("category", "UPDATE")
+  .label("ops", "adds")
+  .build());
   assertNull(

Review Comment:
   same feedback applies



##
solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java:
##
@@ -311,16 +311,24 @@ public void testAddDocs() throws Exception {
   waitForNumDocsInAllReplicas(numDocs, pullReplicas);
 
   for (Replica r : pullReplicas) {
-try (SolrClient pullReplicaClient = getHttpSolrClient(r)) {
-  SolrQuery req = new SolrQuery("qt", "/admin/plugins", "stats", 
"true");
-  QueryResponse statsResponse = pullReplicaClient.query(req);
+JettySolrRunner jetty =
+cluster.getJettySolrRunners().stream()
+.filter(j -> j.getBaseUrl().toString().equals(r.getBaseUrl()))
+.findFirst()
+.orElse(null);

Review Comment:
   I suppose ideally the cluster get could the jetty by Url (a convenience 
method) but I dunno how often that need is



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] SOLR-17853: Migrate Plugins / Stats UI tab with Prometheus metrics [solr]

2025-09-05 Thread via GitHub 


mlbiscoc commented on code in PR #3623:
URL: https://github.com/apache/solr/pull/3623#discussion_r2325706010


##
solr/core/src/java/org/apache/solr/handler/admin/SolrInfoMBeanHandler.java:
##
@@ -1,292 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.solr.handler.admin;
-
-import java.io.StringReader;
-import java.text.NumberFormat;
-import java.util.HashSet;
-import java.util.Locale;
-import java.util.Map;
-import java.util.Map.Entry;
-import java.util.Set;
-import org.apache.solr.client.solrj.impl.XMLResponseParser;
-import org.apache.solr.common.SolrException;
-import org.apache.solr.common.SolrException.ErrorCode;
-import org.apache.solr.common.util.ContentStream;
-import org.apache.solr.common.util.NamedList;
-import org.apache.solr.common.util.SimpleOrderedMap;
-import org.apache.solr.common.util.StrUtils;
-import org.apache.solr.core.SolrInfoBean;
-import org.apache.solr.handler.RequestHandlerBase;
-import org.apache.solr.request.SolrQueryRequest;
-import org.apache.solr.response.JavaBinResponseWriter;
-import org.apache.solr.response.SolrQueryResponse;
-import org.apache.solr.security.AuthorizationContext;
-
-/** A request handler that provides info about all registered SolrInfoMBeans. 
*/
-public class SolrInfoMBeanHandler extends RequestHandlerBase {

Review Comment:
   This handler was being used to populate the admin UI "Plugins / Stats" tab 
on a core and its metrics. I did not like the idea of this tightly coupled with 
metric registries and another endpoint creating additional ways to get these 
metrics. I think there should only be one endpoint to get metrics which is 
`/admin/metrics` and create other formats there instead of making multiple 
handlers and endpoints to get metrics in different ways. If that is the case, 
implement and create a custom `OTEL metric reader` for `SolrMetricManager` to 
keep thing loosely coupled. Also the structure of this handler doesn't fit OTEL 
or prometheus with attributes/labels.



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


Review Comment:
   I liked the structure it came out to in the screenshot categorized by the 
`category` label with tabs of the prometheus metric -> labels and values. I am 
not good with javascript and frontend but I tried my best but tbh an LLM helped 
me clean up a lot of code and the styling in `plugins.css` after I did my 
initial write as I don't know best practices. Maybe @malliaridis you know more 
about this or if all the changes here make no sense? On another note for the 
new UI, I do not think we should follow this structure anymore going forward if 
we want to show metrics on the UI.



##
solr/core/src/java/org/apache/solr/handler/admin/PluginInfoHandler.java:
##
@@ -1,85 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.solr.handler.admin;
-
-import static org.apache.solr.common.params.CommonParams.NAME;
-
-import java.util.Map;
-import org.apache.solr.common.params.SolrParams;
-import org.apache.solr.common.util.SimpleOrderedMap;
-import org.apache.solr.core.SolrCore;
-import org.apache.solr.core.SolrInfoBean;
-import org.apache.solr.handler.RequestHandlerBase;
-import org.apache.solr.request.SolrQueryRequest;
-import org.apache.solr.response.SolrQueryResponse;
-import org.apache.solr.security.AuthorizationContext;
-
-/**
- * @since solr 1.2
- */
-public class PluginInfoHandler extends Reque