cleanup and code snippets

sdk/clientcore/core/pom.xml

@@ -88,7 +88,7 @@
     <dependency>
       <groupId>io.opentelemetry</groupId>
       <artifactId>opentelemetry-sdk</artifactId>
-      <version>1.43.0</version>
+      <version>1.43.0</version> <!-- {x-version-update;io.opentelemetry:opentelemetry-sdk;external_dependency} -->
       <scope>test</scope>
     </dependency>
     <dependency>
@@ -97,6 +97,12 @@
       <version>1.43.0</version> <!-- {x-version-update;io.opentelemetry:opentelemetry-sdk-testing;external_dependency} -->
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>io.opentelemetry</groupId>
+      <artifactId>opentelemetry-sdk-extension-autoconfigure</artifactId>
+      <version>1.43.0</version> <!-- {x-version-update;io.opentelemetry:-sdk-extension-autoconfigure;external_dependency} -->
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.eclipse.jetty</groupId>
       <artifactId>jetty-server</artifactId>

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

@@ -12,10 +12,10 @@
 import io.clientcore.core.telemetry.LibraryTelemetryOptions;
 import io.clientcore.core.telemetry.TelemetryOptions;
 import io.clientcore.core.telemetry.TelemetryProvider;
-import io.clientcore.core.telemetry.Scope;
 import io.clientcore.core.telemetry.tracing.Span;
 import io.clientcore.core.telemetry.tracing.SpanContext;
 import io.clientcore.core.telemetry.tracing.Tracer;
+import io.clientcore.core.telemetry.tracing.TracingScope;
 import io.clientcore.core.util.ClientLogger;
 import io.clientcore.core.util.Context;
 
