dsmiley commented on code in PR #3233:
URL: https://github.com/apache/solr/pull/3233#discussion_r1976742439
##########
solr/core/src/java/org/apache/solr/util/stats/InstrumentedHttpListenerFactory.java:
##########
@@ -86,38 +87,44 @@ private static String methodNameString(Request request) {
}
@Override
- public RequestResponseListener get() {
- return new RequestResponseListener() {
- Timer.Context timerContext;
- Span span = Span.getInvalid();
-
- @Override
- public void onQueued(Request request) {
- // do tracing onQueued because it's called from Solr's thread
- span = Span.current();
- TraceUtils.injectTraceContext(request);
- }
+ public final void onQueued(Request req) {
+ var listener = new PerRequestListener(req);
Review Comment:
this scenario is maybe a bit more awkward because on onQueued, we need to
register a listener specific for this request because the listener has
per-request state (fields). This was previously happening for all listeners in
Http2SolrClient but you'll see it gets removed there now.
##########
solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java:
##########
@@ -377,8 +376,8 @@ PublicKey fetchPublicKeyFromRemote(String nodename) {
@Override
public void setup(Http2SolrClient client) {
- final HttpListenerFactory.RequestResponseListener listener =
- new HttpListenerFactory.RequestResponseListener() {
+ final var listener =
Review Comment:
this listener no per-request state so can simply add this listener to client
level.
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java:
##########
@@ -632,12 +613,6 @@ private void decorateRequest(Request req, SolrRequest<?>
solrRequest, boolean is
}
setBasicAuthHeader(solrRequest, req);
- for (HttpListenerFactory factory : listenerFactory) {
Review Comment:
No longer need to do this; the listeners are now at httpClient level, not
request level. Granted the listener onQueued can choose to register another
listener (and one does do this).
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/PreemptiveBasicAuthClientBuilderFactory.java:
##########
@@ -95,10 +95,9 @@ public void setup(Http2SolrClient client, String
basicAuthUser, String basicAuth
authenticationStore.addAuthentication(
new SolrBasicAuthentication(basicAuthUser, basicAuthPass));
client.setAuthenticationStore(authenticationStore);
- client.getProtocolHandlers().put(new
WWWAuthenticationProtocolHandler(client.getHttpClient()));
Review Comment:
because I removed client.getProtocolHandlers() now that HttpClient is exposed
--
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]
