almost ready

sdk/clientcore/core/pom.xml

@@ -64,6 +64,7 @@
 
       --add-exports io.clientcore.core/io.clientcore.core.shared=ALL-UNNAMED
       --add-exports io.clientcore.core/io.clientcore.core.implementation=ALL-UNNAMED
+      --add-exports io.clientcore.core/io.clientcore.core.implementation.instrumentation.fallback=ALL-UNNAMED
     </javaModulesSurefireArgLine>
 
     <!-- If JMH benchmarking was ran Checkstyle may fail on jmh_benchmark generated classes, ignore them. -->

sdk/clientcore/core/spotbugs-exclude.xml

@@ -122,8 +122,8 @@
       <Class name="io.clientcore.core.serialization.xml.XmlReader" />
       <Class name="io.clientcore.core.shared.HttpClientTests" />
       <Class name="io.clientcore.core.shared.HttpClientTestsServer" />
-      <Class name="io.clientcore.core.util.ClientLoggerTests" />
-      <Class name="io.clientcore.core.util.HttpLoggingPolicyTests" />
+      <Class name="io.clientcore.core.instrumentation.logging.ClientLoggerTests" />
+      <Class name="io.clientcore.core.instrumentation.logging.InstrumentationTestUtils" />
       <Class name="io.clientcore.core.util.binarydata.BinaryDataTest" />
       <Class name="io.clientcore.core.util.serializer.JsonSerializerTests" />
     </Or>
@@ -168,7 +168,7 @@
     <Or>
       <Class name="io.clientcore.core.implementation.instrumentation.DefaultLogger" />
       <Class name="io.clientcore.core.serialization.xml.implementation.aalto.UncheckedStreamException" />
-      <Class name="io.clientcore.core.util.ClientLoggerTests" />
+      <Class name="io.clientcore.core.instrumentation.logging.ClientLoggerTests" />
     </Or>
   </Match>
   <Match>
@@ -238,13 +238,14 @@
       <Class name="io.clientcore.core.implementation.http.rest.LengthValidatingInputStreamTests" />
       <Class name="io.clientcore.core.serialization.json.implementation.StringBuilderWriterTests" />
       <Class name="io.clientcore.core.util.binarydata.BinaryDataTest" />
+      <Class name="io.clientcore.core.implementation.instrumentation.fallback.FallbackInstrumentationTests" />
     </Or>
   </Match>
   <Match>
     <Bug pattern="OS_OPEN_STREAM" />
     <Or>
       <Class name="io.clientcore.core.serialization.json.implementation.StringBuilderWriterTests" />
-      <Class name="io.clientcore.core.util.HttpLoggingPolicyTests" />
+      <Class name="io.clientcore.core.http.pipeline.HttpInstrumentationPolicyLoggingTests" />
     </Or>
   </Match>
   <Match>
@@ -260,6 +261,7 @@
       <Class name="io.clientcore.core.http.pipeline.HttpRetryPolicy" />
       <Class name="io.clientcore.core.shared.HttpClientTests" />
       <Class name="io.clientcore.core.shared.HttpClientTestsServer" />
+      <Class name="io.clientcore.core.implementation.instrumentation.fallback.RandomIdUtils" />
     </Or>
   </Match>
   <Match>
@@ -400,4 +402,16 @@
     <Bug pattern="XXE_XMLSTREAMREADER" />
     <Class name="io.clientcore.core.serialization.xml.XmlReader" />
   </Match>
+  <Match>
+    <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE" />
+    <Class name="io.clientcore.core.http.pipeline.HttpInstrumentationPolicy" />
+  </Match>
+  <Match>
+    <Bug pattern="NP_LOAD_OF_KNOWN_NULL_VALUE" />
+    <Class name="io.clientcore.core.http.pipeline.HttpInstrumentationPolicy" />
+  </Match>
+  <Match>
+    <Bug pattern="AA_ASSERTION_OF_ARGUMENTS" />
+    <Class name="io.clientcore.core.implementation.instrumentation.otel.tracing.OTelSpanContext" />
+  </Match>
 </FindBugsFilter>

