HPCC-586 Code review2

- Moves getCurrentSpanTraceParentHeader to Utils - Removes unrelated autoconfig comments - Utilizes Utils.getCurrentSpanTraceParentHeader - Reports actual HTTP method - Adds autoconfig warning output Signed-off-by: Rodrigo Pastrana <[email protected]>
hpcc-systems · Jun 5, 2024 · 0a0ecb5 · 0a0ecb5
1 parent f763fda
commit 0a0ecb5
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 60 deletions.
diff --git a/wsclient/src/main/java/org/hpccsystems/ws/client/BaseHPCCWsClient.java b/wsclient/src/main/java/org/hpccsystems/ws/client/BaseHPCCWsClient.java
@@ -4,8 +4,6 @@
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
-import java.util.HashMap;
-import java.util.Map;
 
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.ParserConfigurationException;
@@ -52,7 +50,6 @@
 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;
 
@@ -209,23 +206,6 @@ static public void injectCurrentSpanTraceParentHeader(Options options)
         }
     }
 
-    static public String getCurrentSpanTraceParentHeader()
-    {
-        String traceparent = null;
-        Span currentSpan = Span.current();
-        if (currentSpan != null && currentSpan.getSpanContext().isValid())
-        {
-            Map<String, String> carrier = new HashMap<>();
-            TextMapSetter<Map<String, String>> setter = Map::put;
-            W3CTraceContextPropagator.getInstance().inject(Context.current(), carrier, setter);
-
-            traceparent = carrier.getOrDefault("traceparent", "00-" + currentSpan.getSpanContext().getTraceId() + "-" + currentSpan.getSpanContext().getSpanId() + "-00");
-            carrier.clear();
-        }
-
-        return traceparent;
-    }
-
     /**
      * Performs all Otel initialization
      */
@@ -236,15 +216,6 @@ private void initOTel()
          * WARNING: Due to the inherent complications around initialization order involving this classand its single global instance, we strongly recommend *not* using GlobalOpenTelemetry unless youhave a use-case that absolutely requires it. Please favor using instances of OpenTelemetrywherever possible. 
          *  If you are using the OpenTelemetry javaagent, it is generally best to only callGlobalOpenTelemetry.get() once, and then pass the resulting reference where you need to use it.
         */
-
-        //autoconfigured telemetry should be configured via env vars or system attributes:
-        //https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk-extensions/autoconfigure/README.md
-        //For Example:
-        //    -Dotel.traces.exporter=logging
-        //    -Dotel.metrics.exporter=none
-        //    -D.otel.logs.exporter=none
-        //    -Dotel.java.global-autoconfigure.enabled=true
-
         globalOTel = GlobalOpenTelemetry.get();
     }
 

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
@@ -1046,7 +1046,7 @@ public String sendHTTPRequest(String uri, String method) throws Exception
                 .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)
+                .setAttribute(HttpAttributes.HTTP_REQUEST_METHOD, method)
                 .setSpanKind(SpanKind.CLIENT)
                 .startSpan();
 
@@ -1066,17 +1066,7 @@ public String sendHTTPRequest(String uri, String method) throws Exception
 
         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");
-
-                httpURLConnection.setRequestProperty("traceparent", traceparent);
-            }
+            httpURLConnection.setRequestProperty("traceparent", Utils.getCurrentSpanTraceParentHeader());
             httpURLConnection.setRequestMethod(method); //throws ProtocolException
 
             int responseCode = httpURLConnection.getResponseCode(); //throws IOException

diff --git a/wsclient/src/main/java/org/hpccsystems/ws/client/utils/Utils.java b/wsclient/src/main/java/org/hpccsystems/ws/client/utils/Utils.java
@@ -16,7 +16,9 @@
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
@@ -39,6 +41,11 @@
 import org.w3c.dom.NodeList;
 import org.xml.sax.SAXException;
 
+import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator;
+import io.opentelemetry.context.Context;
+import io.opentelemetry.context.propagation.TextMapSetter;
+
 /**
  * Provides multiple functions which support HPCCWsClient actions.
  *
@@ -1121,4 +1128,25 @@ public static String trimTrailing(String originalStr)
 
         return originalStr.substring(0,strIndex+1);
     }
+
+    /**
+     * Returns traceparent value for Open Telemetry based context propagation
+     * @return traceparent of current span if valid, otherwise invalid traceparent header value
+     */
+    static public String getCurrentSpanTraceParentHeader()
+    {
+        String traceparent = null;
+        Span currentSpan = Span.current();
+        if (currentSpan != null && currentSpan.getSpanContext().isValid())
+        {
+            Map<String, String> carrier = new HashMap<>();
+            TextMapSetter<Map<String, String>> setter = Map::put;
+            W3CTraceContextPropagator.getInstance().inject(Context.current(), carrier, setter);
+
+            traceparent = carrier.getOrDefault("traceparent", "00-" + currentSpan.getSpanContext().getTraceId() + "-" + currentSpan.getSpanContext().getSpanId() + "-00");
+            carrier.clear();
+        }
+
+        return traceparent;
+    }
 }
diff --git a/wsclient/src/test/java/org/hpccsystems/ws/client/BaseRemoteTest.java b/wsclient/src/test/java/org/hpccsystems/ws/client/BaseRemoteTest.java
@@ -44,7 +44,6 @@ 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.api.trace.TracerProvider;
 import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
 
 @Category(org.hpccsystems.commons.annotations.RemoteTests.class)
@@ -124,7 +123,9 @@ public static void initialize() throws Exception
     {
         if (Boolean.getBoolean("otel.java.global-autoconfigure.enabled"))
         {
-            System.out.println("OpenTelemetry autoconfiguration enabled:");
+            System.out.println("OpenTelemetry autoconfiguration enabled with following values.");
+            System.out.println("If any of these options are not provided, they will defalt to values which could require additional CLASSPATH dependancies.");
+            System.out.println("If missing dependancies arise, test will halt!");
             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"));
@@ -133,14 +134,8 @@ public static void initialize() throws Exception
             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());
-            }
+
+            globalOTel = AutoConfiguredOpenTelemetrySdk.initialize().getOpenTelemetrySdk();
         }
         else
         {

diff --git a/wsclient/src/test/java/org/hpccsystems/ws/client/platform/test/PlatformTester.java b/wsclient/src/test/java/org/hpccsystems/ws/client/platform/test/PlatformTester.java
@@ -185,7 +185,9 @@ else if (currentParam.matches(WSSQLPORTPATTERN))
             OpenTelemetry globalOTel = null;
             if (Boolean.getBoolean("otel.java.global-autoconfigure.enabled"))
             {
-                System.out.println("OpenTelemetry autoconfiguration enabled:");
+                System.out.println("OpenTelemetry autoconfiguration enabled with following values.");
+                System.out.println("If any of these options are not provided, they will defalt to values which could require additional CLASSPATH dependancies.");
+                System.out.println("If missing dependancies arise, test will halt!");
                 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"));
@@ -194,14 +196,8 @@ else if (currentParam.matches(WSSQLPORTPATTERN))
                 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());
-                }
+
+                globalOTel = AutoConfiguredOpenTelemetrySdk.initialize().getOpenTelemetrySdk();
             }
             else
             {