HPCC4J-612 Code review

- Removes commented out lines - Adds WithSpan annotations to wsclientpool - Adds hascredentials span attribute to current span - Ensures executeECLScript is traced - Externalizes opentelemetry-instrumentation-annotations version variable Signed-off-by: Pastrana <[email protected]>
hpcc-systems · Sep 13, 2024 · 698c498 · 698c498
1 parent f68e38b
commit 698c498
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 46 deletions.
diff --git a/pom.xml b/pom.xml
@@ -64,6 +64,7 @@
     <project.benchmarking>false</project.benchmarking>
     <opentelemetry.bom.version>1.38.0</opentelemetry.bom.version>
     <opentelemetry.semconv.version>1.25.0-alpha</opentelemetry.semconv.version>
+    <opentelemetry.instrumentation.annotations.version>2.6.0</opentelemetry.instrumentation.annotations.version>
   </properties>
 
   <scm>
@@ -139,7 +140,7 @@
       <!-- Not managed by opentelemetry-bom -->
       <groupId>io.opentelemetry.instrumentation</groupId>
       <artifactId>opentelemetry-instrumentation-annotations</artifactId>
-      <version>2.6.0</version>
+      <version>${opentelemetry.instrumentation.annotations.version}</version>
     </dependency>
     <dependency>
       <!-- Not managed by opentelemetry-bom -->

diff --git a/wsclient/src/main/java/org/hpccsystems/ws/client/HPCCWsClientPool.java b/wsclient/src/main/java/org/hpccsystems/ws/client/HPCCWsClientPool.java
@@ -3,7 +3,7 @@
 import org.hpccsystems.ws.client.utils.Connection;
 import org.hpccsystems.ws.client.utils.ObjectPool;
 
-//import io.opentelemetry.instrumentation.annotations.WithSpan;
+import io.opentelemetry.instrumentation.annotations.WithSpan;
 
 /**
  * <p>HPCCWsClientPool class.</p>
@@ -78,7 +78,7 @@ public HPCCWsClientPool(Connection connection)
      * @param timeout
      *            the timeout
      */
