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