dsmiley commented on a change in pull request #115:
URL: https://github.com/apache/solr/pull/115#discussion_r631756496



##########
File path: solr/core/src/java/org/apache/solr/api/V2HttpCall.java
##########
@@ -360,6 +366,85 @@ protected void execute(SolrQueryResponse rsp) {
     SolrCore.postDecorateResponse(handler, solrReq, rsp);
   }
 
+  @Override
+  protected void populateTracingSpan(Span span) {
+    // Set db.instance
+    String coreOrColName = this.origCorename;
+    if (coreOrColName == null) {
+      Map<String, String> pathTemplateValues = getUrlParts(); // == 
solrReq.getPathTemplateValues()
+      coreOrColName = pathTemplateValues.get("collection");
+      if (coreOrColName == null) {
+        coreOrColName = pathTemplateValues.get("core");
+      }
+    }
+    if (coreOrColName != null) {
+      span.setTag(Tags.DB_INSTANCE, coreOrColName);
+    }
+
+    // Get the templatize-ed path
+    String path;
+    if (api instanceof AnnotatedApi) {
+      // ideal scenario; eventually everything might be AnnotatedApi?
+      var aapi = (AnnotatedApi) api;
+      path = aapi.getEndPoint().path()[0]; // consider first to be primary
+    } else {
+      path = computeEndpointPath();
+      // TODO consider getValidators, looking for command & path?

Review comment:
       @noblepaul what do you think of my changes in V2HttpCall?  I figured out 
how to compute both the templatized path and also the command verb.  At this 
particular spot, I wonder if it may be better to call getValidators to find the 
command & path?  At least getValidators appears cached.  

##########
File path: solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
##########
@@ -549,7 +546,9 @@ public InputStream getStream() throws IOException {
     public SolrParams parseParamsAndFillStreams( 
         final HttpServletRequest req, ArrayList<ContentStream> streams ) 
throws Exception
     {
-      streams.add( new HttpRequestContentStream( req ) );
+      if (!req.getMethod().equals("GET")) { // or other conditions? Check 
headers?
+        streams.add(new HttpRequestContentStream(req));
+      }

Review comment:
       @noblepaul  when doing a GET request for V2, I found it to be a problem 
that we add this stream that is empty, resulting in an exception at some point 
about a blank Content-Type (as there is no content in the first place).  
StandardRequestParser.parseParamsAndFillStreams checks !isPost and isV2 and 
then calls this raw impl.
   I could probably be more thorough here in detecting the presence of a 
stream.  Just because it's not a GET doesn't mean there is a stream!  Weirdly, 
req.getInputStream can return non-null for a GET which I observed in a test I 
added in this PR.




-- 
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.

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