Closes #1574: [Bug] - Span attributes not added when obfuscator is en…

…abled (#1575)

* fix(tracing): attribute values are converted accordingly to supported types in the IObfuscatory [#1574]

* fix(tracing):: refactor IObfuscatory and implement appropriate tests [#1574]

* fix(tracing): refactor endpoint padding of exporters [#1574]

inspectit-ocelot-config/src/main/java/rocks/inspectit/ocelot/config/model/exporters/metrics/OtlpMetricsExporterSettings.java

@@ -7,6 +7,7 @@
 import rocks.inspectit.ocelot.config.model.exporters.CompressionMethod;
 import rocks.inspectit.ocelot.config.model.exporters.ExporterEnabledState;
 import rocks.inspectit.ocelot.config.model.exporters.TransportProtocol;
+import rocks.inspectit.ocelot.config.utils.EndpointUtils;
 
 import java.time.Duration;
 import java.util.Map;
@@ -48,7 +49,6 @@ public class OtlpMetricsExporterSettings {
      */
     private Map<String, String> headers;
 
-
     /**
      * The compression method.
      */
@@ -59,4 +59,14 @@ public class OtlpMetricsExporterSettings {
      */
     @DurationMin(millis = 1)
     private Duration timeout;
+
+    /**
+     * Gets the OTLP metrics endpoint. The endpoint is padded with 'http' to meet OTEL's requirement that the URI needs to start with 'http://' or 'https://'.
+     * E.g., if you set the endpoint to 'localhost:4317', it will be returned as 'http://localhost:4317'.
+     *
+     * @return
+     */
+    public String getEndpoint() {
+        return EndpointUtils.padEndpoint(endpoint);
+    }
 }

inspectit-ocelot-config/src/main/java/rocks/inspectit/ocelot/config/model/exporters/trace/JaegerExporterSettings.java

@@ -4,6 +4,7 @@
 import lombok.NoArgsConstructor;
 import rocks.inspectit.ocelot.config.model.exporters.ExporterEnabledState;
 import rocks.inspectit.ocelot.config.model.exporters.TransportProtocol;
+import rocks.inspectit.ocelot.config.utils.EndpointUtils;
 
 @Data
 @NoArgsConstructor
@@ -41,4 +42,22 @@ public class JaegerExporterSettings {
      * The service name. Used in {@link rocks.inspectit.oce.eum.server.exporters.configuration.TraceExportersConfiguration}
      */
     private String serviceName;
+
+    /**
+     * Gets the URL endpoint. The endpoint is padded with 'http' to meet OTEL's requirement that the URI needs to start with 'http://' or 'https://'.
+     * E.g., if you set the endpoint to 'localhost:4317', it will be returned as 'http://localhost:4317'.
+     *
+     * @return
+     */
+    public String getEndpoint() {
+        return EndpointUtils.padEndpoint(endpoint);
+    }
+
+    public String getGrpc() {
+        return EndpointUtils.padEndpoint(grpc);
+    }
+
+    public String getUrl() {
+        return EndpointUtils.padEndpoint(url);
+    }
 }

inspectit-ocelot-config/src/main/java/rocks/inspectit/ocelot/config/model/exporters/trace/OtlpTraceExporterSettings.java

@@ -6,6 +6,7 @@
 import rocks.inspectit.ocelot.config.model.exporters.CompressionMethod;
 import rocks.inspectit.ocelot.config.model.exporters.ExporterEnabledState;
 import rocks.inspectit.ocelot.config.model.exporters.TransportProtocol;
+import rocks.inspectit.ocelot.config.utils.EndpointUtils;
 
 import java.time.Duration;
 import java.util.Map;
@@ -45,4 +46,14 @@ public class OtlpTraceExporterSettings {
      */
     @DurationMin(millis = 1)
     private Duration timeout;
+
+    /**
+     * Gets the OTLP traces endpoint. The endpoint is padded with 'http' to meet OTEL's requirement that the URI needs to start with 'http://' or 'https://'.
+     * E.g., if you set the endpoint to 'localhost:4317', it will be returned as 'http://localhost:4317'.
+     *
+     * @return
+     */
+    public String getEndpoint() {
+        return EndpointUtils.padEndpoint(endpoint);
+    }
 }

inspectit-ocelot-config/src/main/java/rocks/inspectit/ocelot/config/model/exporters/trace/ZipkinExporterSettings.java

@@ -3,6 +3,7 @@
 import lombok.Data;
 import lombok.NoArgsConstructor;
 import rocks.inspectit.ocelot.config.model.exporters.ExporterEnabledState;
+import rocks.inspectit.ocelot.config.utils.EndpointUtils;
 
 @Data
 @NoArgsConstructor
@@ -24,4 +25,17 @@ public class ZipkinExporterSettings {
      */
     private String endpoint;
 
+    public String getUrl() {
+        return EndpointUtils.padEndpoint(url);
+    }
+
+    /**
+     * Gets the endpoint of the Zipkin server. The endpoint is padded with 'http' to meet OTEL's requirement that the URI needs to start with 'http://' or 'https://'.
+     * E.g., if you set the endpoint to 'localhost:4317', it will be returned as 'http://localhost:4317'.
+     *
+     * @return
+     */
+    public String getEndpoint() {
+        return EndpointUtils.padEndpoint(endpoint);
+    }
 }

inspectit-ocelot-config/src/main/java/rocks/inspectit/ocelot/config/utils/EndpointUtils.java

@@ -0,0 +1,20 @@
+package rocks.inspectit.ocelot.config.utils;
+
+/**
+ * Utility class for OpenTelemetry endpoints.
+ */
+public class EndpointUtils {
+
+    /**
+     * Transforms the given {@code endpoint} to meet OTEL's requirement to start with either 'http://' or 'https://'.
+     * E.g., the {@code endpoint} 'localhost:4317' will be returned as 'http://localhost:4317'.
+     * If the {@code endpoint} starts with 'http://' or 'https://', it will be returned unchanged.
+     *
+     * @param endpoint
+     *
+     * @return The padded endpoint
+     */
+    public static String padEndpoint(String endpoint) {
+        return endpoint == null || endpoint.isEmpty() || endpoint.startsWith("http") ? endpoint : String.format("http://%s", endpoint);
+    }
+}

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/exporter/JaegerExporterService.java

@@ -65,8 +65,7 @@ protected boolean checkEnabledForConfig(InspectitConfig conf) {
             TransportProtocol exporterProtocol = getProtocol(jaeger);
 
             if (jaeger.getProtocol() != exporterProtocol) {
-                log.warn("The property 'protocol' was not set. Based on the set property '{}' we assume the protocol '{}'. This fallback will be removed in future releases. Please make sure to use the property 'protocol' in future.", hasUrl ? "url" : "grpc", hasUrl ? TransportProtocol.HTTP_THRIFT
-                        .getConfigRepresentation() : TransportProtocol.GRPC.getConfigRepresentation());
+                log.warn("The property 'protocol' was not set. Based on the set property '{}' we assume the protocol '{}'. This fallback will be removed in future releases. Please make sure to use the property 'protocol' in future.", hasUrl ? "url" : "grpc", hasUrl ? TransportProtocol.HTTP_THRIFT.getConfigRepresentation() : TransportProtocol.GRPC.getConfigRepresentation());
             }
 
             if (SUPPORTED_PROTOCOLS.contains(exporterProtocol)) {

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/exporter/OtlpMetricsExporterService.java

@@ -78,6 +78,7 @@ protected boolean doEnable(InspectitConfig configuration) {
         try {
             OtlpMetricsExporterSettings otlp = configuration.getExporters().getMetrics().getOtlp();
             AggregationTemporalitySelector aggregationTemporalitySelector = otlp.getPreferredTemporality() == AggregationTemporality.DELTA ? AggregationTemporalitySelector.deltaPreferred() : AggregationTemporalitySelector.alwaysCumulative();
+
             switch (otlp.getProtocol()) {
                 case GRPC: {
                     OtlpGrpcMetricExporterBuilder metricExporterBuilder = OtlpGrpcMetricExporter.builder()
@@ -112,7 +113,7 @@ protected boolean doEnable(InspectitConfig configuration) {
 
             boolean success = openTelemetryController.registerMetricExporterService(this);
             if (success) {
-                log.info("Starting {}", getName());
+                log.info("Starting {} with protocol {} on endpoint {}", getName(), otlp.getProtocol(), otlp.getEndpoint());
             } else {
                 log.error("Failed to register {} at the OpenTelemetry controller!", getName());
             }

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/exporter/OtlpTraceExporterService.java

@@ -65,10 +65,11 @@ protected boolean doEnable(InspectitConfig configuration) {
 
             switch (otlp.getProtocol()) {
                 case GRPC: {
-                    OtlpGrpcSpanExporterBuilder otlpGrpcSpanExporterBuilder =OtlpGrpcSpanExporter.builder().setEndpoint(otlp.getEndpoint())
+                    OtlpGrpcSpanExporterBuilder otlpGrpcSpanExporterBuilder = OtlpGrpcSpanExporter.builder()
+                            .setEndpoint(otlp.getEndpoint())
                             .setCompression(otlp.getCompression().toString())
                             .setTimeout(otlp.getTimeout());
-                    if(otlp.getHeaders() != null){
+                    if (otlp.getHeaders() != null) {
                         for (Map.Entry<String, String> headerEntry : otlp.getHeaders().entrySet()) {
                             otlpGrpcSpanExporterBuilder.addHeader(headerEntry.getKey(), headerEntry.getValue());
                         }
@@ -77,9 +78,11 @@ protected boolean doEnable(InspectitConfig configuration) {
                     break;
                 }
                 case HTTP_PROTOBUF: {
-                    OtlpHttpSpanExporterBuilder otlpHttpSpanExporterBuilder =OtlpHttpSpanExporter.builder().setEndpoint(otlp.getEndpoint()).setCompression(otlp.getCompression().toString())
+                    OtlpHttpSpanExporterBuilder otlpHttpSpanExporterBuilder = OtlpHttpSpanExporter.builder()
+                            .setEndpoint(otlp.getEndpoint())
+                            .setCompression(otlp.getCompression().toString())
                             .setTimeout(otlp.getTimeout());
-                    if(otlp.getHeaders() != null){
+                    if (otlp.getHeaders() != null) {
                         for (Map.Entry<String, String> headerEntry : otlp.getHeaders().entrySet()) {
                             otlpHttpSpanExporterBuilder.addHeader(headerEntry.getKey(), headerEntry.getValue());
                         }
@@ -91,7 +94,7 @@ protected boolean doEnable(InspectitConfig configuration) {
 
             boolean success = openTelemetryController.registerTraceExporterService(spanExporter, getName());
             if (success) {
-                log.info("Starting OTLP Trace Exporter with endpoint {}", otlp.getEndpoint());
+                log.info("Starting OTLP Trace Exporter with protocol {} on endpoint {}", otlp.getProtocol(), otlp.getEndpoint());
             } else {
                 log.error("Failed to register {} at the OpenTelemetry controller!", getName());
             }

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/hook/VariableAccessorFactory.java

@@ -27,9 +27,10 @@ public class VariableAccessorFactory {
     /**
      * Creates a {@link VariableAccessor} for a given fixed variable.
      * If the variable is a special variable (it starts with an underscore), {@link #getSpecialVariableAccessor(String)} will be returned.
-     * Otherwise a {@link VariableAccessor} is created which performs a lookup of the given variable in the {@link rocks.inspectit.ocelot.bootstrap.exposed.InspectitContext}.
+     * Otherwise, a {@link VariableAccessor} is created which performs a lookup of the given variable in the {@link rocks.inspectit.ocelot.bootstrap.exposed.InspectitContext}.
      *
      * @param variable the name of the variable to create an accessor for
+     *
      * @return the {@link VariableAccessor} for the given variable, never null
      */
     public VariableAccessor getVariableAccessor(String variable) {
@@ -49,6 +50,7 @@ public VariableAccessor getVariableAccessor(String variable) {
      * Creates a {@link VariableAccessor} which always returns the given value.
      *
      * @param value the value to return
+     *
      * @return an accessor returning the given value.
      */
     public VariableAccessor getConstantAccessor(Object value) {
@@ -59,6 +61,7 @@ public VariableAccessor getConstantAccessor(Object value) {
      * Creates a {@link VariableAccessor} for the given special variable.
      *
      * @param variable the name of the special variable
+     *
      * @return the {@link VariableAccessor} for the given variable or null if "variable" does not denote a special variable
      */
     public VariableAccessor getSpecialVariableAccessor(String variable) {
@@ -97,14 +100,15 @@ public VariableAccessor getSpecialVariableAccessor(String variable) {
      * Given the name of a special variable, this method returns it's value if it is constant.
      * "Constant" hereby means that the special variable is not runtime-dependent.
      * For example, for a given method hook the values "_methodName" and "_attachments" do never change and therefore are "constant".
-     * In contrast "_arg0" for example is dependendent on the current context and therefore NOT constant.
+     * In contrast, "_arg0" for example depends on the current context and therefore NOT constant.
      * <p>
-     * It is allowed to store references to these constant variables within the inspectit classloader.
+     * It is allowed to store references to these constant variables within the inspectIT classloader.
      * This is guaranteed to not cause a memory leak.
      * For this reason "_class" is not a constant special variable, as its storage can cause a memory leak.
      *
      * @param variable      the name of the special variable
      * @param contextMethod the method for which the constant value is being derived
+     *
      * @return the value of the special variable if it is a constant, null otherwise
      */
     public Object getConstantSpecialVariable(String variable, MethodReflectionInformation contextMethod) {

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/privacy/obfuscation/IObfuscatory.java

@@ -16,19 +16,41 @@ public interface IObfuscatory {
      */
     default void putSpanAttribute(Span span, String key, Object value) {
         AttributeKey attributeKey;
+        Object resultValue = value;
+
+        // set the AttributeKey and, if applicable, the result value
+
+        // String as String
         if (value instanceof String) {
             attributeKey = AttributeKey.stringKey(key);
-        } else if (value instanceof Double || value instanceof Float) {
+        }
+        // Double and Float as Double
+        else if (value instanceof Double || value instanceof Float) {
             attributeKey = AttributeKey.doubleKey(key);
-        } else if (value instanceof Number) {
+            if (value instanceof Float) {
+                // as we do not want the added precision, we need to use the String representation instead of the floatValue()
+                resultValue = Double.valueOf(value.toString());
+            }
+        }
+        // Long, Integer and Short as Long
+        else if (value instanceof Long || value instanceof Integer || value instanceof Short) {
             attributeKey = AttributeKey.longKey(key);
+            resultValue = ((Number) value).longValue();
+        }
+        // treat all other Number as double
+        else if (value instanceof Number) {
+            attributeKey = AttributeKey.doubleKey(key);
+            resultValue = ((Number) value).doubleValue();
         } else if (value instanceof Boolean) {
             attributeKey = AttributeKey.booleanKey(key);
-        } else {
+        }
+        // everything else (e.g., Exceptions) will be converted to String
+        else {
             attributeKey = AttributeKey.stringKey(key);
+            resultValue = value.toString();
         }
 
-        span.setAttribute(attributeKey, value);
+        span.setAttribute(attributeKey, resultValue);
     }
 
 }

inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/exporter/ExporterServiceIntegrationTestBase.java

@@ -39,6 +39,7 @@
 import java.io.UncheckedIOException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.CompletionStage;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
@@ -278,16 +279,25 @@ void awaitSpansExported(String parentSpanName, String childSpanName) {
                                     .map(ils -> ils.getSpansList())));
 
             // assert that parent and child span are present and that the parent's spanId equals the child's parentSpanId
-            assertThat(spansLis.anyMatch(s -> s.stream()
-                    .filter(span -> span.getName().equals(childSpanName))
-                    .findFirst()
-                    .orElse(io.opentelemetry.proto.trace.v1.Span.getDefaultInstance())
-                    .getParentSpanId()
-                    .equals(s.stream()
-                            .filter(span -> span.getName().equals(parentSpanName))
-                            .findFirst()
-                            .orElse(io.opentelemetry.proto.trace.v1.Span.getDefaultInstance())
-                            .getSpanId()))).isTrue();
+            assertThat(spansLis.anyMatch(s -> {
+                Optional<io.opentelemetry.proto.trace.v1.Span> childSpan = s.stream()
+                        .filter(span -> span.getName().equals(childSpanName))
+                        .findFirst();
+                if (!childSpan.isPresent()) {
+                    return false;
+                }
+
+                Optional<io.opentelemetry.proto.trace.v1.Span> parentSpan = s.stream()
+                        .filter(span -> span.getName().equals(parentSpanName))
+                        .findFirst();
+                if (!parentSpan.isPresent()) {
+                    return false;
+                }
+
+                return childSpan.get().getParentSpanId().equals(parentSpan.get().getSpanId());
+
+            })).isTrue();
+
         });
 
     }