dsmiley commented on code in PR #1856:
URL: https://github.com/apache/solr/pull/1856#discussion_r1320994177
##########
solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java:
##########
@@ -150,4 +167,11 @@ public static void setOperations(SolrQueryRequest req,
String clazz, List<String
req.getSpan().setAttribute(TAG_CLASS, clazz);
}
}
+
+ public static Span newInterNodeCommunicationSpan(String name, String
collection) {
Review Comment:
Still waiting on the SpanKind change to be "async" based.
Also... this method feels like it was generalized too soon. We are using it
in a specific spot where the command is executed, either by the Overseer or in
"distributed" mode. It isn't necessarily "interNodeCommunicationSpan" and I'm
not sure that internNode vs non-InterNode is meaningful.
##########
solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/TestDistributedTracing.java:
##########
@@ -173,20 +167,137 @@ public void testV2Api() throws Exception {
.process(cloudClient);
finishedSpans = getAndClearSpans();
assertEquals("get:/c/{collection}/select", finishedSpans.get(0).getName());
- assertCollectionName(finishedSpans.get(0));
+ assertCollectionName(finishedSpans.get(0), COLLECTION);
assertEquals(1, ((SolrDocumentList)
v2Response.getResponse().get("response")).getNumFound());
}
+ /**
+ * Best effort test of the apache http client tracing. the test assumes the
request uses the http
+ * client but there is no way to enforce it, so when the api will be
rewritten this test will
+ * become obsolete
+ */
+ @Test
+ public void testApacheClient() throws Exception {
+ getAndClearSpans(); // reset
+
+ CollectionAdminRequest.ColStatus a1 =
CollectionAdminRequest.collectionStatus(COLLECTION);
+ CollectionAdminResponse r1 = a1.process(cluster.getSolrClient());
+ assertEquals(0, r1.getStatus());
+ var finishedSpans = getAndClearSpans();
+ var parentTraceId = getRootTraceId(finishedSpans);
+ for (var span : finishedSpans) {
+ if (isRootSpan(span)) {
+ continue;
+ }
+ assertEquals(span.getParentSpanContext().getTraceId(), parentTraceId);
+ assertEquals(span.getTraceId(), parentTraceId);
+ }
+ }
+
+ @Test
+ public void testInternalCollectionApiCommands() throws Exception {
+ String collecton = "testInternalCollectionApiCommands";
+ verifyCollectionCreation(collecton);
+ verifyCollectionDeletion(collecton);
+ }
+
+ private void verifyCollectionCreation(String collection) throws Exception {
+ getAndClearSpans(); // reset
Review Comment:
feels like this should be in a `@Before`
--
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]
