diff --git a/changelog/unreleased/PR#4871-httpSolrClientDefaultUrlParams.yml b/changelog/unreleased/PR#4871-httpSolrClientDefaultUrlParams.yml new file mode 100644 index 000000000000..7dfcae61bdf2 --- /dev/null +++ b/changelog/unreleased/PR#4871-httpSolrClientDefaultUrlParams.yml @@ -0,0 +1,9 @@ +title: > + HttpSolrClient impls now sends certain interesting request parameters in the URL query string when a POST of parameters is submitted. In other words, withTheseParamNamesInTheUrl now has a default set. + This improves observability, particularly for distributed search & admin commands. +type: changed +authors: + - name: David Smiley +links: + - name: PR#4871 + url: https://github.com/apache/solr/pull/4871 diff --git a/solr/modules/opentelemetry/src/test-files/solr/tracing/TestDistributedTracing/test.json b/solr/modules/opentelemetry/src/test-files/solr/tracing/TestDistributedTracing/test.json index 8cd2d28437af..87782172c2d6 100644 --- a/solr/modules/opentelemetry/src/test-files/solr/tracing/TestDistributedTracing/test.json +++ b/solr/modules/opentelemetry/src/test-files/solr/tracing/TestDistributedTracing/test.json @@ -84,7 +84,8 @@ "db.type":"solr", "http.request.method":"POST", "http.response.status_code":200, - "http.url":"http://NORMALIZED/solr/collection1_shard1_replica_nN/select"}, + "http.url":"http://NORMALIZED/solr/collection1_shard1_replica_nN/select", + "http.params":"distrib=false&isShard=true&shards.purpose=16388"}, { "name":"post:/{core}/select", "kind":"SERVER", @@ -92,7 +93,8 @@ "db.type":"solr", "http.request.method":"POST", "http.response.status_code":200, - "http.url":"http://NORMALIZED/solr/collection1_shard1_replica_nN/select"}, + "http.url":"http://NORMALIZED/solr/collection1_shard1_replica_nN/select", + "http.params":"distrib=false&isShard=true&shards.purpose=64"}, { "name":"post:/{core}/select", "kind":"SERVER", @@ -100,7 +102,8 @@ "db.type":"solr", "http.request.method":"POST", "http.response.status_code":200, - "http.url":"http://NORMALIZED/solr/collection1_shard2_replica_nN/select"}, + "http.url":"http://NORMALIZED/solr/collection1_shard2_replica_nN/select", + "http.params":"distrib=false&isShard=true&shards.purpose=16388"}, { "name":"post:/{core}/select", "kind":"SERVER", @@ -108,4 +111,5 @@ "db.type":"solr", "http.request.method":"POST", "http.response.status_code":200, - "http.url":"http://NORMALIZED/solr/collection1_shard2_replica_nN/select"}]}]}]} \ No newline at end of file + "http.url":"http://NORMALIZED/solr/collection1_shard2_replica_nN/select", + "http.params":"distrib=false&isShard=true&shards.purpose=64"}]}]}]} \ No newline at end of file diff --git a/solr/modules/opentelemetry/src/test-files/solr/tracing/TestDistributedTracing/testV2Api.json b/solr/modules/opentelemetry/src/test-files/solr/tracing/TestDistributedTracing/testV2Api.json index 975aac135329..047c3acc1f08 100644 --- a/solr/modules/opentelemetry/src/test-files/solr/tracing/TestDistributedTracing/testV2Api.json +++ b/solr/modules/opentelemetry/src/test-files/solr/tracing/TestDistributedTracing/testV2Api.json @@ -8,37 +8,41 @@ "db.instance":"collection1", "children":[ { - "name":"post:/admin/cores", + "name":"reload:/admin/cores", "kind":"SERVER", "db.instance":"collection1_shard1_replica_nN", "db.type":"solr", "http.request.method":"POST", "http.response.status_code":200, - "http.url":"http://NORMALIZED/solr/admin/cores"}, + "http.url":"http://NORMALIZED/solr/admin/cores", + "http.params":"action=RELOAD"}, { - "name":"post:/admin/cores", + "name":"reload:/admin/cores", "kind":"SERVER", "db.instance":"collection1_shard1_replica_nN", "db.type":"solr", "http.request.method":"POST", "http.response.status_code":200, - "http.url":"http://NORMALIZED/solr/admin/cores"}, + "http.url":"http://NORMALIZED/solr/admin/cores", + "http.params":"action=RELOAD"}, { - "name":"post:/admin/cores", + "name":"reload:/admin/cores", "kind":"SERVER", "db.instance":"collection1_shard2_replica_nN", "db.type":"solr", "http.request.method":"POST", "http.response.status_code":200, - "http.url":"http://NORMALIZED/solr/admin/cores"}, + "http.url":"http://NORMALIZED/solr/admin/cores", + "http.params":"action=RELOAD"}, { - "name":"post:/admin/cores", + "name":"reload:/admin/cores", "kind":"SERVER", "db.instance":"collection1_shard2_replica_nN", "db.type":"solr", "http.request.method":"POST", "http.response.status_code":200, - "http.url":"http://NORMALIZED/solr/admin/cores"}]}, + "http.url":"http://NORMALIZED/solr/admin/cores", + "http.params":"action=RELOAD"}]}, { "name":"post:/collections/{collection}/reload", "kind":"SERVER", @@ -124,7 +128,8 @@ "db.type":"solr", "http.request.method":"POST", "http.response.status_code":200, - "http.url":"http://NORMALIZED/solr/collection1_shard1_replica_nN/select"}, + "http.url":"http://NORMALIZED/solr/collection1_shard1_replica_nN/select", + "http.params":"distrib=false&isShard=true&shards.purpose=16388"}, { "name":"post:/{core}/select", "kind":"SERVER", @@ -132,7 +137,8 @@ "db.type":"solr", "http.request.method":"POST", "http.response.status_code":200, - "http.url":"http://NORMALIZED/solr/collection1_shard2_replica_nN/select"}, + "http.url":"http://NORMALIZED/solr/collection1_shard2_replica_nN/select", + "http.params":"distrib=false&isShard=true&shards.purpose=16388"}, { "name":"post:/{core}/select", "kind":"SERVER", @@ -140,4 +146,5 @@ "db.type":"solr", "http.request.method":"POST", "http.response.status_code":200, - "http.url":"http://NORMALIZED/solr/collection1_shard2_replica_nN/select"}]}]}]} \ No newline at end of file + "http.url":"http://NORMALIZED/solr/collection1_shard2_replica_nN/select", + "http.params":"distrib=false&isShard=true&shards.purpose=64"}]}]}]} \ No newline at end of file diff --git a/solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/TestDistributedTracing.java b/solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/TestDistributedTracing.java index f73d47060032..f713a604e62d 100644 --- a/solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/TestDistributedTracing.java +++ b/solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/TestDistributedTracing.java @@ -201,7 +201,7 @@ private void verifyCollectionCreation(String collection) throws Exception { // db.instance=testInternalCollectionApiCommands // - this will be the parent span, all following spans will have the same traceId // - // 3..6 (4 times) name=post:/admin/cores + // 3..6 (4 times) name=create:/admin/cores // db.instance=testInternalCollectionApiCommands_shard1_replica_n2 // db.instance=testInternalCollectionApiCommands_shard2_replica_n4 // db.instance=testInternalCollectionApiCommands_shard2_replica_n1 @@ -238,7 +238,7 @@ private void verifyCollectionCreation(String collection) throws Exception { ops.put(span.getName(), ops.getOrDefault(span.getName(), 0) + 1); } var expectedOps = - Map.of("CreateCollectionCmd", 1, "post:/admin/cores", 4, "post:/{core}/get", 6); + Map.of("CreateCollectionCmd", 1, "create:/admin/cores", 4, "post:/{core}/get", 6); assertEquals(expectedOps, ops); } @@ -254,7 +254,7 @@ private void verifyCollectionDeletion(String collection) throws Exception { // db.instance=testInternalCollectionApiCommands // - this will be the parent span, all following spans will have the same traceId // - // 3..6 (4 times) name=post:/admin/cores + // 3..6 (4 times) name=unload:/admin/cores // db.instance=testInternalCollectionApiCommands_shard2_replica_n1 // db.instance=testInternalCollectionApiCommands_shard1_replica_n2 // db.instance=testInternalCollectionApiCommands_shard2_replica_n4 @@ -278,7 +278,7 @@ private void verifyCollectionDeletion(String collection) throws Exception { assertEquals(span.getTraceId(), parentTraceId); ops.put(span.getName(), ops.getOrDefault(span.getName(), 0) + 1); } - var expectedOps = Map.of("DeleteCollectionCmd", 1, "post:/admin/cores", 4); + var expectedOps = Map.of("DeleteCollectionCmd", 1, "unload:/admin/cores", 4); assertEquals(expectedOps, ops); } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java index e79555c7bdac..2926ab78c0dd 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java @@ -46,8 +46,12 @@ import org.apache.solr.client.solrj.response.ResponseParser; import org.apache.solr.client.solrj.util.ClientUtils; import org.apache.solr.common.SolrException; +import org.apache.solr.common.params.CollectionAdminParams; +import org.apache.solr.common.params.CommonAdminParams; import org.apache.solr.common.params.CommonParams; +import org.apache.solr.common.params.CoreAdminParams; import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.params.ShardParams; import org.apache.solr.common.util.NamedList; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -65,6 +69,20 @@ public abstract class HttpSolrClient extends SolrClient { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); protected static final Charset FALLBACK_CHARSET = StandardCharsets.UTF_8; + /** Default set are interesting for routing or fundamental request purpose */ + private static final Set DEFAULT_URL_PARAM_NAMES = + Set.of( + CoreAdminParams.ACTION, + CommonAdminParams.ASYNC, + CollectionAdminParams.COLLECTION, + "name", // core/collection name + "command", // e.g. for replication + ShardParams.IS_SHARD, + CommonParams.DISTRIB, + ShardParams._ROUTE_, + ShardParams.SHARDS_PREFERENCE, + ShardParams.SHARDS_PURPOSE); + protected final String baseUrl; protected final long requestTimeoutMillis; @@ -87,11 +105,7 @@ protected HttpSolrClient(String serverBaseUrl, BuilderBase builder) { this.parser = builder.responseParser; } this.defaultCollection = builder.defaultCollection; - if (builder.urlParamNames != null) { - this.urlParamNames = builder.urlParamNames; - } else { - this.urlParamNames = Set.of(); - } + this.urlParamNames = Objects.requireNonNullElse(builder.urlParamNames, DEFAULT_URL_PARAM_NAMES); } private static String extractBaseUrl(String serverBaseUrl) {