stillalex commented on code in PR #1884:
URL: https://github.com/apache/solr/pull/1884#discussion_r1315841099


##########
solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/BasicAuthIntegrationTracingTest.java:
##########
@@ -99,5 +104,12 @@ public void testSetupBasicAuth() throws Exception {
             .build();
     req.setBasicAuthCredentials(USER, PASS);
     assertEquals(0, req.process(cloudClient, COLLECTION).getStatus());
+
+    var finishedSpans = getAndClearSpans();
+    assertEquals(1, finishedSpans.size());
+    var span = finishedSpans.get(0);
+    assertEquals("post:/cluster/security/authentication", span.getName());
+    assertEquals("solr", span.getAttributes().get(TraceUtils.TAG_USER));
+    assertEquals(List.of("set-user"), 
span.getAttributes().get(TraceUtils.TAG_OPS));

Review Comment:
   could you explain a bit better what you expect to see in the operation name? 
I thought having `post:/cluster/security/authentication` is in line with all 
other apis.
   also one part that is different here is that it's not a single op, there is 
a list in the body, so I went with a list attribute and the previous approach 
of taking the first entry to set as a name seems fragile. I am open to 
suggestions on what we could do.



-- 
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: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to