sdk/clientcore/core/src/main/java/io/clientcore/core/http/pipeline/HttpInstrumentationPolicy.java

@@ -13,7 +13,6 @@
 import io.clientcore.core.http.models.Response;
 import io.clientcore.core.implementation.http.HttpRequestAccessHelper;
 import io.clientcore.core.implementation.instrumentation.LibraryInstrumentationOptionsAccessHelper;
-import io.clientcore.core.implementation.util.LoggingKeys;
 import io.clientcore.core.instrumentation.Instrumentation;
 import io.clientcore.core.instrumentation.InstrumentationContext;
 import io.clientcore.core.instrumentation.LibraryInstrumentationOptions;
@@ -39,7 +38,6 @@
 import java.util.stream.Collectors;
 import java.net.URI;
 
-import static io.clientcore.core.http.models.HttpHeaderName.TRACEPARENT;
 import static io.clientcore.core.implementation.UrlRedactionUtil.getRedactedUri;
 import static io.clientcore.core.implementation.instrumentation.AttributeKeys.HTTP_REQUEST_BODY_KEY;
 import static io.clientcore.core.implementation.instrumentation.AttributeKeys.HTTP_REQUEST_BODY_SIZE_KEY;
@@ -109,9 +107,11 @@
  * <pre>
  *
  * HttpPipelinePolicy enrichingPolicy = &#40;request, next&#41; -&gt; &#123;
- *     Object span = request.getRequestOptions&#40;&#41;.getContext&#40;&#41;.get&#40;TRACE_CONTEXT_KEY&#41;;
- *     if &#40;span instanceof Span&#41; &#123;
- *         &#40;&#40;Span&#41;span&#41;.setAttribute&#40;&quot;custom.request.id&quot;, request.getHeaders&#40;&#41;.getValue&#40;CUSTOM_REQUEST_ID&#41;&#41;;
+ *     Span span = request.getRequestOptions&#40;&#41; == null
+ *         ? Span.noop&#40;&#41;
+ *         : request.getRequestOptions&#40;&#41;.getInstrumentationContext&#40;&#41;.getSpan&#40;&#41;;
+ *     if &#40;span.isRecording&#40;&#41;&#41; &#123;
+ *         span.setAttribute&#40;&quot;custom.request.id&quot;, request.getHeaders&#40;&#41;.getValue&#40;CUSTOM_REQUEST_ID&#41;&#41;;
  *     &#125;
  *
  *     return next.process&#40;&#41;;
