Merge branch 'main' into feature

open-telemetry · Feb 5, 2024 · c0eb638 · c0eb638
2 parents 5154a68 + a31bca8
commit c0eb638
Show file tree

Hide file tree

Showing 17 changed files with 117 additions and 58 deletions.
diff --git a/.github/workflows/Component.BuildTest.yml b/.github/workflows/Component.BuildTest.yml
@@ -68,13 +68,15 @@ jobs:
       run: dotnet-coverage merge -r -f cobertura -o ./TestResults/Cobertura.xml ./TestResults/*.coverage
 
     - name: Upload code coverage ${{ inputs.code-cov-prefix }}-${{ inputs.code-cov-name }}
-      uses: codecov/codecov-action@v3.1.5
+      uses: codecov/codecov-action@v4
       continue-on-error: true # Note: Don't fail for upload failures
       env:
         OS: ${{ matrix.os }}
         TFM: ${{ matrix.version }}
+        token: ${{ secrets.CODECOV_TOKEN }}
       with:
         file: TestResults/Cobertura.xml
         env_vars: OS,TFM
         flags: ${{ inputs.code-cov-prefix }}-${{ inputs.code-cov-name }}
         name: Code Coverage for ${{ inputs.code-cov-prefix }}-${{ inputs.code-cov-name }} on [${{ matrix.os }}.${{ matrix.version }}]
+        codecov_yml_path: .github/codecov.yml
diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml
@@ -25,7 +25,7 @@ jobs:
 
     steps:
     - name: configure Pagefile
-      uses: al-cheb/configure-pagefile-action@v1.3
+      uses: al-cheb/configure-pagefile-action@v1.4
       with:
         minimum-size: 8GB
         maximum-size: 32GB

diff --git a/.github/workflows/markdownlint.yml b/.github/workflows/markdownlint.yml
@@ -14,7 +14,7 @@ jobs:
       uses: actions/checkout@v4
 
     - name: run markdownlint
-      uses: DavidAnson/markdownlint-cli2-action@v14.0.0
+      uses: DavidAnson/markdownlint-cli2-action@v15.0.0
       with:
         globs: |
           **/*.md

diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md
@@ -21,6 +21,11 @@
   experimental Logs Bridge API.
   ([#5268](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5268))
 
+* Updated `OtlpLogExporter` to set instrumentation scope name on the data model
+  from `LogRecord.Logger.Name` if `LogRecord.CategoryName` is `null`. This is
+  typically the case when using the experimental Logs Bridge API.
+  ([#5300](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5300))
+
 ## 1.7.0
 
 Released 2023-Dec-08

diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpLogRecordTransformer.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpLogRecordTransformer.cs
@@ -18,6 +18,8 @@ internal sealed class OtlpLogRecordTransformer
 {
     internal static readonly ConcurrentBag<OtlpLogs.ScopeLogs> LogListPool = new();
 
+    private const string DefaultScopeName = "";
+
     private readonly SdkLimitOptions sdkLimitOptions;
     private readonly ExperimentalOptions experimentalOptions;
 
@@ -47,10 +49,11 @@ internal OtlpCollector.ExportLogsServiceRequest BuildExportRequest(
             var otlpLogRecord = this.ToOtlpLog(logRecord);
             if (otlpLogRecord != null)
             {
-                if (!logsByCategory.TryGetValue(logRecord.CategoryName, out var scopeLogs))
+                var scopeName = logRecord.CategoryName ?? logRecord.Logger?.Name ?? DefaultScopeName;
+                if (!logsByCategory.TryGetValue(scopeName, out var scopeLogs))
                 {
-                    scopeLogs = this.GetLogListFromPool(logRecord.CategoryName);
-                    logsByCategory.Add(logRecord.CategoryName, scopeLogs);
+                    scopeLogs = this.GetLogListFromPool(scopeName);
+                    logsByCategory.Add(scopeName, scopeLogs);
                     resourceLogs.ScopeLogs.Add(scopeLogs);
                 }
 

diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md
@@ -11,6 +11,8 @@
 * **Breaking Change**: Renamed `SqlClientInstrumentationOptions` to
   `SqlClientTraceInstrumentationOptions`.
   ([#5285](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5285))
+* **Breaking Change**: Stop emitting `db.statement_type` attribute.
+  ([#5301](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5301))
 
 ## 1.6.0-beta.3
 

diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs
@@ -108,7 +108,6 @@ public override void OnEventWritten(string name, object payload)
                             switch (commandType)
                             {
                                 case CommandType.StoredProcedure:
-                                    activity.SetTag(SpanAttributeConstants.DatabaseStatementTypeKey, nameof(CommandType.StoredProcedure));
                                     if (this.options.SetDbStatementForStoredProcedure)
                                     {
                                         activity.SetTag(SemanticConventions.AttributeDbStatement, (string)commandText);
@@ -117,7 +116,6 @@ public override void OnEventWritten(string name, object payload)
                                     break;
 
                                 case CommandType.Text:
-                                    activity.SetTag(SpanAttributeConstants.DatabaseStatementTypeKey, nameof(CommandType.Text));
                                     if (this.options.SetDbStatementForText)
                                     {
                                         activity.SetTag(SemanticConventions.AttributeDbStatement, (string)commandText);
@@ -126,7 +124,6 @@ public override void OnEventWritten(string name, object payload)
                                     break;
 
                                 case CommandType.TableDirect:
-                                    activity.SetTag(SpanAttributeConstants.DatabaseStatementTypeKey, nameof(CommandType.TableDirect));
                                     break;
                             }
                         }

diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs
@@ -12,7 +12,7 @@ namespace OpenTelemetry.Metrics;
 internal sealed class AggregatorStore
 {
     internal readonly bool OutputDelta;
-    internal readonly bool ShouldReclaimUnusedMetricPoints;
+    internal readonly bool OutputDeltaWithUnusedMetricPointReclaimEnabled;
     internal long DroppedMeasurements = 0;
 
     private static readonly string MetricPointCapHitFixMessage = "Consider opting in for the experimental SDK feature to emit all the throttled metrics under the overflow attribute by setting env variable OTEL_DOTNET_EXPERIMENTAL_METRICS_EMIT_OVERFLOW_ATTRIBUTE = true. You could also modify instrumentation to reduce the number of unique key/value pair combinations. Or use Views to drop unwanted tags. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit.";
@@ -101,9 +101,9 @@ internal AggregatorStore(
             reservedMetricPointsCount++;
         }
 
-        this.ShouldReclaimUnusedMetricPoints = shouldReclaimUnusedMetricPoints;
+        this.OutputDeltaWithUnusedMetricPointReclaimEnabled = shouldReclaimUnusedMetricPoints && this.OutputDelta;
 
-        if (this.OutputDelta && shouldReclaimUnusedMetricPoints)
+        if (this.OutputDeltaWithUnusedMetricPointReclaimEnabled)
         {
             this.availableMetricPoints = new Queue<int>(maxMetricPoints - reservedMetricPointsCount);
 
@@ -158,17 +158,14 @@ internal void Update(double value, ReadOnlySpan<KeyValuePair<string, object?>> t
     internal int Snapshot()
     {
         this.batchSize = 0;
-        if (this.OutputDelta)
+        if (this.OutputDeltaWithUnusedMetricPointReclaimEnabled)
         {
-            if (this.ShouldReclaimUnusedMetricPoints)
-            {
-                this.SnapshotDeltaWithMetricPointReclaim();
-            }
-            else
-            {
-                var indexSnapshot = Math.Min(this.metricPointIndex, this.maxMetricPoints - 1);
-                this.SnapshotDelta(indexSnapshot);
-            }
+            this.SnapshotDeltaWithMetricPointReclaim();
+        }
+        else if (this.OutputDelta)
+        {
+            var indexSnapshot = Math.Min(this.metricPointIndex, this.maxMetricPoints - 1);
+            this.SnapshotDelta(indexSnapshot);
         }
         else
         {

diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs
@@ -50,11 +50,7 @@ internal MetricPoint(
     {
         Debug.Assert(aggregatorStore != null, "AggregatorStore was null.");
         Debug.Assert(histogramExplicitBounds != null, "Histogram explicit Bounds was null.");
-
-        if (aggregatorStore!.OutputDelta && aggregatorStore.ShouldReclaimUnusedMetricPoints)
-        {
-            Debug.Assert(lookupData != null, "LookupData was null.");
-        }
+        Debug.Assert(!aggregatorStore!.OutputDeltaWithUnusedMetricPointReclaimEnabled || lookupData != null, "LookupData was null.");
 
         this.aggType = aggType;
         this.Tags = new ReadOnlyTagCollection(tagKeysAndValues);
@@ -445,7 +441,7 @@ internal void Update(long number)
         // by ignoring Zero points
         this.MetricPointStatus = MetricPointStatus.CollectPending;
 
-        if (this.aggregatorStore.OutputDelta)
+        if (this.aggregatorStore.OutputDeltaWithUnusedMetricPointReclaimEnabled)
         {
             Interlocked.Decrement(ref this.ReferenceCount);
         }
@@ -570,7 +566,7 @@ internal void UpdateWithExemplar(long number, ReadOnlySpan<KeyValuePair<string,
         // by ignoring Zero points
         this.MetricPointStatus = MetricPointStatus.CollectPending;
 
-        if (this.aggregatorStore.OutputDelta)
+        if (this.aggregatorStore.OutputDeltaWithUnusedMetricPointReclaimEnabled)
         {
             Interlocked.Decrement(ref this.ReferenceCount);
         }
@@ -666,7 +662,7 @@ internal void Update(double number)
         // by ignoring Zero points
         this.MetricPointStatus = MetricPointStatus.CollectPending;
 
-        if (this.aggregatorStore.OutputDelta)
+        if (this.aggregatorStore.OutputDeltaWithUnusedMetricPointReclaimEnabled)
         {
             Interlocked.Decrement(ref this.ReferenceCount);
         }
@@ -797,7 +793,7 @@ internal void UpdateWithExemplar(double number, ReadOnlySpan<KeyValuePair<string
         // by ignoring Zero points
         this.MetricPointStatus = MetricPointStatus.CollectPending;
 
-        if (this.aggregatorStore.OutputDelta)
+        if (this.aggregatorStore.OutputDeltaWithUnusedMetricPointReclaimEnabled)
         {
             Interlocked.Decrement(ref this.ReferenceCount);
         }

diff --git a/src/Shared/SpanAttributeConstants.cs b/src/Shared/SpanAttributeConstants.cs
@@ -12,5 +12,4 @@ internal static class SpanAttributeConstants
 {
     public const string StatusCodeKey = "otel.status_code";
     public const string StatusDescriptionKey = "otel.status_description";
-    public const string DatabaseStatementTypeKey = "db.statement_type";
 }
diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs
@@ -1469,6 +1469,44 @@ public void VerifyEnvironmentVariablesTakenFromIConfigurationWhenUsingLoggingBui
             });
     }
 
+    [Theory]
+    [InlineData("my_instrumentation_scope_name", "my_instrumentation_scope_name")]
+    [InlineData(null, "")]
+    public void LogRecordLoggerNameIsExportedWhenUsingBridgeApi(string loggerName, string expectedScopeName)
+    {
+        LogRecordAttributeList attributes = default;
+        attributes.Add("name", "tomato");
+        attributes.Add("price", 2.99);
+        attributes.Add("{OriginalFormat}", "Hello from {name} {price}.");
+
+        var logRecords = new List<LogRecord>();
+
+        using (var loggerProvider = Sdk.CreateLoggerProviderBuilder()
+                   .AddInMemoryExporter(logRecords)
+                   .Build())
+        {
+            var logger = loggerProvider.GetLogger(loggerName);
+
+            logger.EmitLog(new LogRecordData());
+        }
+
+        Assert.Single(logRecords);
+
+        var otlpLogRecordTransformer = new OtlpLogRecordTransformer(DefaultSdkLimitOptions, new());
+
+        var batch = new Batch<LogRecord>(new[] { logRecords[0] }, 1);
+
+        var request = otlpLogRecordTransformer.BuildExportRequest(
+            new Proto.Resource.V1.Resource(),
+            batch);
+
+        Assert.NotNull(request);
+        Assert.Single(request.ResourceLogs);
+        Assert.Single(request.ResourceLogs[0].ScopeLogs);
+
+        Assert.Equal(expectedScopeName, request.ResourceLogs[0].ScopeLogs[0].Scope?.Name);
+    }
+
     private static void RunVerifyEnvironmentVariablesTakenFromIConfigurationTest(
         string optionsName,
         Func<Action<IServiceCollection>, (IDisposable Container, ILoggerFactory LoggerFactory)> createLoggerFactoryFunc)

diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.net6.0.md b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.net6.0.md
@@ -8,9 +8,9 @@
 | :green_heart: | ConventionalRouting | [Not Found (404)](#conventionalrouting-not-found-404) |
 | :green_heart: | ConventionalRouting | [Route template with parameter constraint](#conventionalrouting-route-template-with-parameter-constraint) |
 | :green_heart: | ConventionalRouting | [Path that does not match parameter constraint](#conventionalrouting-path-that-does-not-match-parameter-constraint) |
-| :broken_heart: | ConventionalRouting | [Area using area:exists, default controller/action](#conventionalrouting-area-using-areaexists-default-controlleraction) |
-| :broken_heart: | ConventionalRouting | [Area using area:exists, non-default action](#conventionalrouting-area-using-areaexists-non-default-action) |
-| :broken_heart: | ConventionalRouting | [Area w/o area:exists, default controller/action](#conventionalrouting-area-wo-areaexists-default-controlleraction) |
+| :broken_heart: | ConventionalRouting | [Area using `area:exists`, default controller/action](#conventionalrouting-area-using-areaexists-default-controlleraction) |
+| :broken_heart: | ConventionalRouting | [Area using `area:exists`, non-default action](#conventionalrouting-area-using-areaexists-non-default-action) |
+| :broken_heart: | ConventionalRouting | [Area w/o `area:exists`, default controller/action](#conventionalrouting-area-wo-areaexists-default-controlleraction) |
 | :green_heart: | AttributeRouting | [Default action](#attributerouting-default-action) |
 | :green_heart: | AttributeRouting | [Action without parameter](#attributerouting-action-without-parameter) |
 | :green_heart: | AttributeRouting | [Action with parameter](#attributerouting-action-with-parameter) |
@@ -194,7 +194,7 @@
 }
 ```
 
-## ConventionalRouting: Area using area:exists, default controller/action
+## ConventionalRouting: Area using `area:exists`, default controller/action
 
 ```json
 {
@@ -225,7 +225,7 @@
 }
 ```
 
-## ConventionalRouting: Area using area:exists, non-default action
+## ConventionalRouting: Area using `area:exists`, non-default action
 
 ```json
 {
@@ -256,7 +256,7 @@
 }
 ```
 
-## ConventionalRouting: Area w/o area:exists, default controller/action
+## ConventionalRouting: Area w/o `area:exists`, default controller/action
 
 ```json
 {

diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.net7.0.md b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.net7.0.md
@@ -8,9 +8,9 @@
 | :green_heart: | ConventionalRouting | [Not Found (404)](#conventionalrouting-not-found-404) |
 | :green_heart: | ConventionalRouting | [Route template with parameter constraint](#conventionalrouting-route-template-with-parameter-constraint) |
 | :green_heart: | ConventionalRouting | [Path that does not match parameter constraint](#conventionalrouting-path-that-does-not-match-parameter-constraint) |
-| :broken_heart: | ConventionalRouting | [Area using area:exists, default controller/action](#conventionalrouting-area-using-areaexists-default-controlleraction) |
-| :broken_heart: | ConventionalRouting | [Area using area:exists, non-default action](#conventionalrouting-area-using-areaexists-non-default-action) |
-| :broken_heart: | ConventionalRouting | [Area w/o area:exists, default controller/action](#conventionalrouting-area-wo-areaexists-default-controlleraction) |
+| :broken_heart: | ConventionalRouting | [Area using `area:exists`, default controller/action](#conventionalrouting-area-using-areaexists-default-controlleraction) |
+| :broken_heart: | ConventionalRouting | [Area using `area:exists`, non-default action](#conventionalrouting-area-using-areaexists-non-default-action) |
+| :broken_heart: | ConventionalRouting | [Area w/o `area:exists`, default controller/action](#conventionalrouting-area-wo-areaexists-default-controlleraction) |
 | :green_heart: | AttributeRouting | [Default action](#attributerouting-default-action) |
 | :green_heart: | AttributeRouting | [Action without parameter](#attributerouting-action-without-parameter) |
 | :green_heart: | AttributeRouting | [Action with parameter](#attributerouting-action-with-parameter) |
@@ -196,7 +196,7 @@
 }
 ```
 
-## ConventionalRouting: Area using area:exists, default controller/action
+## ConventionalRouting: Area using `area:exists`, default controller/action
 
 ```json
 {
@@ -227,7 +227,7 @@
 }
 ```
 
-## ConventionalRouting: Area using area:exists, non-default action
+## ConventionalRouting: Area using `area:exists`, non-default action
 
 ```json
 {
@@ -258,7 +258,7 @@
 }
 ```
 
-## ConventionalRouting: Area w/o area:exists, default controller/action
+## ConventionalRouting: Area w/o `area:exists`, default controller/action
 
 ```json
 {