Support item count and operation name in azmonexporter

Author: trask

agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/configuration/Configuration.java

@@ -156,6 +156,28 @@ public static class Sampling {
 
   public static class SamplingPreview {
 
+    // this is not the default for now at least, because
+    //
+    // parent not-sampled -> child not-sampled (always, to avoid broken traces)
+    // parent sampled -> child will still sample at its desired rate
+    //                   note: this is just sample rate of child, not sample rate of parent times
+    //                         sample rate of child, as both are using the same trace-id hash
+    //
+    // ??? if child sample rate is higher than the parent sample rate
+    //     parent sampled --> child sampled
+    //     parent not-sampled --> child not-sampled
+    //     which means that child has same effective sample rate as parent, and so its item count
+    //           will be wrong
+    //
+    // AND SO: if want to use parent-based sampler, then need to propagate the sample rate,
+    //         otherwise can end up with incorrect math
+    //
+    // Another (lesser) reason is because .NET SDK always propagates trace flags "00" (not
+    // sampled)
+    //
+    // IMPORTANT if changing this default, we need to keep it at least on Azure Functions
+    public boolean parentBased;
+
     public List<SamplingOverride> overrides = new ArrayList<>();
   }
 
@@ -261,10 +283,6 @@ public static class PreviewConfiguration {
     // world,
     // so safer to only allow single interval for now
     public int metricIntervalSeconds = 60;
-    // ignoreRemoteParentNotSampled is sometimes needed because .NET SDK always propagates trace
-    // flags "00" (not sampled)
-    // in particular, it is always needed in Azure Functions worker
-    public boolean ignoreRemoteParentNotSampled = DiagnosticsHelper.rpIntegrationChar() == 'f';
     public boolean captureControllerSpans;
     // this is just here to detect if using this old setting in order to give a helpful message
     @Deprecated public boolean httpMethodInOperationName;

agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/AppIdSupplier.java

@@ -29,7 +29,7 @@
 import com.azure.core.http.HttpResponse;
 import com.azure.monitor.opentelemetry.exporter.implementation.configuration.ConnectionString;
 import com.azure.monitor.opentelemetry.exporter.implementation.logging.NetworkFriendlyExceptions;
-import com.azure.monitor.opentelemetry.exporter.implementation.logging.WarningLogger;
+import com.azure.monitor.opentelemetry.exporter.implementation.logging.OperationLogger;
 import com.azure.monitor.opentelemetry.exporter.implementation.utils.ThreadPoolUtils;
 import com.microsoft.applicationinsights.agent.bootstrap.AiAppId;
 import com.microsoft.applicationinsights.agent.internal.httpclient.LazyHttpClient;
@@ -53,8 +53,8 @@ public class AppIdSupplier implements AiAppId.Supplier {
       Executors.newSingleThreadScheduledExecutor(
           ThreadPoolUtils.createDaemonThreadFactory(AppIdSupplier.class));
 
-  private static final WarningLogger warningLogger =
-      new WarningLogger(GetAppIdTask.class, "Unable to retrieve appId");
+  private static final OperationLogger operationLogger =
+      new OperationLogger(GetAppIdTask.class, "Retrieving appId");
 
   // guarded by taskLock
   private GetAppIdTask task;
@@ -147,7 +147,7 @@ public void run() {
       } catch (RuntimeException ex) {
         if (!NetworkFriendlyExceptions.logSpecialOneTimeFriendlyException(
             ex, url.toString(), friendlyExceptionThrown, logger)) {
-          warningLogger.recordWarning("exception sending request to " + url, ex, APP_ID_ERROR);
+          operationLogger.recordFailure("exception sending request to " + url, ex, APP_ID_ERROR);
         }
         backOff();
         return;
@@ -161,7 +161,7 @@ public void run() {
       String body = response.getBodyAsString().block();
       int statusCode = response.getStatusCode();
       if (statusCode != 200) {
-        warningLogger.recordWarning(
+        operationLogger.recordFailure(
             "received " + statusCode + " from " + url + "\nfull response:\n" + body,
             null,
             APP_ID_ERROR);
@@ -171,12 +171,12 @@ public void run() {
 
       // check for case when breeze returns invalid value
       if (body == null || body.isEmpty()) {
-        warningLogger.recordWarning("received empty body from " + url, null, APP_ID_ERROR);
+        operationLogger.recordFailure("received empty body from " + url, null, APP_ID_ERROR);
         backOff();
         return;
       }
 
-      logger.debug("appId retrieved: {}", body);
+      operationLogger.recordSuccess();
       appId = body;
     }
 

agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/InheritedAttributesLogProcessor.java

@@ -21,7 +21,6 @@
 
 package com.microsoft.applicationinsights.agent.internal.init;
 
-import com.azure.monitor.opentelemetry.exporter.implementation.AiOperationNameSpanProcessor;
 import com.microsoft.applicationinsights.agent.internal.configuration.Configuration;
 import com.microsoft.applicationinsights.agent.internal.processors.MyLogData;
 import io.opentelemetry.api.common.AttributeKey;
@@ -36,9 +35,6 @@
 
 public class InheritedAttributesLogProcessor implements LogProcessor {
 
-  private static final AttributeKey<String> AI_OPERATION_NAME_KEY =
-      AttributeKey.stringKey("applicationinsights.internal.operation_name");
-
   private static final AttributeKey<String> INSTRUMENTATION_KEY_KEY =
       AttributeKey.stringKey("ai.preview.instrumentation_key");
 
@@ -60,7 +56,6 @@ private static List<AttributeKey<?>> buildInheritedAttributesList(
     for (Configuration.InheritedAttribute inheritedAttribute : inheritedAttributes) {
       list.add(inheritedAttribute.getAttributeKey());
     }
-    list.add(AI_OPERATION_NAME_KEY);
     list.add(INSTRUMENTATION_KEY_KEY);
     list.add(ROLE_NAME_KEY);
     return list;
@@ -78,12 +73,7 @@ public void emit(LogData log) {
     ReadableSpan readableSpan = (ReadableSpan) currentSpan;
     AttributesBuilder builder = log.getAttributes().toBuilder();
     for (AttributeKey<?> inheritedAttributeKey : inheritedAttributes) {
-      Object value;
-      if (inheritedAttributeKey == AI_OPERATION_NAME_KEY) {
-        value = AiOperationNameSpanProcessor.getOperationName(readableSpan);
-      } else {
-        value = readableSpan.getAttribute(inheritedAttributeKey);
-      }
+      Object value = readableSpan.getAttribute(inheritedAttributeKey);
       if (value != null) {
         if (builder == null) {
           builder = log.getAttributes().toBuilder();

agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/SecondEntryPoint.java

@@ -23,7 +23,8 @@
 
 import static java.util.concurrent.TimeUnit.MINUTES;
 
-import com.azure.monitor.opentelemetry.exporter.implementation.AiOperationNameSpanProcessor;
+import com.azure.monitor.opentelemetry.exporter.implementation.AzureMonitorLogProcessor;
+import com.azure.monitor.opentelemetry.exporter.implementation.AzureMonitorSpanProcessor;
 import com.azure.monitor.opentelemetry.exporter.implementation.LogDataMapper;
 import com.azure.monitor.opentelemetry.exporter.implementation.MetricDataMapper;
 import com.azure.monitor.opentelemetry.exporter.implementation.SpanDataMapper;
@@ -295,32 +296,22 @@ private static SdkTracerProviderBuilder configureTracing(
       // and the default for DelegatingSampler is to not sample anything)
     }
 
-    // operation name span processor is only applied on span start, so doesn't need to be chained
-    // with the batch span processor
-    tracerProvider.addSpanProcessor(new AiOperationNameSpanProcessor());
-    // inherited attributes span processor is only applied on span start, so doesn't need to be
-    // chained with the batch span processor
+    tracerProvider.addSpanProcessor(new AzureMonitorSpanProcessor());
     if (!configuration.preview.inheritedAttributes.isEmpty()) {
       tracerProvider.addSpanProcessor(
           new InheritedAttributesSpanProcessor(configuration.preview.inheritedAttributes));
     }
-    // legacy span processor is only applied on span start, so doesn't need to be chained with the
-    // batch span processor
-    // it is used to pass legacy attributes from the context (extracted by the AiLegacyPropagator)
-    // to the span attributes (since there is no way to update attributes on span directly from
-    // propagator)
+    // legacy span processor is used to pass legacy attributes from the context (extracted by the
+    // AiLegacyPropagator) to the span attributes (since there is no way to update attributes on
+    // span directly from propagator)
     if (configuration.preview.legacyRequestIdPropagation.enabled) {
       tracerProvider.addSpanProcessor(new AiLegacyHeaderSpanProcessor());
     }
-    // instrumentation key overrides span processor is only applied on span start, so doesn't need
-    // to be chained with the batch span processor
     if (!configuration.preview.instrumentationKeyOverrides.isEmpty()) {
       tracerProvider.addSpanProcessor(
           new InheritedInstrumentationKeySpanProcessor(
               configuration.preview.instrumentationKeyOverrides));
     }
-    // role name overrides span processor is only applied on span start, so doesn't need
-    // to be chained with the batch span processor
     if (!configuration.preview.roleNameOverrides.isEmpty()) {
       tracerProvider.addSpanProcessor(
           new InheritedRoleNameSpanProcessor(configuration.preview.roleNameOverrides));
@@ -439,12 +430,10 @@ private static SdkLogEmitterProviderBuilder configureLogging(
     batchLogProcessor =
         BatchLogProcessor.builder(logExporter).setScheduleDelay(getBatchProcessorDelay()).build();
 
-    // inherited attributes log processor also handles operation name, ikey and role name attributes
-    // and these all need access to Span.current(), so must be run before passing off to the
-    // BatchLogProcessor
     return builder.addLogProcessor(
-        new InheritedAttributesLogProcessor(
-            configuration.preview.inheritedAttributes, batchLogProcessor));
+        new AzureMonitorLogProcessor(
+            new InheritedAttributesLogProcessor(
+                configuration.preview.inheritedAttributes, batchLogProcessor)));
   }
 
   private static LogExporter createLogExporter(

agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/legacyheaders/DelegatingPropagator.java

@@ -21,12 +21,7 @@
 
 package com.microsoft.applicationinsights.agent.internal.legacyheaders;
 
-import com.azure.monitor.opentelemetry.exporter.implementation.utils.TelemetryUtil;
 import io.opentelemetry.api.baggage.propagation.W3CBaggagePropagator;
-import io.opentelemetry.api.trace.Span;
-import io.opentelemetry.api.trace.SpanContext;
-import io.opentelemetry.api.trace.TraceFlags;
-import io.opentelemetry.api.trace.TraceState;
 import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.propagation.TextMapGetter;
@@ -73,9 +68,7 @@ public void setUpStandardDelegate(
       propagators.add(AiLegacyPropagator.getInstance());
     }
 
-    // currently using modified W3CTraceContextPropagator because "ai-internal-sp" trace state
-    // shouldn't be sent over the wire (at least not yet, and not with that name)
-    propagators.add(new ModifiedW3cTraceContextPropagator());
+    propagators.add(W3CTraceContextPropagator.getInstance());
     propagators.add(W3CBaggagePropagator.getInstance());
 
     delegate = TextMapPropagator.composite(propagators);
@@ -95,75 +88,4 @@ public <C> void inject(Context context, @Nullable C carrier, TextMapSetter<C> se
   public <C> Context extract(Context context, @Nullable C carrier, TextMapGetter<C> getter) {
     return delegate.extract(context, carrier, getter);
   }
-
-  private static class ModifiedW3cTraceContextPropagator implements TextMapPropagator {
-
-    private final TextMapPropagator delegate = W3CTraceContextPropagator.getInstance();
-
-    @Override
-    public Collection<String> fields() {
-      return delegate.fields();
-    }
-
-    @Override
-    public <C> void inject(Context context, @Nullable C carrier, TextMapSetter<C> setter) {
-      // do not propagate sampling percentage downstream YET
-      SpanContext spanContext = Span.fromContext(context).getSpanContext();
-      // sampling percentage should always be present, so no need to optimize with checking if
-      // present
-      TraceState traceState = spanContext.getTraceState();
-      TraceState updatedTraceState;
-      if (traceState.size() == 1
-          && traceState.get(TelemetryUtil.SAMPLING_PERCENTAGE_TRACE_STATE) != null) {
-        // this is a common case, worth optimizing
-        updatedTraceState = TraceState.getDefault();
-      } else {
-        updatedTraceState =
-            traceState.toBuilder().remove(TelemetryUtil.SAMPLING_PERCENTAGE_TRACE_STATE).build();
-      }
-      SpanContext updatedSpanContext = new ModifiedSpanContext(spanContext, updatedTraceState);
-      delegate.inject(Context.root().with(Span.wrap(updatedSpanContext)), carrier, setter);
-    }
-
-    @Override
-    public <C> Context extract(Context context, @Nullable C carrier, TextMapGetter<C> getter) {
-      return delegate.extract(context, carrier, getter);
-    }
-  }
-
-  private static class ModifiedSpanContext implements SpanContext {
-
-    private final SpanContext delegate;
-    private final TraceState traceState;
-
-    private ModifiedSpanContext(SpanContext delegate, TraceState traceState) {
-      this.delegate = delegate;
-      this.traceState = traceState;
-    }
-
-    @Override
-    public String getTraceId() {
-      return delegate.getTraceId();
-    }
-
-    @Override
-    public String getSpanId() {
-      return delegate.getSpanId();
-    }
-
-    @Override
-    public TraceFlags getTraceFlags() {
-      return delegate.getTraceFlags();
-    }
-
-    @Override
-    public TraceState getTraceState() {
-      return traceState;
-    }
-
-    @Override
-    public boolean isRemote() {
-      return delegate.isRemote();
-    }
-  }
 }