HPCC-586 Code review

- Removes dependancy on otlp exporter - Renames project name variable - Adds manual http span on Utils.Connection.sendHTTPRequest - Injects traceparent http header on outgoing Utils.Connection.sendHTTPRequest - Only attempts to autoconfig remotetest if otel.java.global-autoconfig enabled - Catches autoconfiguration exeptions - Adds root span around platform tester Signed-off-by: Rodrigo Pastrana <[email protected]>
hpcc-systems · Jun 4, 2024 · 15cce48 · 15cce48
1 parent ad53e15
commit 15cce48
Show file tree

Hide file tree

Showing 5 changed files with 343 additions and 229 deletions.
diff --git a/pom.xml b/pom.xml
@@ -113,35 +113,31 @@
   </dependencyManagement>
   <dependencies>
     <dependency>
-            <groupId>io.opentelemetry</groupId>
-            <artifactId>opentelemetry-api</artifactId>
-        </dependency>
-        <dependency>
-            <groupId>io.opentelemetry</groupId>
-            <artifactId>opentelemetry-sdk</artifactId>
-        </dependency>
-        <dependency>
-            <groupId>io.opentelemetry</groupId>
-            <artifactId>opentelemetry-exporter-logging</artifactId>
-        </dependency>
-        <dependency>
-          <groupId>io.opentelemetry</groupId>
-          <artifactId>opentelemetry-sdk-extension-autoconfigure</artifactId>
-        </dependency>
-        <dependency>
-          <groupId>io.opentelemetry</groupId>
-          <artifactId>opentelemetry-sdk-extension-autoconfigure-spi</artifactId>
-        </dependency>
-        <dependency>
-          <groupId>io.opentelemetry</groupId>
-          <artifactId>opentelemetry-exporter-otlp</artifactId>
-        </dependency>
-        <dependency>
-          <!-- Not managed by opentelemetry-bom -->
-          <groupId>io.opentelemetry.semconv</groupId>
-          <artifactId>opentelemetry-semconv</artifactId>
-          <version>1.25.0-alpha</version>
-        </dependency>
+      <groupId>io.opentelemetry</groupId>
+      <artifactId>opentelemetry-api</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>io.opentelemetry</groupId>
+      <artifactId>opentelemetry-sdk</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>io.opentelemetry</groupId>
+      <artifactId>opentelemetry-exporter-logging</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>io.opentelemetry</groupId>
+      <artifactId>opentelemetry-sdk-extension-autoconfigure</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>io.opentelemetry</groupId>
+      <artifactId>opentelemetry-sdk-extension-autoconfigure-spi</artifactId>
+    </dependency>
+    <dependency>
+      <!-- Not managed by opentelemetry-bom -->
+      <groupId>io.opentelemetry.semconv</groupId>
+      <artifactId>opentelemetry-semconv</artifactId>
+      <version>1.25.0-alpha</version>
+    </dependency>
     <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>