-    //@WithSpan
+    @WithSpan
     public HPCCWsClientPool(Connection connection, long timeout)
     {
         super(timeout);
@@ -92,7 +92,7 @@ public HPCCWsClientPool(Connection connection, long timeout)
      */
     /** {@inheritDoc} */
     @Override
-    //@WithSpan
+    @WithSpan
     protected HPCCWsClient create()
     {
         return (new HPCCWsClient(m_connection));
@@ -105,6 +105,7 @@ protected HPCCWsClient create()
      */
     /** {@inheritDoc} */
     @Override
+    @WithSpan
     public boolean validate(HPCCWsClient client)
     {
         try

diff --git a/wsclient/src/main/java/org/hpccsystems/ws/client/HPCCWsStoreClient.java b/wsclient/src/main/java/org/hpccsystems/ws/client/HPCCWsStoreClient.java
@@ -42,8 +42,6 @@
 import io.opentelemetry.instrumentation.annotations.SpanAttribute;
 import io.opentelemetry.instrumentation.annotations.WithSpan;
 
-//import io.opentelemetry.instrumentation.annotations.WithSpan;
-
 /**
  * Facilitates access to HPCC Systems key/value based Storage.
  *

diff --git a/wsclient/src/main/java/org/hpccsystems/ws/client/utils/Connection.java b/wsclient/src/main/java/org/hpccsystems/ws/client/utils/Connection.java
@@ -1065,61 +1065,50 @@ public String sendHTTPRequest(@SpanAttribute String uri, @SpanAttribute String m
 
         URL url = new URL (getBaseUrl() + (uri != null && uri.startsWith("/") ? "" : "/") + uri);
 
-        /*Span sendHTTPReqSpan = GlobalOpenTelemetry.get().getTracer(BaseHPCCWsClient.PROJECT_NAME)
-                .spanBuilder(method.toUpperCase() + " " + url.toExternalForm())
-                .setAttribute(ServerAttributes.SERVER_ADDRESS, getHost())
-                .setAttribute(ServerAttributes.SERVER_PORT, Long.getLong(getPort()))
-                .setAttribute(HttpAttributes.HTTP_REQUEST_METHOD, method)
-                .setSpanKind(SpanKind.CLIENT)
-                .startSpan();
-*/
         HttpURLConnection httpURLConnection = (HttpURLConnection) url.openConnection(); //throws IOException
 
         Connection.log.info("Sending HTTP " + method + "Request to:" + url.toString());
 
+        boolean isTraced = Span.current().isRecording();
         if (hasCredentials())
         {
             httpURLConnection.setRequestProperty("Authorization", getBasicAuthString());
-            //sendHTTPReqSpan.setAttribute("hasCredentials", true);
+            if (isTraced)
+                Span.current().setAttribute("hasCredentials", true);
         }
-        //else
-        //{
-            //sendHTTPReqSpan.setAttribute("hasCredentials", false);
-        //}
-
-        //try (Scope scope = sendHTTPReqSpan.makeCurrent())
+        else
         {
-            httpURLConnection.setRequestProperty("traceparent", Utils.getCurrentSpanTraceParentHeader());
-            httpURLConnection.setRequestMethod(method); //throws ProtocolException
+            if (isTraced)
+                Span.current().setAttribute("hasCredentials", false);
+        }
 
-            int responseCode = httpURLConnection.getResponseCode(); //throws IOException
-            //sendHTTPReqSpan.setAttribute("http.response.status_code", responseCode);
-            Connection.log.info("HTTP Response code: " + responseCode);
+        if (isTraced)
+             httpURLConnection.setRequestProperty("traceparent", Utils.getCurrentSpanTraceParentHeader());
 
-            if (responseCode == HttpURLConnection.HTTP_OK) //success
-            {
-                BufferedReader in = new BufferedReader(new InputStreamReader(httpURLConnection.getInputStream())); //throws IOException
-                String inputLine;
-                StringBuffer response = new StringBuffer();
+        httpURLConnection.setRequestMethod(method); //throws ProtocolException
 
-                while ((inputLine = in.readLine()) != null) // throws IOException
-                {
-                    response.append(inputLine);
-                }
+        int responseCode = httpURLConnection.getResponseCode(); //throws IOException
 
-                in.close(); //throws IOException
-              //  sendHTTPReqSpan.setStatus(StatusCode.OK);
-                return response.toString();
-            }
-            else
+        Connection.log.info("HTTP Response code: " + responseCode);
+
+        if (responseCode == HttpURLConnection.HTTP_OK) //success
+        {
+            BufferedReader in = new BufferedReader(new InputStreamReader(httpURLConnection.getInputStream())); //throws IOException
+            String inputLine;
+            StringBuffer response = new StringBuffer();
+
+            while ((inputLine = in.readLine()) != null) // throws IOException
             {
-                //sendHTTPReqSpan.setStatus(StatusCode.ERROR);
-                throw new IOException("HTTP request failed! Code (" + responseCode + ") " + httpURLConnection.getResponseMessage() );
+                response.append(inputLine);
             }
+
+            in.close(); //throws IOException
+
+            return response.toString();
+        }
+        else
+        {
+            throw new IOException("HTTP request failed! Code (" + responseCode + ") " + httpURLConnection.getResponseMessage() );
         }
-        //finally
-        //{
-        //    sendHTTPReqSpan.end();
-        //}
     }
 }
diff --git a/wsclient/src/test/java/org/hpccsystems/ws/client/BaseRemoteTest.java b/wsclient/src/test/java/org/hpccsystems/ws/client/BaseRemoteTest.java
@@ -45,6 +45,7 @@ HPCC SYSTEMS software Copyright (C) 2019 HPCC Systems®.
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.trace.SpanBuilder;
 import io.opentelemetry.api.trace.Tracer;
+import io.opentelemetry.instrumentation.annotations.WithSpan;
 import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
 
 @Category(org.hpccsystems.commons.annotations.RemoteTests.class)
@@ -285,6 +286,7 @@ public boolean verify(String hostname,javax.net.ssl.SSLSession sslSession)
         }
     }
 
+    @WithSpan
     public static String executeECLScript(String eclFile) throws Exception
     {
         InputStream resourceStream = BaseRemoteTest.class.getClassLoader().getResourceAsStream(eclFile);