@@ -32,8 +32,61 @@
 import static io.clientcore.core.telemetry.tracing.SpanKind.CLIENT;
 
 /**
- * The instrumentation policy is responsible for instrumenting the HTTP request and response with distributed tracing
- * and (in the future) metrics.
+ * The {@link InstrumentationPolicy} is responsible for instrumenting the HTTP request and response with distributed tracing
+ * and (in the future) metrics following
+ * <a href="https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md">OpenTelemetry Semantic Conventions</a>.
+ * <p>
+ * It propagates context to the downstream service following <a href="https://www.w3.org/TR/trace-context-1/">W3C Trace Context</a> specification.
+ * <p>
+ *
+ * The {@link InstrumentationPolicy} should be added to the HTTP pipeline by client libraries. It should be added between
+ * {@link HttpRetryPolicy} and {@link HttpLoggingPolicy} so that it's executed on each try (redirect), and logging happens
+ * in the scope of the span.
+ * <p>
+ * The policy supports basic customizations using {@link TelemetryOptions}, {@link HttpLogOptions}, and additional request and
+ * response headers to record as attributes.
+ * <p>
+ * If your client library needs a different approach to distributed tracing,
+ * you can create a custom policy and use it instead of the {@link InstrumentationPolicy}. If you want to enrich instrumentation
+ * policy spans with additional attributes, you can create a custom policy and add it under the {@link InstrumentationPolicy}
+ * so that it's executed in the scope of the span created by the {@link InstrumentationPolicy}.
+ * <p>
+ * <strong>Configure instrumentation policy:</strong>
+ * <p>
+ * <!-- src_embed io.clientcore.core.telemetry.tracing.instrumentationpolicy -->
+ * <pre>
+ *
+ * HttpPipeline pipeline = new HttpPipelineBuilder&#40;&#41;
+ *     .policies&#40;
+ *         new HttpRetryPolicy&#40;&#41;,
+ *         new InstrumentationPolicy&#40;telemetryOptions, logOptions&#41;,
+ *         new HttpLoggingPolicy&#40;logOptions&#41;&#41;
+ *     .build&#40;&#41;;
+ *
+ * </pre>
+ * <!-- end io.clientcore.core.telemetry.tracing.instrumentationpolicy -->
+ * <p>
+ * <strong>Customize instrumentation policy:</strong>
+ * <p>
+ * <!-- src_embed io.clientcore.core.telemetry.tracing.customizeinstrumentationpolicy -->
+ * <pre>
+ *
+ * &#47;&#47; InstrumentationPolicy can capture custom headers from requests and responses - for example when the endpoint
+ * &#47;&#47; supports legacy correlation headers.
+ * Map&lt;HttpHeaderName, String&gt; requestHeadersToRecord
+ *     = Collections.singletonMap&#40;HttpHeaderName.CLIENT_REQUEST_ID, &quot;custom.request.id&quot;&#41;;
+ * Map&lt;HttpHeaderName, String&gt; responseHeadersToRecord
+ *     = Collections.singletonMap&#40;HttpHeaderName.REQUEST_ID, &quot;custom.response.id&quot;&#41;;
+ *
+ * HttpPipeline pipeline = new HttpPipelineBuilder&#40;&#41;
+ *     .policies&#40;
+ *         new HttpRetryPolicy&#40;&#41;,
+ *         new InstrumentationPolicy&#40;telemetryOptions, logOptions, requestHeadersToRecord, responseHeadersToRecord&#41;,
+ *         new HttpLoggingPolicy&#40;logOptions&#41;&#41;
+ *     .build&#40;&#41;;
+ *
+ * </pre>
+ * <!-- end io.clientcore.core.telemetry.tracing.customizeinstrumentationpolicy -->
  */
 public class InstrumentationPolicy implements HttpPipelinePolicy {
     private static final ClientLogger LOGGER = new ClientLogger(InstrumentationPolicy.class);
@@ -72,7 +125,6 @@ public class InstrumentationPolicy implements HttpPipelinePolicy {
 
     /**
      * Creates a new instrumentation policy.
-     *
      * @param telemetryOptions Application telemetry options.
      * @param logOptions Http log options. TODO: we should merge this with telemetry options.
      */
@@ -81,12 +133,12 @@ public InstrumentationPolicy(TelemetryOptions<?> telemetryOptions, HttpLogOption
     }
 
     /**
-     * Creates a new instrumentation policy with the ability to capture request and response headers.
+     * Creates a new instrumentation policy with additional request and response headers to record as attributes.
      *
      * @param telemetryOptions Application telemetry options.
      * @param logOptions Http log options. TODO: we should merge this with telemetry options.
-     * @param requestHeaders The request headers to capture.
-     * @param responseHeaders The response headers to capture.
+     * @param requestHeaders The request headers to capture as span attributes.
+     * @param responseHeaders The response headers to capture as span attributes.
      */
     public InstrumentationPolicy(TelemetryOptions<?> telemetryOptions, HttpLogOptions logOptions,
         Map<HttpHeaderName, String> requestHeaders, Map<HttpHeaderName, String> responseHeaders) {
@@ -114,7 +166,7 @@ public Response<?> process(HttpRequest request, HttpPipelineNextPolicy next) {
             .setAttribute(SERVER_PORT, request.getUri().getPort() == -1 ? 443 : request.getUri().getPort())
             .startSpan();
 
-        try (Scope scope = span.makeCurrent()) {
+        try (TracingScope scope = span.makeCurrent()) {
             propagateContext(request, span.getSpanContext());
             Response<?> response = next.process();
 
@@ -141,12 +193,11 @@ public Response<?> process(HttpRequest request, HttpPipelineNextPolicy next) {
                 span.setError(String.valueOf(response.getStatusCode()));
             }
 
+            span.end();
             return response;
         } catch (Throwable t) {
-            span.setError(unwrap(t));
+            span.end(unwrap(t));
             throw t;
-        } finally {
-            span.end();
         }
     }
 

sdk/clientcore/core/src/main/java/io/clientcore/core/implementation/telemetry/otel/tracing/OTelSpan.java

@@ -5,15 +5,16 @@
 
 import io.clientcore.core.implementation.telemetry.otel.OTelAttributeKey;
 import io.clientcore.core.implementation.telemetry.otel.OTelInitializer;
-import io.clientcore.core.telemetry.Scope;
 import io.clientcore.core.telemetry.tracing.Span;
 import io.clientcore.core.telemetry.tracing.SpanContext;
 import io.clientcore.core.telemetry.tracing.SpanKind;
+import io.clientcore.core.telemetry.tracing.TracingScope;
 import io.clientcore.core.util.ClientLogger;
 
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
+import java.util.Objects;
 
 import static io.clientcore.core.implementation.telemetry.otel.OTelInitializer.ATTRIBUTE_KEY_CLASS;
 import static io.clientcore.core.implementation.telemetry.otel.OTelInitializer.CONTEXT_CLASS;
@@ -25,7 +26,7 @@
 public class OTelSpan implements Span {
     private static final MethodHandles.Lookup LOOKUP = MethodHandles.publicLookup();
     private static final ClientLogger LOGGER = new ClientLogger(OTelSpan.class);
-    private static final Scope NOOP_SCOPE = () -> {
+    private static final TracingScope NOOP_SCOPE = () -> {
     };
     private static final MethodHandle SET_ATTRIBUTE_INVOKER;
     private static final MethodHandle SET_STATUS_INVOKER;
@@ -38,6 +39,8 @@ public class OTelSpan implements Span {
     private static final Object ERROR_STATUS_CODE;
     private final Object otelSpan;
     private final Object otelContext;
+    private String errorType;
+    private boolean isRecording;
 
     static {
         MethodHandle setAttributeInvoker = null;
@@ -92,13 +95,14 @@ public class OTelSpan implements Span {
         assert otelSpan == null || SPAN_CLASS.isInstance(otelSpan);
 
         this.otelSpan = otelSpan;
+        this.isRecording = otelSpan != null && (boolean) IS_RECORDING_INVOKER.invoke(otelSpan);
 
         Object contextWithSpan = otelSpan != null ? storeInContext(otelSpan, otelParentContext) : otelParentContext;
         this.otelContext = markCoreSpan(contextWithSpan, spanKind);
     }
 
     public OTelSpan setAttribute(String key, Object value) {
-        if (OTelInitializer.isInitialized() && otelSpan != null) {
+        if (OTelInitializer.isInitialized() && otelSpan != null && isRecording) {
             try {
                 SET_ATTRIBUTE_INVOKER.invoke(otelSpan, OTelAttributeKey.getKey(key, value),
                     OTelAttributeKey.castAttributeValue(value));
@@ -112,36 +116,30 @@ public OTelSpan setAttribute(String key, Object value) {
 
     @Override
     public Span setError(String errorType) {
-        setAttribute("error.type", errorType);
-        if (OTelInitializer.isInitialized() && otelSpan != null) {
-            try {
-                SET_STATUS_INVOKER.invoke(otelSpan, ERROR_STATUS_CODE, null);
-            } catch (Throwable t) {
-                OTelInitializer.runtimeError(LOGGER, t);
-            }
-        }
-
+        this.errorType = errorType;
         return this;
     }
 
     @Override
-    public Span setError(Throwable error) {
-        setAttribute("error.type", error.getClass().getCanonicalName());
-        if (OTelInitializer.isInitialized() && otelSpan != null) {
-            try {
-                SET_STATUS_INVOKER.invoke(otelSpan, ERROR_STATUS_CODE, error.getMessage());
-            } catch (Throwable t) {
-                OTelInitializer.runtimeError(LOGGER, t);
-            }
-        }
-
-        return this;
+    public void end(Throwable throwable) {
+        Objects.requireNonNull(throwable, "'throwable' cannot be null");
+        endSpan(throwable);
     }
 
     @Override
     public void end() {
+        endSpan(null);
+    }
+
+    private void endSpan(Throwable throwable) {
         if (OTelInitializer.isInitialized() && otelSpan != null) {
             try {
+                if (errorType != null || throwable != null) {
+                    setAttribute("error.type", errorType != null ? errorType : throwable.getClass().getCanonicalName());
+                    SET_STATUS_INVOKER.invoke(otelSpan, ERROR_STATUS_CODE,
+                        throwable == null ? null : throwable.getMessage());
+                }
+
                 END_INVOKER.invoke(otelSpan);
             } catch (Throwable t) {
                 OTelInitializer.runtimeError(LOGGER, t);
@@ -164,19 +162,11 @@ public SpanContext getSpanContext() {
 
     @Override
     public boolean isRecording() {
-        if (OTelInitializer.isInitialized() && otelSpan != null) {
-            try {
-                return (boolean) IS_RECORDING_INVOKER.invoke(otelSpan);
-            } catch (Throwable t) {
-                OTelInitializer.runtimeError(LOGGER, t);
-            }
-        }
-
-        return false;
+        return isRecording;
     }
 
     @Override
-    public Scope makeCurrent() {
+    public TracingScope makeCurrent() {
         if (OTelInitializer.isInitialized() && otelSpan != null) {
             try {
                 return wrapOTelScope(OTelContext.makeCurrent(otelContext));
@@ -203,11 +193,11 @@ static Object createPropagatingSpan(Object otelContext) throws Throwable {
         return propagatingSpan;
     }
 
-    Object getOtelContext()  {
+    Object getOtelContext() {
         return otelContext;
     }
 
-    private static Scope wrapOTelScope(AutoCloseable otelScope) {
+    private static TracingScope wrapOTelScope(AutoCloseable otelScope) {
         return () -> {
             try {
                 otelScope.close();

sdk/clientcore/core/src/main/java/io/clientcore/core/implementation/telemetry/otel/tracing/OTelSpanBuilder.java

@@ -25,7 +25,8 @@
 import static io.clientcore.core.implementation.telemetry.otel.OTelInitializer.SPAN_KIND_CLASS;
 
 public class OTelSpanBuilder implements SpanBuilder {
-    static final OTelSpanBuilder NOOP = new OTelSpanBuilder(null,  SpanKind.INTERNAL, Context.none(), new LibraryTelemetryOptions("noop"));
+    static final OTelSpanBuilder NOOP
+        = new OTelSpanBuilder(null, SpanKind.INTERNAL, Context.none(), new LibraryTelemetryOptions("noop"));
 
     private static final MethodHandles.Lookup LOOKUP = MethodHandles.publicLookup();
     private static final ClientLogger LOGGER = new ClientLogger(OTelSpanBuilder.class);
@@ -145,7 +146,7 @@ private static Object getParent(Context context) throws Throwable {
         if (CONTEXT_CLASS.isInstance(parent)) {
             return parent;
         } else if (parent instanceof OTelSpan) {
-            return  ((OTelSpan) parent).getOtelContext();
+            return ((OTelSpan) parent).getOtelContext();
         } else if (parent != null) {
             LOGGER.atVerbose()
                 .addKeyValue("expectedType", CONTEXT_CLASS.getName())
@@ -160,12 +161,16 @@ private Object toOtelSpanKind(SpanKind spanKind) {
         switch (spanKind) {
             case SERVER:
                 return SERVER_KIND;
+
             case CLIENT:
                 return CLIENT_KIND;
+
             case PRODUCER:
                 return PRODUCER_KIND;
+
             case CONSUMER:
                 return CONSUMER_KIND;
+
             default:
                 return INTERNAL_KIND;
         }

sdk/clientcore/core/src/main/java/io/clientcore/core/implementation/telemetry/otel/tracing/OTelTracer.java

@@ -88,11 +88,12 @@ private OTelTracer(Object otelTracer, LibraryTelemetryOptions libraryOptions) {
     }
 
     @Override
-    public SpanBuilder spanBuilder(String spanName, SpanKind kind, RequestOptions options) {
+    public SpanBuilder spanBuilder(String spanName, SpanKind spanKind, RequestOptions options) {
         if (isEnabled()) {
             try {
                 Context parent = options == null ? Context.none() : options.getContext();
-                return new OTelSpanBuilder(SPAN_BUILDER_INVOKER.invoke(otelTracer, spanName), kind, parent, libraryOptions);
+                return new OTelSpanBuilder(SPAN_BUILDER_INVOKER.invoke(otelTracer, spanName), spanKind, parent,
+                    libraryOptions);
             } catch (Throwable t) {
                 OTelInitializer.runtimeError(LOGGER, t);
             }

sdk/clientcore/core/src/main/java/io/clientcore/core/telemetry/LibraryTelemetryOptions.java

@@ -9,9 +9,19 @@
 
 /**
  * Options for configuring library-specific telemetry settings.
- * <p>
  *
- * It's provided by the client library and is not intended to be used directly by the end users.
+ * <p><strong>This class is intended to be used by the client libraries only. Library options must not be provided or modified
+ * by application code</strong>
+ * <p>
+ * Library options describe the client library - it's name, version, and schema URL.
+ * Schema URL describes telemetry schema and version.
+ * <p>
+ * If your client library adds any attributes (links, events, etc.) to the spans,
+ * these properties SHOULD follow specific version of <a href="https://github.com/open-telemetry/semantic-conventions">OpenTelemetry Semantic Conventions</a>.
+ * And provide the corresponding schema URL.
+ * <p>
+ * The {@link LibraryTelemetryOptions} are usually static and shared across all instances of the client.
+ * Application developers are not expected to change them.
  */
 public final class LibraryTelemetryOptions {
     private final String libraryName;