diff --git a/wsclient/src/main/java/org/hpccsystems/ws/client/BaseHPCCWsClient.java b/wsclient/src/main/java/org/hpccsystems/ws/client/BaseHPCCWsClient.java
@@ -63,7 +63,7 @@
  */
 public abstract class BaseHPCCWsClient extends DataSingleton
 {
-    public static final String INSTRUMENTED_LIBRARY_NAME = "WsClient";
+    public static final String PROJECT_NAME = "WsClient";
     private static OpenTelemetry globalOTel = null;
     /** Constant <code>log</code> */
     protected static final Logger log                    = LogManager.getLogger(BaseHPCCWsClient.class);
@@ -253,7 +253,7 @@ public Tracer getWsClientTracer()
         if (globalOTel == null)
             initOTel();
 
-        return globalOTel.getTracer(INSTRUMENTED_LIBRARY_NAME);
+        return globalOTel.getTracer(PROJECT_NAME);
     }
     /**
      * All instances of HPCCWsXYZClient should utilize this init function

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
@@ -12,9 +12,23 @@
 import java.util.Base64;
 import java.util.Base64.Decoder;
 import java.util.Base64.Encoder;
+import java.util.HashMap;
+import java.util.Map;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.hpccsystems.ws.client.BaseHPCCWsClient;
+
+import io.opentelemetry.api.GlobalOpenTelemetry;
+import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.api.trace.SpanKind;
+import io.opentelemetry.api.trace.StatusCode;
+import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator;
+import io.opentelemetry.context.Context;
+import io.opentelemetry.context.Scope;
+import io.opentelemetry.context.propagation.TextMapSetter;
+import io.opentelemetry.semconv.HttpAttributes;
+import io.opentelemetry.semconv.ServerAttributes;
 
 /**
  * Represents and structures connection information.
@@ -1028,39 +1042,71 @@ public String sendHTTPRequest(String uri, String method) throws Exception
 
         URL url = new URL (getBaseUrl() + (uri != null && uri.startsWith("/") ? "" : "/") + uri);
 
-        HttpURLConnection httpURLConnection = (HttpURLConnection) url.openConnection(); //throws IOException
+        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, HttpAttributes.HttpRequestMethodValues.GET)
+                .setSpanKind(SpanKind.CLIENT)
+                .startSpan();
 
-         Connection.log.info("Sending HTTP " + method + "Request to:" + url.toString());
+        HttpURLConnection httpURLConnection = (HttpURLConnection) url.openConnection(); //throws IOException
 
-         if (hasCredentials())
-         {
-             httpURLConnection.setRequestProperty("Authorization", getBasicAuthString());
-         }
+        Connection.log.info("Sending HTTP " + method + "Request to:" + url.toString());
 
-         httpURLConnection.setRequestMethod(method); //throws ProtocolException
+        if (hasCredentials())
+        {
+            httpURLConnection.setRequestProperty("Authorization", getBasicAuthString());
+            sendHTTPReqSpan.setAttribute("hasCredentials", true);
+        }
+        else
+        {
+            sendHTTPReqSpan.setAttribute("hasCredentials", false);
+        }
 
-         int responseCode = httpURLConnection.getResponseCode(); //throws IOException
+        try (Scope scope = sendHTTPReqSpan.makeCurrent())
+        {
+            Span currentSpan = Span.current();
+            if (currentSpan != null && currentSpan.getSpanContext().isValid())
+            {
+                //overkill? we could just construct traceparent manually
+                Map<String, String> carrier = new HashMap<>();
+                TextMapSetter<Map<String, String>> setter = Map::put;
+                W3CTraceContextPropagator.getInstance().inject(Context.current(), carrier, setter);
+                String traceparent = carrier.getOrDefault("traceparent", "00-" + currentSpan.getSpanContext().getTraceId() + "-" + currentSpan.getSpanContext().getSpanId() + "-00");
 
-         Connection.log.info("HTTP Response code: " + responseCode);
+                httpURLConnection.setRequestProperty("traceparent", traceparent);
+            }
+            httpURLConnection.setRequestMethod(method); //throws ProtocolException
 
-         if (responseCode == HttpURLConnection.HTTP_OK) //success
-         {
-             BufferedReader in = new BufferedReader(new InputStreamReader(httpURLConnection.getInputStream())); //throws IOException
-             String inputLine;
-             StringBuffer response = new StringBuffer();
+            int responseCode = httpURLConnection.getResponseCode(); //throws IOException
+            sendHTTPReqSpan.setAttribute("http.response.status_code", responseCode);
+            Connection.log.info("HTTP Response code: " + responseCode);
 
-             while ((inputLine = in.readLine()) != null) // throws IOException
-             {
-                 response.append(inputLine);
-             }
+            if (responseCode == HttpURLConnection.HTTP_OK) //success
+            {
+                BufferedReader in = new BufferedReader(new InputStreamReader(httpURLConnection.getInputStream())); //throws IOException
+                String inputLine;
+                StringBuffer response = new StringBuffer();
 
-             in.close(); //throws IOException
+                while ((inputLine = in.readLine()) != null) // throws IOException
+                {
+                    response.append(inputLine);
+                }
 
-             return response.toString();
-         }
-         else
-         {
-             throw new IOException("HTTP request failed! Code (" + responseCode + ") " + httpURLConnection.getResponseMessage() );
-         }
+                in.close(); //throws IOException
+                sendHTTPReqSpan.setStatus(StatusCode.OK);
+                return response.toString();
+            }
+            else
+            {
+                sendHTTPReqSpan.setStatus(StatusCode.ERROR);
+                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
@@ -40,9 +40,11 @@ HPCC SYSTEMS software Copyright (C) 2019 HPCC Systems®.
 import org.junit.BeforeClass;
 import org.junit.experimental.categories.Category;
 
+import io.opentelemetry.api.GlobalOpenTelemetry;
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.trace.SpanBuilder;
 import io.opentelemetry.api.trace.Tracer;
+import io.opentelemetry.api.trace.TracerProvider;
 import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
 
 @Category(org.hpccsystems.commons.annotations.RemoteTests.class)
@@ -70,7 +72,7 @@ public abstract class BaseRemoteTest
 
     protected final static int  testThreadCount = Integer.parseInt(System.getProperty("testthreadcount", "10"));
     public static final String DEFAULTHPCCFILENAME      = "benchmark::all_types::200kb";
-    protected static OpenTelemetry globalOTel = AutoConfiguredOpenTelemetrySdk.initialize().getOpenTelemetrySdk();
+    protected static OpenTelemetry globalOTel = null;
 
     /*
      * Code to generate superfile with default file as subfile
@@ -120,6 +122,31 @@ public static SpanBuilder getRemoteTestTraceBuilder(String spanName)
 
     public static void initialize() throws Exception
     {
+        if (Boolean.getBoolean("otel.java.global-autoconfigure.enabled"))
+        {
+            System.out.println("OpenTelemetry autoconfiguration enabled:");
+            System.out.println("    otel.traces.exporter sys property: "+System.getProperty("otel.traces.exporter"));
+            System.out.println("    OTEL_TRACES_EXPORTER Env var: " + System.getenv("OTEL_TRACES_EXPORTER"));
+            System.out.println("        OTEL_TRACES_SAMPLER Env var: " + System.getenv("OTEL_TRACES_SAMPLER"));
+            System.out.println("        otel.traces.sampler sys property: "+System.getProperty("otel.traces.sampler"));
+            System.out.println("    otel.logs.exporter: "+ System.getProperty("otel.logs.exporter"));
+            System.out.println("    OTEL_LOGS_EXPORTER Env var: " + System.getenv("OTEL_LOGS_EXPORTER"));
+            System.out.println("    otel.metrics.exporter: "+ System.getProperty("otel.metrics.exporter"));
+            System.out.println("    OTEL_METRICS_EXPORTER Env var: " + System.getenv("OTEL_METRICS_EXPORTER"));
+            try
+            {
+                globalOTel = AutoConfiguredOpenTelemetrySdk.initialize().getOpenTelemetrySdk();
+            }
+            catch (Exception e)
+            {
+                System.out.println("OpenTelemetry autoconfiguration failed: " + e.getMessage());
+            }
+        }
+        else
+        {
+            globalOTel = GlobalOpenTelemetry.get();
+        }
+
         // This allows testing against locally created self signed certs to work.
         // In production certs will need to be created valid hostnames
         javax.net.ssl.HttpsURLConnection.setDefaultHostnameVerifier(