@@ -131,7 +131,6 @@
  */
 public final class HttpInstrumentationPolicy implements HttpPipelinePolicy {
     private static final ClientLogger LOGGER = new ClientLogger(HttpInstrumentationPolicy.class);
-    private static final Set<HttpHeaderName> ALWAYS_ALLOWED_HEADERS = Set.of(TRACEPARENT);
     private static final HttpLogOptions DEFAULT_HTTP_LOG_OPTIONS = new HttpLogOptions();
     private static final String LIBRARY_NAME;
     private static final String LIBRARY_VERSION;
@@ -210,14 +209,17 @@ public Response<?> process(HttpRequest request, HttpPipelineNextPolicy next) {
         int tryCount = HttpRequestAccessHelper.getTryCount(request);
         final long requestContentLength = getContentLength(logger, request.getBody(), request.getHeaders());
 
+        InstrumentationContext context
+            = request.getRequestOptions() == null ? null : request.getRequestOptions().getInstrumentationContext();
         Span span = Span.noop();
         if (isTracingEnabled(request)) {
-            span = startHttpSpan(request, redactedUrl);
-            request.getRequestOptions().setInstrumentationContext(span.getInstrumentationContext());
+            span = startHttpSpan(request, redactedUrl, context);
+            context = span.getInstrumentationContext();
+            request.getRequestOptions().setInstrumentationContext(context);
             traceContextPropagator.inject(span.getInstrumentationContext(), request.getHeaders(), SETTER);
         }
 
-        logRequest(logger, request, startNs, requestContentLength, redactedUrl, tryCount, span.getInstrumentationContext());
+        logRequest(logger, request, startNs, requestContentLength, redactedUrl, tryCount, context);
 
         try (TracingScope scope = span.makeCurrent()) {
             Response<?> response = next.process();
@@ -227,34 +229,33 @@ public Response<?> process(HttpRequest request, HttpPipelineNextPolicy next) {
                     .setContext(span.getInstrumentationContext())
                     .addKeyValue(HTTP_REQUEST_METHOD_KEY, request.getHttpMethod())
                     .addKeyValue(URL_FULL_KEY, redactedUrl)
-                    .log("HTTP response is null and no exception is thrown. Please report it to the client library maintainers.");
+                    .log(
+                        "HTTP response is null and no exception is thrown. Please report it to the client library maintainers.");
 
                 return null;
             }
 
             addDetails(request, response, tryCount, span);
-            response =  logResponse(logger, response, startNs, requestContentLength, redactedUrl, tryCount, span.getInstrumentationContext());
+            response = logResponse(logger, response, startNs, requestContentLength, redactedUrl, tryCount, context);
             span.end();
             return response;
         } catch (RuntimeException t) {
-            var ex = logException(logger, request, null, t, startNs, null, requestContentLength, redactedUrl, tryCount, span.getInstrumentationContext());
+            var ex = logException(logger, request, null, t, startNs, null, requestContentLength, redactedUrl, tryCount,
+                context);
             span.end(unwrap(t));
             throw ex;
         }
     }
 
-    private Span startHttpSpan(HttpRequest request, String sanitizedUrl) {
+    private Span startHttpSpan(HttpRequest request, String sanitizedUrl, InstrumentationContext context) {
         if (request.getRequestOptions() == null || request.getRequestOptions() == RequestOptions.none()) {
             request.setRequestOptions(new RequestOptions());
         }
 
-        InstrumentationContext context = request.getRequestOptions().getInstrumentationContext();
-
-        SpanBuilder spanBuilder
-            = tracer.spanBuilder(request.getHttpMethod().toString(), CLIENT, context)
-                .setAttribute(HTTP_REQUEST_METHOD_KEY, request.getHttpMethod().toString())
-                .setAttribute(URL_FULL_KEY, sanitizedUrl)
-                .setAttribute(SERVER_ADDRESS, request.getUri().getHost());
+        SpanBuilder spanBuilder = tracer.spanBuilder(request.getHttpMethod().toString(), CLIENT, context)
+            .setAttribute(HTTP_REQUEST_METHOD_KEY, request.getHttpMethod().toString())
+            .setAttribute(URL_FULL_KEY, sanitizedUrl)
+            .setAttribute(SERVER_ADDRESS, request.getUri().getHost());
         maybeSetServerPort(spanBuilder, request.getUri());
         return spanBuilder.startSpan();
     }
@@ -337,7 +338,7 @@ private static Throwable unwrap(Throwable t) {
     private ClientLogger getLogger(HttpRequest httpRequest) {
         ClientLogger logger = null;
 
-        if (httpRequest.getRequestOptions() != null) {
+        if (httpRequest.getRequestOptions() != null && httpRequest.getRequestOptions().getLogger() != null) {
             logger = httpRequest.getRequestOptions().getLogger();
         }
 
@@ -370,8 +371,7 @@ private void logRequest(ClientLogger logger, HttpRequest request, long startNano
             return;
         }
 
-        logBuilder
-            .setEventName(HTTP_REQUEST_EVENT_NAME)
+        logBuilder.setEventName(HTTP_REQUEST_EVENT_NAME)
             .setContext(context)
             .addKeyValue(HTTP_REQUEST_METHOD_KEY, request.getHttpMethod())
             .addKeyValue(URL_FULL_KEY, redactedUrl)
@@ -503,13 +503,6 @@ private void addHeadersToLogMessage(HttpHeaders headers, ClientLogger.LoggingEve
                 String headerValue = allowedHeaderNames.contains(headerName) ? header.getValue() : REDACTED_PLACEHOLDER;
                 logBuilder.addKeyValue(headerName.toString(), headerValue);
             }
-        } else {
-            for (HttpHeaderName headerName : ALWAYS_ALLOWED_HEADERS) {
-                String headerValue = headers.getValue(headerName);
-                if (headerValue != null) {
-                    logBuilder.addKeyValue(headerName.toString(), headerValue);
-                }
-            }
         }
     }
 
@@ -543,7 +536,7 @@ private static long getContentLength(ClientLogger logger, BinaryData body, HttpH
 
         try {
             contentLength = Long.parseLong(contentLengthString);
-        } catch (NumberFormatException | NullPointerException e) {
+        } catch (NumberFormatException e) {
             logger.atVerbose()
                 .addKeyValue("contentLength", contentLengthString)
                 .log("Could not parse the HTTP header content-length", e);
@@ -559,7 +552,7 @@ private static final class LoggingHttpResponse<T> extends HttpResponse<T> {
         private BinaryData bufferedBody;
 
         private LoggingHttpResponse(Response<T> actualResponse, Consumer<BinaryData> onContent,
-                                    Consumer<Throwable> onException) {
+            Consumer<Throwable> onException) {
             super(actualResponse.getRequest(), actualResponse.getStatusCode(), actualResponse.getHeaders(),
                 actualResponse.getValue());
 

sdk/clientcore/core/src/main/java/io/clientcore/core/http/pipeline/HttpRedirectPolicy.java

@@ -9,8 +9,8 @@
 import io.clientcore.core.http.models.HttpRequest;
 import io.clientcore.core.http.models.Response;
 import io.clientcore.core.implementation.instrumentation.AttributeKeys;
+import io.clientcore.core.instrumentation.InstrumentationContext;
 import io.clientcore.core.instrumentation.logging.ClientLogger;
-import io.clientcore.core.util.Context;
 
 import java.io.IOException;
 import java.io.UncheckedIOException;
@@ -78,50 +78,54 @@ public HttpRedirectPolicy(HttpRedirectOptions redirectOptions) {
     @Override
     public Response<?> process(HttpRequest httpRequest, HttpPipelineNextPolicy next) {
         // Reset the attemptedRedirectUris for each individual request.
-        return attemptRedirect(next, 1, new LinkedHashSet<>(), httpRequest.getRequestOptions().getContext());
+        InstrumentationContext instrumentationContext = httpRequest.getRequestOptions() == null
+            ? null
+            : httpRequest.getRequestOptions().getInstrumentationContext();
+        return attemptRedirect(next, 1, new LinkedHashSet<>(), instrumentationContext);
     }
 
     /**
      * Function to process through the HTTP Response received in the pipeline and redirect sending the request with a
      * new redirect URI.
      */
     private Response<?> attemptRedirect(final HttpPipelineNextPolicy next, final int redirectAttempt,
-        LinkedHashSet<String> attemptedRedirectUris, Context context) {
+        LinkedHashSet<String> attemptedRedirectUris, InstrumentationContext instrumentationContext) {
 
         // Make sure the context is not modified during redirect, except for the URI
         Response<?> response = next.clone().process();
 
         HttpRequestRedirectCondition requestRedirectCondition
             = new HttpRequestRedirectCondition(response, redirectAttempt, attemptedRedirectUris);
         if ((shouldRedirectCondition != null && shouldRedirectCondition.test(requestRedirectCondition))
-            || (shouldRedirectCondition == null && defaultShouldAttemptRedirect(requestRedirectCondition, context))) {
+            || (shouldRedirectCondition == null
+                && defaultShouldAttemptRedirect(requestRedirectCondition, instrumentationContext))) {
             createRedirectRequest(response);
-            return attemptRedirect(next, redirectAttempt + 1, attemptedRedirectUris, context);
+            return attemptRedirect(next, redirectAttempt + 1, attemptedRedirectUris, instrumentationContext);
         }
 
         return response;
     }
 
     private boolean defaultShouldAttemptRedirect(HttpRequestRedirectCondition requestRedirectCondition,
-        Context context) {
+        InstrumentationContext instrumentationContext) {
         Response<?> response = requestRedirectCondition.getResponse();
         int tryCount = requestRedirectCondition.getTryCount();
         Set<String> attemptedRedirectUris = requestRedirectCondition.getRedirectedUris();
         String redirectUri = response.getHeaders().getValue(this.locationHeader);
 
         if (isValidRedirectStatusCode(response.getStatusCode())
-            && isValidRedirectCount(tryCount, context)
-            && isAllowedRedirectMethod(response.getRequest().getHttpMethod(), context)
+            && isValidRedirectCount(tryCount, instrumentationContext)
+            && isAllowedRedirectMethod(response.getRequest().getHttpMethod(), instrumentationContext)
             && redirectUri != null
-            && !alreadyAttemptedRedirectUri(redirectUri, attemptedRedirectUris, context)) {
+            && !alreadyAttemptedRedirectUri(redirectUri, attemptedRedirectUris, instrumentationContext)) {
 
             LOGGER.atVerbose()
                 .addKeyValue(HTTP_REQUEST_RESEND_COUNT_KEY, tryCount - 1)
                 .addKeyValue("url.full.from", response.getRequest().getUri())
                 .addKeyValue("url.full.to", redirectUri)
                 .addKeyValue("url.full.all", attemptedRedirectUris::toString)
                 .setEventName("http.request.redirect")
-                //.setContext(context)
+                .setContext(instrumentationContext)
                 .log();
 
             attemptedRedirectUris.add(redirectUri);
@@ -139,11 +143,11 @@ && isAllowedRedirectMethod(response.getRequest().getHttpMethod(), context)
      *
      * @return {@code true} if the {@code tryCount} is greater than the {@code maxAttempts}, {@code false} otherwise.
      */
-    private boolean isValidRedirectCount(int tryCount, Context context) {
+    private boolean isValidRedirectCount(int tryCount, InstrumentationContext instrumentationContext) {
         if (tryCount >= this.maxAttempts) {
             LOGGER.atError()
                 .addKeyValue("maxAttempts", this.maxAttempts)
-                //.setContext(context)
+                .setContext(instrumentationContext)
                 .log("Redirect attempts have been exhausted.");
 
             return false;
@@ -162,11 +166,11 @@ private boolean isValidRedirectCount(int tryCount, Context context) {
      * {@code false} otherwise.
      */
     private boolean alreadyAttemptedRedirectUri(String redirectUri, Set<String> attemptedRedirectUris,
-        Context context) {
+        InstrumentationContext instrumentationContext) {
         if (attemptedRedirectUris.contains(redirectUri)) {
             LOGGER.atError()
                 .addKeyValue(URL_FULL_KEY, redirectUri)
-                //.setContext(context)
+                .setContext(instrumentationContext)
                 .log("Request was redirected more than once to the same URI.");
 
             return true;
@@ -182,12 +186,12 @@ private boolean alreadyAttemptedRedirectUri(String redirectUri, Set<String> atte
      *
      * @return {@code true} if the request {@code httpMethod} is a valid http redirect method, {@code false} otherwise.
      */
-    private boolean isAllowedRedirectMethod(HttpMethod httpMethod, Context context) {
+    private boolean isAllowedRedirectMethod(HttpMethod httpMethod, InstrumentationContext instrumentationContext) {
         if (allowedRedirectHttpMethods.contains(httpMethod)) {
             return true;
         } else {
             LOGGER.atError()
-                //.setContext(context)
+                .setContext(instrumentationContext)
                 .addKeyValue(AttributeKeys.HTTP_REQUEST_METHOD_KEY, httpMethod)
                 .log("Request redirection is not enabled for this HTTP method.");