From 9dcb8393a5eff65b7e61852303f7d4cfdcc7f247 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Wed, 16 Feb 2022 15:21:10 -0800 Subject: [PATCH 01/15] poc for fix to InMemoryExporter --- .../InMemoryExporter.cs | 9 +++ src/OpenTelemetry/Metrics/AggregatorStore.cs | 11 +++ src/OpenTelemetry/Metrics/HistogramBuckets.cs | 4 + src/OpenTelemetry/Metrics/Metric.cs | 5 +- src/OpenTelemetry/Metrics/MetricPoint.cs | 81 +++++++++++++------ .../Metrics/InMemoryExporterTests.cs | 49 ++++++++++- 6 files changed, 133 insertions(+), 26 deletions(-) diff --git a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs index d76995f8fb6..da9777fe2bb 100644 --- a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs +++ b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs @@ -22,10 +22,12 @@ public class InMemoryExporter : BaseExporter where T : class { private readonly ICollection exportedItems; + private readonly bool isMetric; public InMemoryExporter(ICollection exportedItems) { this.exportedItems = exportedItems; + this.isMetric = typeof(T) == typeof(Metrics.Metric); } public override ExportResult Export(in Batch batch) @@ -35,6 +37,13 @@ public override ExportResult Export(in Batch batch) return ExportResult.Failure; } + // Note: this mitigates the growing ExportedItems issue. + // Need to discuss if this is an acceptible change. + if (this.isMetric) + { + this.exportedItems.Clear(); + } + foreach (var data in batch) { this.exportedItems.Add(data); diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index c61f1a59273..09ef496272b 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -156,6 +156,17 @@ internal MetricPointsAccessor GetMetricPoints() return new MetricPointsAccessor(this.metricPoints, this.currentMetricPointBatch, this.batchSize, this.startTimeExclusive, this.endTimeInclusive); } + internal MetricPointsAccessor GetDeepCloneMetricPoints() + { + var deepClonedMetricPoints = new MetricPoint[this.metricPoints.Length]; + for (int i = 0; i < this.metricPoints.Length; i++) + { + deepClonedMetricPoints[i] = this.metricPoints[i].DeepCopy(); + } + + return new MetricPointsAccessor(deepClonedMetricPoints, this.currentMetricPointBatch, this.batchSize, this.startTimeExclusive, this.endTimeInclusive); + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] private void InitializeZeroTagPointIfNotInitialized() { diff --git a/src/OpenTelemetry/Metrics/HistogramBuckets.cs b/src/OpenTelemetry/Metrics/HistogramBuckets.cs index a68033d4300..d58300ba864 100644 --- a/src/OpenTelemetry/Metrics/HistogramBuckets.cs +++ b/src/OpenTelemetry/Metrics/HistogramBuckets.cs @@ -43,6 +43,10 @@ internal HistogramBuckets(double[] explicitBounds) public Enumerator GetEnumerator() => new(this); + // This works because all private fields are value types. + // If this class changes significantly, this may need to change. + internal HistogramBuckets DeepCopy() => (HistogramBuckets)this.MemberwiseClone(); + /// /// Enumerates the elements of a . /// diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index 983976d5e9e..2abd0b6e6d5 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -128,7 +128,10 @@ internal Metric( public MetricPointsAccessor GetMetricPoints() { - return this.aggStore.GetMetricPoints(); + // Note: This appears to be safe for all existing unit tests. + // This is safe because the enumerator only moves forward. + // This may not be desirable for all use cases, in which we would need a new public method. + return this.aggStore.GetDeepCloneMetricPoints(); } internal void UpdateLong(long value, ReadOnlySpan> tags) diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs index e91c1253ace..b416d5a2382 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint.cs @@ -28,7 +28,8 @@ public struct MetricPoint { private readonly AggregationType aggType; - private readonly HistogramBuckets histogramBuckets; + // Note: not necessary to remove this, but makes DeepCopy easier. + //private readonly HistogramBuckets histogramBuckets; // Represents temporality adjusted "value" for double/long metric types or "count" when histogram private MetricPointValueStorage runningValue; @@ -59,18 +60,35 @@ internal MetricPoint( if (this.aggType == AggregationType.Histogram) { - this.histogramBuckets = new HistogramBuckets(histogramExplicitBounds); + this.HistogramBuckets = new HistogramBuckets(histogramExplicitBounds); } else if (this.aggType == AggregationType.HistogramSumCount) { - this.histogramBuckets = new HistogramBuckets(null); + this.HistogramBuckets = new HistogramBuckets(null); } else { - this.histogramBuckets = null; + this.HistogramBuckets = null; } } + //internal MetricPoint(MetricPoint other) + //{ + // Note: This is an alternative to the DeepCopy method below. + // I'm not a fan of this because it creates extra maintenance. + + // this.aggType = other.aggType; + // this.HistogramBuckets = other.HistogramBuckets?.DeepClone(); + // this.runningValue = other.runningValue; + // this.snapshotValue = other.snapshotValue; + // this.deltaLastValue = other.deltaLastValue; + // this.Tags = other.Tags; + // this.MetricPointStatus = other.MetricPointStatus; + // this.Tags = other.Tags; + // this.StartTime = other.StartTime; + // this.EndTime = other.EndTime; + //} + /// /// Gets the tags associated with the metric point. /// @@ -113,6 +131,13 @@ internal MetricPointStatus MetricPointStatus private set; } + // Note: changing this to a Property loses the "readonly", + // but enables DeepCopy using the MemberwiseClone pattern below. + private HistogramBuckets HistogramBuckets + { + get; set; + } + /// /// Gets the sum long value associated with the metric point. /// @@ -218,7 +243,7 @@ public double GetHistogramSum() this.ThrowNotSupportedMetricTypeException(nameof(this.GetHistogramSum)); } - return this.histogramBuckets.SnapshotSum; + return this.HistogramBuckets.SnapshotSum; } /// @@ -227,7 +252,7 @@ public double GetHistogramSum() /// /// Applies to metric type. /// - /// . + /// . [MethodImpl(MethodImplOptions.AggressiveInlining)] public HistogramBuckets GetHistogramBuckets() { @@ -236,7 +261,17 @@ public HistogramBuckets GetHistogramBuckets() this.ThrowNotSupportedMetricTypeException(nameof(this.GetHistogramBuckets)); } - return this.histogramBuckets; + return this.HistogramBuckets; + } + + // this would be easier to maintain. + // Currently this doesn't work because histogramBuckets is readonly, but could hack using Reflection. + internal MetricPoint DeepCopy() + { + var other = (MetricPoint)this.MemberwiseClone(); + other.HistogramBuckets = this.HistogramBuckets?.DeepCopy(); + + return other; } internal void Update(long number) @@ -314,20 +349,20 @@ internal void Update(double number) case AggregationType.Histogram: { int i; - for (i = 0; i < this.histogramBuckets.ExplicitBounds.Length; i++) + for (i = 0; i < this.HistogramBuckets.ExplicitBounds.Length; i++) { // Upper bound is inclusive - if (number <= this.histogramBuckets.ExplicitBounds[i]) + if (number <= this.HistogramBuckets.ExplicitBounds[i]) { break; } } - lock (this.histogramBuckets.LockObject) + lock (this.HistogramBuckets.LockObject) { this.runningValue.AsLong++; - this.histogramBuckets.RunningSum += number; - this.histogramBuckets.RunningBucketCounts[i]++; + this.HistogramBuckets.RunningSum += number; + this.HistogramBuckets.RunningBucketCounts[i]++; } break; @@ -335,10 +370,10 @@ internal void Update(double number) case AggregationType.HistogramSumCount: { - lock (this.histogramBuckets.LockObject) + lock (this.HistogramBuckets.LockObject) { this.runningValue.AsLong++; - this.histogramBuckets.RunningSum += number; + this.HistogramBuckets.RunningSum += number; } break; @@ -460,22 +495,22 @@ internal void TakeSnapshot(bool outputDelta) case AggregationType.Histogram: { - lock (this.histogramBuckets.LockObject) + lock (this.HistogramBuckets.LockObject) { this.snapshotValue.AsLong = this.runningValue.AsLong; - this.histogramBuckets.SnapshotSum = this.histogramBuckets.RunningSum; + this.HistogramBuckets.SnapshotSum = this.HistogramBuckets.RunningSum; if (outputDelta) { this.runningValue.AsLong = 0; - this.histogramBuckets.RunningSum = 0; + this.HistogramBuckets.RunningSum = 0; } - for (int i = 0; i < this.histogramBuckets.RunningBucketCounts.Length; i++) + for (int i = 0; i < this.HistogramBuckets.RunningBucketCounts.Length; i++) { - this.histogramBuckets.SnapshotBucketCounts[i] = this.histogramBuckets.RunningBucketCounts[i]; + this.HistogramBuckets.SnapshotBucketCounts[i] = this.HistogramBuckets.RunningBucketCounts[i]; if (outputDelta) { - this.histogramBuckets.RunningBucketCounts[i] = 0; + this.HistogramBuckets.RunningBucketCounts[i] = 0; } } @@ -487,14 +522,14 @@ internal void TakeSnapshot(bool outputDelta) case AggregationType.HistogramSumCount: { - lock (this.histogramBuckets.LockObject) + lock (this.HistogramBuckets.LockObject) { this.snapshotValue.AsLong = this.runningValue.AsLong; - this.histogramBuckets.SnapshotSum = this.histogramBuckets.RunningSum; + this.HistogramBuckets.SnapshotSum = this.HistogramBuckets.RunningSum; if (outputDelta) { this.runningValue.AsLong = 0; - this.histogramBuckets.RunningSum = 0; + this.HistogramBuckets.RunningSum = 0; } this.MetricPointStatus = MetricPointStatus.NoCollectPending; diff --git a/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs b/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs index 5f5914be28e..926097728d8 100644 --- a/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs @@ -16,15 +16,17 @@ using System.Collections.Generic; using System.Diagnostics.Metrics; + using OpenTelemetry.Exporter; using OpenTelemetry.Tests; + using Xunit; namespace OpenTelemetry.Metrics.Tests { public class InMemoryExporterTests { - [Fact(Skip = "To be run after https://github.com/open-telemetry/opentelemetry-dotnet/issues/2361 is fixed")] + [Fact] public void InMemoryExporterShouldDeepCopyMetricPoints() { var exportedItems = new List(); @@ -56,7 +58,7 @@ public void InMemoryExporterShouldDeepCopyMetricPoints() meterProvider.ForceFlush(); - metric = exportedItems[1]; // Second Metric object is added to the collection at this point + metric = exportedItems[0]; // Second Metric object is added to the collection at this point metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator(); Assert.True(metricPointsEnumerator.MoveNext()); // One MetricPoint is emitted for the Metric var metricPointForSecondExport = metricPointsEnumerator.Current; @@ -65,5 +67,48 @@ public void InMemoryExporterShouldDeepCopyMetricPoints() // MetricPoint.LongValue for the first exporter metric should still be 10 Assert.Equal(10, metricPointForFirstExport.GetSumLong()); } + + [Fact] + public void Investigate_2361() + { + // https://github.com/open-telemetry/opentelemetry-dotnet/issues/2361 + + var exportedItems = new List(); + + using var meter = new Meter(Utils.GetCurrentMethodName()); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems) + .Build(); + + int i = 0; + var counterLong = meter.CreateObservableCounter( + "observable-counter", + () => ++i * 10); + + meterProvider.ForceFlush(); + Assert.Equal(1, i); // verify that callback is invoked when calling Flush + Assert.Single(exportedItems); // verify that List contains 1 item + var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); + Assert.Equal(10, metricPoint1.GetSumLong()); + + meterProvider.ForceFlush(); + Assert.Equal(2, i); // verify that callback is invoked when calling Flush + Assert.Single(exportedItems); // verify that List contains 1 item + var metricPoint2 = GetSingleMetricPoint(exportedItems[0]); + Assert.Equal(20, metricPoint2.GetSumLong()); + + // Retest 1st item, this is expected to be unchanged. + Assert.Equal(10, metricPoint1.GetSumLong()); + } + + private static MetricPoint GetSingleMetricPoint(Metric metric) + { + var metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator(); + Assert.True(metricPointsEnumerator.MoveNext()); // One MetricPoint is emitted for the Metric + ref readonly var metricPoint = ref metricPointsEnumerator.Current; + Assert.False(metricPointsEnumerator.MoveNext()); + return metricPoint; + } } } From 34d15ecf5bcd7d5e418f8c6d2de32f320a04a6f2 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Tue, 22 Feb 2022 13:39:07 -0800 Subject: [PATCH 02/15] Revert "poc for fix to InMemoryExporter" This reverts commit 9dcb8393a5eff65b7e61852303f7d4cfdcc7f247. --- .../InMemoryExporter.cs | 9 --- src/OpenTelemetry/Metrics/AggregatorStore.cs | 11 --- src/OpenTelemetry/Metrics/HistogramBuckets.cs | 4 - src/OpenTelemetry/Metrics/Metric.cs | 5 +- src/OpenTelemetry/Metrics/MetricPoint.cs | 81 ++++++------------- .../Metrics/InMemoryExporterTests.cs | 49 +---------- 6 files changed, 26 insertions(+), 133 deletions(-) diff --git a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs index da9777fe2bb..d76995f8fb6 100644 --- a/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs +++ b/src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs @@ -22,12 +22,10 @@ public class InMemoryExporter : BaseExporter where T : class { private readonly ICollection exportedItems; - private readonly bool isMetric; public InMemoryExporter(ICollection exportedItems) { this.exportedItems = exportedItems; - this.isMetric = typeof(T) == typeof(Metrics.Metric); } public override ExportResult Export(in Batch batch) @@ -37,13 +35,6 @@ public override ExportResult Export(in Batch batch) return ExportResult.Failure; } - // Note: this mitigates the growing ExportedItems issue. - // Need to discuss if this is an acceptible change. - if (this.isMetric) - { - this.exportedItems.Clear(); - } - foreach (var data in batch) { this.exportedItems.Add(data); diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index 09ef496272b..c61f1a59273 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -156,17 +156,6 @@ internal MetricPointsAccessor GetMetricPoints() return new MetricPointsAccessor(this.metricPoints, this.currentMetricPointBatch, this.batchSize, this.startTimeExclusive, this.endTimeInclusive); } - internal MetricPointsAccessor GetDeepCloneMetricPoints() - { - var deepClonedMetricPoints = new MetricPoint[this.metricPoints.Length]; - for (int i = 0; i < this.metricPoints.Length; i++) - { - deepClonedMetricPoints[i] = this.metricPoints[i].DeepCopy(); - } - - return new MetricPointsAccessor(deepClonedMetricPoints, this.currentMetricPointBatch, this.batchSize, this.startTimeExclusive, this.endTimeInclusive); - } - [MethodImpl(MethodImplOptions.AggressiveInlining)] private void InitializeZeroTagPointIfNotInitialized() { diff --git a/src/OpenTelemetry/Metrics/HistogramBuckets.cs b/src/OpenTelemetry/Metrics/HistogramBuckets.cs index d58300ba864..a68033d4300 100644 --- a/src/OpenTelemetry/Metrics/HistogramBuckets.cs +++ b/src/OpenTelemetry/Metrics/HistogramBuckets.cs @@ -43,10 +43,6 @@ internal HistogramBuckets(double[] explicitBounds) public Enumerator GetEnumerator() => new(this); - // This works because all private fields are value types. - // If this class changes significantly, this may need to change. - internal HistogramBuckets DeepCopy() => (HistogramBuckets)this.MemberwiseClone(); - /// /// Enumerates the elements of a . /// diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index 2abd0b6e6d5..983976d5e9e 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -128,10 +128,7 @@ internal Metric( public MetricPointsAccessor GetMetricPoints() { - // Note: This appears to be safe for all existing unit tests. - // This is safe because the enumerator only moves forward. - // This may not be desirable for all use cases, in which we would need a new public method. - return this.aggStore.GetDeepCloneMetricPoints(); + return this.aggStore.GetMetricPoints(); } internal void UpdateLong(long value, ReadOnlySpan> tags) diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs index b416d5a2382..e91c1253ace 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint.cs @@ -28,8 +28,7 @@ public struct MetricPoint { private readonly AggregationType aggType; - // Note: not necessary to remove this, but makes DeepCopy easier. - //private readonly HistogramBuckets histogramBuckets; + private readonly HistogramBuckets histogramBuckets; // Represents temporality adjusted "value" for double/long metric types or "count" when histogram private MetricPointValueStorage runningValue; @@ -60,35 +59,18 @@ internal MetricPoint( if (this.aggType == AggregationType.Histogram) { - this.HistogramBuckets = new HistogramBuckets(histogramExplicitBounds); + this.histogramBuckets = new HistogramBuckets(histogramExplicitBounds); } else if (this.aggType == AggregationType.HistogramSumCount) { - this.HistogramBuckets = new HistogramBuckets(null); + this.histogramBuckets = new HistogramBuckets(null); } else { - this.HistogramBuckets = null; + this.histogramBuckets = null; } } - //internal MetricPoint(MetricPoint other) - //{ - // Note: This is an alternative to the DeepCopy method below. - // I'm not a fan of this because it creates extra maintenance. - - // this.aggType = other.aggType; - // this.HistogramBuckets = other.HistogramBuckets?.DeepClone(); - // this.runningValue = other.runningValue; - // this.snapshotValue = other.snapshotValue; - // this.deltaLastValue = other.deltaLastValue; - // this.Tags = other.Tags; - // this.MetricPointStatus = other.MetricPointStatus; - // this.Tags = other.Tags; - // this.StartTime = other.StartTime; - // this.EndTime = other.EndTime; - //} - /// /// Gets the tags associated with the metric point. /// @@ -131,13 +113,6 @@ internal MetricPointStatus MetricPointStatus private set; } - // Note: changing this to a Property loses the "readonly", - // but enables DeepCopy using the MemberwiseClone pattern below. - private HistogramBuckets HistogramBuckets - { - get; set; - } - /// /// Gets the sum long value associated with the metric point. /// @@ -243,7 +218,7 @@ public double GetHistogramSum() this.ThrowNotSupportedMetricTypeException(nameof(this.GetHistogramSum)); } - return this.HistogramBuckets.SnapshotSum; + return this.histogramBuckets.SnapshotSum; } /// @@ -252,7 +227,7 @@ public double GetHistogramSum() /// /// Applies to metric type. /// - /// . + /// . [MethodImpl(MethodImplOptions.AggressiveInlining)] public HistogramBuckets GetHistogramBuckets() { @@ -261,17 +236,7 @@ public HistogramBuckets GetHistogramBuckets() this.ThrowNotSupportedMetricTypeException(nameof(this.GetHistogramBuckets)); } - return this.HistogramBuckets; - } - - // this would be easier to maintain. - // Currently this doesn't work because histogramBuckets is readonly, but could hack using Reflection. - internal MetricPoint DeepCopy() - { - var other = (MetricPoint)this.MemberwiseClone(); - other.HistogramBuckets = this.HistogramBuckets?.DeepCopy(); - - return other; + return this.histogramBuckets; } internal void Update(long number) @@ -349,20 +314,20 @@ internal void Update(double number) case AggregationType.Histogram: { int i; - for (i = 0; i < this.HistogramBuckets.ExplicitBounds.Length; i++) + for (i = 0; i < this.histogramBuckets.ExplicitBounds.Length; i++) { // Upper bound is inclusive - if (number <= this.HistogramBuckets.ExplicitBounds[i]) + if (number <= this.histogramBuckets.ExplicitBounds[i]) { break; } } - lock (this.HistogramBuckets.LockObject) + lock (this.histogramBuckets.LockObject) { this.runningValue.AsLong++; - this.HistogramBuckets.RunningSum += number; - this.HistogramBuckets.RunningBucketCounts[i]++; + this.histogramBuckets.RunningSum += number; + this.histogramBuckets.RunningBucketCounts[i]++; } break; @@ -370,10 +335,10 @@ internal void Update(double number) case AggregationType.HistogramSumCount: { - lock (this.HistogramBuckets.LockObject) + lock (this.histogramBuckets.LockObject) { this.runningValue.AsLong++; - this.HistogramBuckets.RunningSum += number; + this.histogramBuckets.RunningSum += number; } break; @@ -495,22 +460,22 @@ internal void TakeSnapshot(bool outputDelta) case AggregationType.Histogram: { - lock (this.HistogramBuckets.LockObject) + lock (this.histogramBuckets.LockObject) { this.snapshotValue.AsLong = this.runningValue.AsLong; - this.HistogramBuckets.SnapshotSum = this.HistogramBuckets.RunningSum; + this.histogramBuckets.SnapshotSum = this.histogramBuckets.RunningSum; if (outputDelta) { this.runningValue.AsLong = 0; - this.HistogramBuckets.RunningSum = 0; + this.histogramBuckets.RunningSum = 0; } - for (int i = 0; i < this.HistogramBuckets.RunningBucketCounts.Length; i++) + for (int i = 0; i < this.histogramBuckets.RunningBucketCounts.Length; i++) { - this.HistogramBuckets.SnapshotBucketCounts[i] = this.HistogramBuckets.RunningBucketCounts[i]; + this.histogramBuckets.SnapshotBucketCounts[i] = this.histogramBuckets.RunningBucketCounts[i]; if (outputDelta) { - this.HistogramBuckets.RunningBucketCounts[i] = 0; + this.histogramBuckets.RunningBucketCounts[i] = 0; } } @@ -522,14 +487,14 @@ internal void TakeSnapshot(bool outputDelta) case AggregationType.HistogramSumCount: { - lock (this.HistogramBuckets.LockObject) + lock (this.histogramBuckets.LockObject) { this.snapshotValue.AsLong = this.runningValue.AsLong; - this.HistogramBuckets.SnapshotSum = this.HistogramBuckets.RunningSum; + this.histogramBuckets.SnapshotSum = this.histogramBuckets.RunningSum; if (outputDelta) { this.runningValue.AsLong = 0; - this.HistogramBuckets.RunningSum = 0; + this.histogramBuckets.RunningSum = 0; } this.MetricPointStatus = MetricPointStatus.NoCollectPending; diff --git a/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs b/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs index 926097728d8..5f5914be28e 100644 --- a/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs @@ -16,17 +16,15 @@ using System.Collections.Generic; using System.Diagnostics.Metrics; - using OpenTelemetry.Exporter; using OpenTelemetry.Tests; - using Xunit; namespace OpenTelemetry.Metrics.Tests { public class InMemoryExporterTests { - [Fact] + [Fact(Skip = "To be run after https://github.com/open-telemetry/opentelemetry-dotnet/issues/2361 is fixed")] public void InMemoryExporterShouldDeepCopyMetricPoints() { var exportedItems = new List(); @@ -58,7 +56,7 @@ public void InMemoryExporterShouldDeepCopyMetricPoints() meterProvider.ForceFlush(); - metric = exportedItems[0]; // Second Metric object is added to the collection at this point + metric = exportedItems[1]; // Second Metric object is added to the collection at this point metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator(); Assert.True(metricPointsEnumerator.MoveNext()); // One MetricPoint is emitted for the Metric var metricPointForSecondExport = metricPointsEnumerator.Current; @@ -67,48 +65,5 @@ public void InMemoryExporterShouldDeepCopyMetricPoints() // MetricPoint.LongValue for the first exporter metric should still be 10 Assert.Equal(10, metricPointForFirstExport.GetSumLong()); } - - [Fact] - public void Investigate_2361() - { - // https://github.com/open-telemetry/opentelemetry-dotnet/issues/2361 - - var exportedItems = new List(); - - using var meter = new Meter(Utils.GetCurrentMethodName()); - using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddInMemoryExporter(exportedItems) - .Build(); - - int i = 0; - var counterLong = meter.CreateObservableCounter( - "observable-counter", - () => ++i * 10); - - meterProvider.ForceFlush(); - Assert.Equal(1, i); // verify that callback is invoked when calling Flush - Assert.Single(exportedItems); // verify that List contains 1 item - var metricPoint1 = GetSingleMetricPoint(exportedItems[0]); - Assert.Equal(10, metricPoint1.GetSumLong()); - - meterProvider.ForceFlush(); - Assert.Equal(2, i); // verify that callback is invoked when calling Flush - Assert.Single(exportedItems); // verify that List contains 1 item - var metricPoint2 = GetSingleMetricPoint(exportedItems[0]); - Assert.Equal(20, metricPoint2.GetSumLong()); - - // Retest 1st item, this is expected to be unchanged. - Assert.Equal(10, metricPoint1.GetSumLong()); - } - - private static MetricPoint GetSingleMetricPoint(Metric metric) - { - var metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator(); - Assert.True(metricPointsEnumerator.MoveNext()); // One MetricPoint is emitted for the Metric - ref readonly var metricPoint = ref metricPointsEnumerator.Current; - Assert.False(metricPointsEnumerator.MoveNext()); - return metricPoint; - } } } From 752f6fd09da02d7d9c21c8178667549c02e2a46f Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Mon, 14 Mar 2022 11:06:23 -0700 Subject: [PATCH 03/15] includescopes --- docs/logs/customizing-the-sdk/Program.cs | 8 ++++++ docs/logs/customizing-the-sdk/README.md | 33 +++++++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/docs/logs/customizing-the-sdk/Program.cs b/docs/logs/customizing-the-sdk/Program.cs index a88618e6494..eac8cf998de 100644 --- a/docs/logs/customizing-the-sdk/Program.cs +++ b/docs/logs/customizing-the-sdk/Program.cs @@ -26,6 +26,7 @@ public static void Main() { builder.AddOpenTelemetry(options => { + options.IncludeScopes = true; options.AddConsoleExporter(); }); }); @@ -35,5 +36,12 @@ public static void Main() logger.LogInformation("Hello Information"); logger.LogWarning("Hello Warning"); logger.LogError("Hello Error"); + + // TESTING options.IncludeScopes + using (logger.BeginScope("My Scope 1")) + using (logger.BeginScope("My Scope 2")) + { + logger.LogInformation("Hello Information within scope"); + } } } diff --git a/docs/logs/customizing-the-sdk/README.md b/docs/logs/customizing-the-sdk/README.md index 5a225f713c2..d88bd653eb5 100644 --- a/docs/logs/customizing-the-sdk/README.md +++ b/docs/logs/customizing-the-sdk/README.md @@ -14,7 +14,38 @@ TODO ### IncludeScopes -TODO +A "scope" is an ILogger concept that can group a set of logical operations and +attach data to each log created as part of a set. + +`IncludeScopes` will include all scopes with the exported `LogRecord`. + +The snippet below demonstrates `IncludeScopes = true` using the +`ConsoleExporter`: + +``` +using(logger.BeginScope("My Scope 1")) +using(logger.BeginScope("My Scope 2")) +{ + logger.LogInformation("Hello Information within scope"); +} +``` + +``` +LogRecord.TraceId: 00000000000000000000000000000000 +LogRecord.SpanId: 0000000000000000 +LogRecord.Timestamp: 0001-01-01T00:00:00.0000000Z +LogRecord.EventId: 0 +LogRecord.EventName: +LogRecord.CategoryName: Program +LogRecord.LogLevel: Information +LogRecord.TraceFlags: None +LogRecord.State: Hello Information within scope +LogRecord.ScopeValues (Key:Value): +[Scope.0]: My Scope 1 +[Scope.1]: My Scope 2 +``` + +See [Program.cs](Program.cs) for complete example. ### IncludeFormattedMessage From e3779adfde3bfcdcea716184e59d366b7e64bec9 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Tue, 15 Mar 2022 14:54:11 -0700 Subject: [PATCH 04/15] prevent LoggerOptions from being modified after init. update tests --- .../Logs/OpenTelemetryLoggerOptions.cs | 2 + .../Logs/OpenTelemetryLoggerProvider.cs | 2 +- .../OpenTelemetry.Tests/Logs/LogRecordTest.cs | 549 +++++++++--------- 3 files changed, 278 insertions(+), 275 deletions(-) diff --git a/src/OpenTelemetry/Logs/OpenTelemetryLoggerOptions.cs b/src/OpenTelemetry/Logs/OpenTelemetryLoggerOptions.cs index 002fe742ae6..9dc48025f1a 100644 --- a/src/OpenTelemetry/Logs/OpenTelemetryLoggerOptions.cs +++ b/src/OpenTelemetry/Logs/OpenTelemetryLoggerOptions.cs @@ -78,5 +78,7 @@ public OpenTelemetryLoggerOptions SetResourceBuilder(ResourceBuilder resourceBui this.ResourceBuilder = resourceBuilder; return this; } + + internal OpenTelemetryLoggerOptions DeepCopy() => (OpenTelemetryLoggerOptions)this.MemberwiseClone(); } } diff --git a/src/OpenTelemetry/Logs/OpenTelemetryLoggerProvider.cs b/src/OpenTelemetry/Logs/OpenTelemetryLoggerProvider.cs index c79fa57270c..77d9d6206bb 100644 --- a/src/OpenTelemetry/Logs/OpenTelemetryLoggerProvider.cs +++ b/src/OpenTelemetry/Logs/OpenTelemetryLoggerProvider.cs @@ -48,7 +48,7 @@ internal OpenTelemetryLoggerProvider(OpenTelemetryLoggerOptions options) { Guard.ThrowIfNull(options); - this.Options = options; + this.Options = options.DeepCopy(); this.Resource = options.ResourceBuilder.Build(); foreach (var processor in options.Processors) diff --git a/test/OpenTelemetry.Tests/Logs/LogRecordTest.cs b/test/OpenTelemetry.Tests/Logs/LogRecordTest.cs index d462bcf9968..5ae6d1d300f 100644 --- a/test/OpenTelemetry.Tests/Logs/LogRecordTest.cs +++ b/test/OpenTelemetry.Tests/Logs/LogRecordTest.cs @@ -29,38 +29,15 @@ namespace OpenTelemetry.Logs.Tests { - public sealed class LogRecordTest : IDisposable + public sealed class LogRecordTest { - private readonly ILogger logger; - private readonly List exportedItems = new(); - private readonly ILoggerFactory loggerFactory; - private readonly BaseExportProcessor processor; - private readonly BaseExporter exporter; - private OpenTelemetryLoggerOptions options; - - public LogRecordTest() - { - this.exporter = new InMemoryExporter(this.exportedItems); - this.processor = new TestLogRecordProcessor(this.exporter); - this.loggerFactory = LoggerFactory.Create(builder => - { - builder.AddOpenTelemetry(options => - { - this.options = options; - options - .AddProcessor(this.processor); - }); - builder.AddFilter(typeof(LogRecordTest).FullName, LogLevel.Trace); - }); - - this.logger = this.loggerFactory.CreateLogger(); - } - [Fact] public void CheckCateogryNameForLog() { - this.logger.LogInformation("Log"); - var categoryName = this.exportedItems[0].CategoryName; + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + + logger.LogInformation("Log"); + var categoryName = exportedItems[0].CategoryName; Assert.Equal(typeof(LogRecordTest).FullName, categoryName); } @@ -74,19 +51,23 @@ public void CheckCateogryNameForLog() [InlineData(LogLevel.Critical)] public void CheckLogLevel(LogLevel logLevel) { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + var message = $"Log {logLevel}"; - this.logger.Log(logLevel, message); + logger.Log(logLevel, message); - var logLevelRecorded = this.exportedItems[0].LogLevel; + var logLevelRecorded = exportedItems[0].LogLevel; Assert.Equal(logLevel, logLevelRecorded); } [Fact] public void CheckStateForUnstructuredLog() { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + var message = "Hello, World!"; - this.logger.LogInformation(message); - var state = this.exportedItems[0].State as IReadOnlyList>; + logger.LogInformation(message); + var state = exportedItems[0].State as IReadOnlyList>; // state only has {OriginalFormat} Assert.Equal(1, state.Count); @@ -97,9 +78,11 @@ public void CheckStateForUnstructuredLog() [Fact] public void CheckStateForUnstructuredLogWithStringInterpolation() { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + var message = $"Hello from potato {0.99}."; - this.logger.LogInformation(message); - var state = this.exportedItems[0].State as IReadOnlyList>; + logger.LogInformation(message); + var state = exportedItems[0].State as IReadOnlyList>; // state only has {OriginalFormat} Assert.Equal(1, state.Count); @@ -110,9 +93,11 @@ public void CheckStateForUnstructuredLogWithStringInterpolation() [Fact] public void CheckStateForStructuredLogWithTemplate() { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + var message = "Hello from {name} {price}."; - this.logger.LogInformation(message, "tomato", 2.99); - var state = this.exportedItems[0].State as IReadOnlyList>; + logger.LogInformation(message, "tomato", 2.99); + var state = exportedItems[0].State as IReadOnlyList>; // state has name, price and {OriginalFormat} Assert.Equal(3, state.Count); @@ -135,9 +120,11 @@ public void CheckStateForStructuredLogWithTemplate() [Fact] public void CheckStateForStructuredLogWithStrongType() { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + var food = new Food { Name = "artichoke", Price = 3.99 }; - this.logger.LogInformation("{food}", food); - var state = this.exportedItems[0].State as IReadOnlyList>; + logger.LogInformation("{food}", food); + var state = exportedItems[0].State as IReadOnlyList>; // state has food and {OriginalFormat} Assert.Equal(2, state.Count); @@ -159,9 +146,11 @@ public void CheckStateForStructuredLogWithStrongType() [Fact] public void CheckStateForStructuredLogWithAnonymousType() { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + var anonymousType = new { Name = "pumpkin", Price = 5.99 }; - this.logger.LogInformation("{food}", anonymousType); - var state = this.exportedItems[0].State as IReadOnlyList>; + logger.LogInformation("{food}", anonymousType); + var state = exportedItems[0].State as IReadOnlyList>; // state has food and {OriginalFormat} Assert.Equal(2, state.Count); @@ -183,13 +172,15 @@ public void CheckStateForStructuredLogWithAnonymousType() [Fact] public void CheckStateForStrucutredLogWithGeneralType() { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + var food = new Dictionary { ["Name"] = "truffle", ["Price"] = 299.99, }; - this.logger.LogInformation("{food}", food); - var state = this.exportedItems[0].State as IReadOnlyList>; + logger.LogInformation("{food}", food); + var state = exportedItems[0].State as IReadOnlyList>; // state only has food and {OriginalFormat} Assert.Equal(2, state.Count); @@ -219,18 +210,20 @@ public void CheckStateForStrucutredLogWithGeneralType() [Fact] public void CheckStateForExceptionLogged() { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + var exceptionMessage = "Exception Message"; var exception = new Exception(exceptionMessage); var message = "Exception Occurred"; - this.logger.LogInformation(exception, message); + logger.LogInformation(exception, message); - var state = this.exportedItems[0].State; + var state = exportedItems[0].State; var itemCount = state.GetType().GetProperty("Count").GetValue(state); // state only has {OriginalFormat} Assert.Equal(1, itemCount); - var loggedException = this.exportedItems[0].Exception; + var loggedException = exportedItems[0].Exception; Assert.NotNull(loggedException); Assert.Equal(exceptionMessage, loggedException.Message); @@ -240,8 +233,10 @@ public void CheckStateForExceptionLogged() [Fact] public void CheckTraceIdForLogWithinDroppedActivity() { - this.logger.LogInformation("Log within a dropped activity"); - var logRecord = this.exportedItems[0]; + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + + logger.LogInformation("Log within a dropped activity"); + var logRecord = exportedItems[0]; Assert.Null(Activity.Current); Assert.Equal(default, logRecord.TraceId); @@ -252,6 +247,8 @@ public void CheckTraceIdForLogWithinDroppedActivity() [Fact] public void CheckTraceIdForLogWithinActivityMarkedAsRecordOnly() { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + var sampler = new RecordOnlySampler(); var exportedActivityList = new List(); var activitySourceName = "LogRecordTest"; @@ -264,8 +261,8 @@ public void CheckTraceIdForLogWithinActivityMarkedAsRecordOnly() using var activity = activitySource.StartActivity("Activity"); - this.logger.LogInformation("Log within activity marked as RecordOnly"); - var logRecord = this.exportedItems[0]; + logger.LogInformation("Log within activity marked as RecordOnly"); + var logRecord = exportedItems[0]; var currentActivity = Activity.Current; Assert.NotNull(Activity.Current); @@ -277,6 +274,8 @@ public void CheckTraceIdForLogWithinActivityMarkedAsRecordOnly() [Fact] public void CheckTraceIdForLogWithinActivityMarkedAsRecordAndSample() { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + var sampler = new AlwaysOnSampler(); var exportedActivityList = new List(); var activitySourceName = "LogRecordTest"; @@ -289,8 +288,8 @@ public void CheckTraceIdForLogWithinActivityMarkedAsRecordAndSample() using var activity = activitySource.StartActivity("Activity"); - this.logger.LogInformation("Log within activity marked as RecordAndSample"); - var logRecord = this.exportedItems[0]; + logger.LogInformation("Log within activity marked as RecordAndSample"); + var logRecord = exportedItems[0]; var currentActivity = Activity.Current; Assert.NotNull(Activity.Current); @@ -300,315 +299,317 @@ public void CheckTraceIdForLogWithinActivityMarkedAsRecordAndSample() } [Fact] - public void IncludeFormattedMessageTest() + public void VerifyIncludeFormattedMessage_False() { - this.logger.LogInformation("OpenTelemetry!"); - var logRecord = this.exportedItems[0]; + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeFormattedMessage = false); + + logger.LogInformation("OpenTelemetry!"); + var logRecord = exportedItems[0]; Assert.Null(logRecord.FormattedMessage); + } - this.options.IncludeFormattedMessage = true; - try - { - this.logger.LogInformation("OpenTelemetry!"); - logRecord = this.exportedItems[1]; - Assert.Equal("OpenTelemetry!", logRecord.FormattedMessage); + [Fact] + public void VerifyIncludeFormattedMessage_True() + { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeFormattedMessage = true); - this.logger.LogInformation("OpenTelemetry {Greeting} {Subject}!", "Hello", "World"); - logRecord = this.exportedItems[2]; - Assert.Equal("OpenTelemetry Hello World!", logRecord.FormattedMessage); - } - finally - { - this.options.IncludeFormattedMessage = false; - } + logger.LogInformation("OpenTelemetry!"); + var logRecord = exportedItems[0]; + Assert.Equal("OpenTelemetry!", logRecord.FormattedMessage); + + logger.LogInformation("OpenTelemetry {Greeting} {Subject}!", "Hello", "World"); + logRecord = exportedItems[1]; + Assert.Equal("OpenTelemetry Hello World!", logRecord.FormattedMessage); } [Fact] public void IncludeFormattedMessageTestWhenFormatterNull() { - this.logger.Log(LogLevel.Information, default, "Hello World!", null, null); - var logRecord = this.exportedItems[0]; + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeFormattedMessage = true); + + logger.Log(LogLevel.Information, default, "Hello World!", null, null); + var logRecord = exportedItems[0]; Assert.Null(logRecord.FormattedMessage); - this.options.IncludeFormattedMessage = true; - try - { - // Pass null as formatter function - this.logger.Log(LogLevel.Information, default, "Hello World!", null, null); - logRecord = this.exportedItems[1]; - Assert.Null(logRecord.FormattedMessage); - - var expectedFormattedMessage = "formatted message"; - this.logger.Log(LogLevel.Information, default, "Hello World!", null, (state, ex) => expectedFormattedMessage); - logRecord = this.exportedItems[2]; - Assert.Equal(expectedFormattedMessage, logRecord.FormattedMessage); - } - finally - { - this.options.IncludeFormattedMessage = false; - } + // Pass null as formatter function + logger.Log(LogLevel.Information, default, "Hello World!", null, null); + logRecord = exportedItems[1]; + Assert.Null(logRecord.FormattedMessage); + + var expectedFormattedMessage = "formatted message"; + logger.Log(LogLevel.Information, default, "Hello World!", null, (state, ex) => expectedFormattedMessage); + logRecord = exportedItems[2]; + Assert.Equal(expectedFormattedMessage, logRecord.FormattedMessage); } [Fact] - public void IncludeScopesTest() + public void VerifyIncludeScopes_False() { - using var scope = this.logger.BeginScope("string_scope"); + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeScopes = false); - this.logger.LogInformation("OpenTelemetry!"); - var logRecord = this.exportedItems[0]; + using var scope = logger.BeginScope("string_scope"); + + logger.LogInformation("OpenTelemetry!"); + var logRecord = exportedItems[0]; List scopes = new List(); logRecord.ForEachScope((scope, state) => scopes.Add(scope.Scope), null); Assert.Empty(scopes); + } - this.options.IncludeScopes = true; - try - { - this.logger.LogInformation("OpenTelemetry!"); - logRecord = this.exportedItems[1]; + [Fact] + public void VerifyIncludeScopes_True() + { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeScopes = true); - int reachedDepth = -1; - logRecord.ForEachScope( - (scope, state) => - { - reachedDepth++; - scopes.Add(scope.Scope); - foreach (KeyValuePair item in scope) - { - Assert.Equal(string.Empty, item.Key); - Assert.Equal("string_scope", item.Value); - } - }, - null); - Assert.Single(scopes); - Assert.Equal(0, reachedDepth); - Assert.Equal("string_scope", scopes[0]); + using var scope = logger.BeginScope("string_scope"); - scopes.Clear(); + logger.LogInformation("OpenTelemetry!"); + var logRecord = exportedItems[0]; - List> expectedScope2 = new List> + List scopes = new List(); + + logger.LogInformation("OpenTelemetry!"); + logRecord = exportedItems[1]; + + int reachedDepth = -1; + logRecord.ForEachScope( + (scope, state) => { - new KeyValuePair("item1", "value1"), - new KeyValuePair("item2", "value2"), - }; - using var scope2 = this.logger.BeginScope(expectedScope2); + reachedDepth++; + scopes.Add(scope.Scope); + foreach (KeyValuePair item in scope) + { + Assert.Equal(string.Empty, item.Key); + Assert.Equal("string_scope", item.Value); + } + }, + null); + Assert.Single(scopes); + Assert.Equal(0, reachedDepth); + Assert.Equal("string_scope", scopes[0]); + + scopes.Clear(); + + List> expectedScope2 = new List> + { + new KeyValuePair("item1", "value1"), + new KeyValuePair("item2", "value2"), + }; + using var scope2 = logger.BeginScope(expectedScope2); - this.logger.LogInformation("OpenTelemetry!"); - logRecord = this.exportedItems[2]; + logger.LogInformation("OpenTelemetry!"); + logRecord = exportedItems[2]; - reachedDepth = -1; - logRecord.ForEachScope( - (scope, state) => + reachedDepth = -1; + logRecord.ForEachScope( + (scope, state) => + { + scopes.Add(scope.Scope); + if (reachedDepth++ == 1) { - scopes.Add(scope.Scope); - if (reachedDepth++ == 1) + foreach (KeyValuePair item in scope) { - foreach (KeyValuePair item in scope) - { - Assert.Contains(item, expectedScope2); - } + Assert.Contains(item, expectedScope2); } - }, - null); - Assert.Equal(2, scopes.Count); - Assert.Equal(1, reachedDepth); - Assert.Equal("string_scope", scopes[0]); - Assert.Same(expectedScope2, scopes[1]); + } + }, + null); + Assert.Equal(2, scopes.Count); + Assert.Equal(1, reachedDepth); + Assert.Equal("string_scope", scopes[0]); + Assert.Same(expectedScope2, scopes[1]); - scopes.Clear(); + scopes.Clear(); - KeyValuePair[] expectedScope3 = new KeyValuePair[] - { - new KeyValuePair("item3", "value3"), - new KeyValuePair("item4", "value4"), - }; - using var scope3 = this.logger.BeginScope(expectedScope3); + KeyValuePair[] expectedScope3 = new KeyValuePair[] + { + new KeyValuePair("item3", "value3"), + new KeyValuePair("item4", "value4"), + }; + using var scope3 = logger.BeginScope(expectedScope3); - this.logger.LogInformation("OpenTelemetry!"); - logRecord = this.exportedItems[3]; + logger.LogInformation("OpenTelemetry!"); + logRecord = exportedItems[3]; - reachedDepth = -1; - logRecord.ForEachScope( - (scope, state) => + reachedDepth = -1; + logRecord.ForEachScope( + (scope, state) => + { + scopes.Add(scope.Scope); + if (reachedDepth++ == 2) { - scopes.Add(scope.Scope); - if (reachedDepth++ == 2) + foreach (KeyValuePair item in scope) { - foreach (KeyValuePair item in scope) - { - Assert.Contains(item, expectedScope3); - } + Assert.Contains(item, expectedScope3); } - }, - null); - Assert.Equal(3, scopes.Count); - Assert.Equal(2, reachedDepth); - Assert.Equal("string_scope", scopes[0]); - Assert.Same(expectedScope2, scopes[1]); - Assert.Same(expectedScope3, scopes[2]); - } - finally - { - this.options.IncludeScopes = false; - } + } + }, + null); + Assert.Equal(3, scopes.Count); + Assert.Equal(2, reachedDepth); + Assert.Equal("string_scope", scopes[0]); + Assert.Same(expectedScope2, scopes[1]); + Assert.Same(expectedScope3, scopes[2]); } [Fact] - public void ParseStateValuesUsingStandardExtensionsTest() + public void VerifyParseStateValues_False_UsingStandardExtensions() { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = false); + // Tests state parsing with standard extensions. - this.logger.LogInformation("{Product} {Year}!", "OpenTelemetry", 2021); - var logRecord = this.exportedItems[0]; + logger.LogInformation("{Product} {Year}!", "OpenTelemetry", 2021); + var logRecord = exportedItems[0]; Assert.NotNull(logRecord.State); Assert.Null(logRecord.StateValues); + } - this.options.ParseStateValues = true; - try - { - var complex = new { Property = "Value" }; + [Fact] + public void VerifyParseStateValues_True_UsingStandardExtensions() + { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); - this.logger.LogInformation("{Product} {Year} {Complex}!", "OpenTelemetry", 2021, complex); - logRecord = this.exportedItems[1]; + // Tests state parsing with standard extensions. - Assert.Null(logRecord.State); - Assert.NotNull(logRecord.StateValues); - Assert.Equal(4, logRecord.StateValues.Count); - Assert.Equal(new KeyValuePair("Product", "OpenTelemetry"), logRecord.StateValues[0]); - Assert.Equal(new KeyValuePair("Year", 2021), logRecord.StateValues[1]); - Assert.Equal(new KeyValuePair("{OriginalFormat}", "{Product} {Year} {Complex}!"), logRecord.StateValues[3]); + logger.LogInformation("{Product} {Year}!", "OpenTelemetry", 2021); + var logRecord = exportedItems[0]; - KeyValuePair actualComplex = logRecord.StateValues[2]; - Assert.Equal("Complex", actualComplex.Key); - Assert.Same(complex, actualComplex.Value); - } - finally - { - this.options.ParseStateValues = false; - } + Assert.Null(logRecord.State); + Assert.NotNull(logRecord.StateValues); + Assert.Equal(3, logRecord.StateValues.Count); + Assert.Equal(new KeyValuePair("Product", "OpenTelemetry"), logRecord.StateValues[0]); + Assert.Equal(new KeyValuePair("Year", 2021), logRecord.StateValues[1]); + Assert.Equal(new KeyValuePair("{OriginalFormat}", "{Product} {Year}!"), logRecord.StateValues[2]); + + var complex = new { Property = "Value" }; + + logger.LogInformation("{Product} {Year} {Complex}!", "OpenTelemetry", 2021, complex); + logRecord = exportedItems[1]; + + Assert.Null(logRecord.State); + Assert.NotNull(logRecord.StateValues); + Assert.Equal(4, logRecord.StateValues.Count); + Assert.Equal(new KeyValuePair("Product", "OpenTelemetry"), logRecord.StateValues[0]); + Assert.Equal(new KeyValuePair("Year", 2021), logRecord.StateValues[1]); + Assert.Equal(new KeyValuePair("{OriginalFormat}", "{Product} {Year} {Complex}!"), logRecord.StateValues[3]); + + KeyValuePair actualComplex = logRecord.StateValues[2]; + Assert.Equal("Complex", actualComplex.Key); + Assert.Same(complex, actualComplex.Value); } [Fact] public void ParseStateValuesUsingStructTest() { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); + // Tests struct IReadOnlyList> parse path. - this.options.ParseStateValues = true; - try - { - this.logger.Log( - LogLevel.Information, - 0, - new StructState(new KeyValuePair("Key1", "Value1")), - null, - (s, e) => "OpenTelemetry!"); - var logRecord = this.exportedItems[0]; - - Assert.Null(logRecord.State); - Assert.NotNull(logRecord.StateValues); - Assert.Equal(1, logRecord.StateValues.Count); - Assert.Equal(new KeyValuePair("Key1", "Value1"), logRecord.StateValues[0]); - } - finally - { - this.options.ParseStateValues = false; - } + logger.Log( + LogLevel.Information, + 0, + new StructState(new KeyValuePair("Key1", "Value1")), + null, + (s, e) => "OpenTelemetry!"); + var logRecord = exportedItems[0]; + + Assert.Null(logRecord.State); + Assert.NotNull(logRecord.StateValues); + Assert.Equal(1, logRecord.StateValues.Count); + Assert.Equal(new KeyValuePair("Key1", "Value1"), logRecord.StateValues[0]); } [Fact] public void ParseStateValuesUsingListTest() { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); + // Tests ref IReadOnlyList> parse path. - this.options.ParseStateValues = true; - try - { - this.logger.Log( - LogLevel.Information, - 0, - new List> { new KeyValuePair("Key1", "Value1") }, - null, - (s, e) => "OpenTelemetry!"); - var logRecord = this.exportedItems[0]; - - Assert.Null(logRecord.State); - Assert.NotNull(logRecord.StateValues); - Assert.Equal(1, logRecord.StateValues.Count); - Assert.Equal(new KeyValuePair("Key1", "Value1"), logRecord.StateValues[0]); - } - finally - { - this.options.ParseStateValues = false; - } + logger.Log( + LogLevel.Information, + 0, + new List> { new KeyValuePair("Key1", "Value1") }, + null, + (s, e) => "OpenTelemetry!"); + var logRecord = exportedItems[0]; + + Assert.Null(logRecord.State); + Assert.NotNull(logRecord.StateValues); + Assert.Equal(1, logRecord.StateValues.Count); + Assert.Equal(new KeyValuePair("Key1", "Value1"), logRecord.StateValues[0]); } [Fact] public void ParseStateValuesUsingIEnumerableTest() { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); + // Tests IEnumerable> parse path. - this.options.ParseStateValues = true; - try - { - this.logger.Log( - LogLevel.Information, - 0, - new ListState(new KeyValuePair("Key1", "Value1")), - null, - (s, e) => "OpenTelemetry!"); - var logRecord = this.exportedItems[0]; - - Assert.Null(logRecord.State); - Assert.NotNull(logRecord.StateValues); - Assert.Equal(1, logRecord.StateValues.Count); - Assert.Equal(new KeyValuePair("Key1", "Value1"), logRecord.StateValues[0]); - } - finally - { - this.options.ParseStateValues = false; - } + logger.Log( + LogLevel.Information, + 0, + new ListState(new KeyValuePair("Key1", "Value1")), + null, + (s, e) => "OpenTelemetry!"); + var logRecord = exportedItems[0]; + + Assert.Null(logRecord.State); + Assert.NotNull(logRecord.StateValues); + Assert.Equal(1, logRecord.StateValues.Count); + Assert.Equal(new KeyValuePair("Key1", "Value1"), logRecord.StateValues[0]); } [Fact] public void ParseStateValuesUsingCustomTest() { + InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); + // Tests unknown state parse path. - this.options.ParseStateValues = true; - try + CustomState state = new CustomState { - CustomState state = new CustomState - { - Property = "Value", - }; + Property = "Value", + }; - this.logger.Log( - LogLevel.Information, - 0, - state, - null, - (s, e) => "OpenTelemetry!"); - var logRecord = this.exportedItems[0]; + logger.Log( + LogLevel.Information, + 0, + state, + null, + (s, e) => "OpenTelemetry!"); + var logRecord = exportedItems[0]; - Assert.Null(logRecord.State); - Assert.NotNull(logRecord.StateValues); - Assert.Equal(1, logRecord.StateValues.Count); + Assert.Null(logRecord.State); + Assert.NotNull(logRecord.StateValues); + Assert.Equal(1, logRecord.StateValues.Count); - KeyValuePair actualState = logRecord.StateValues[0]; + KeyValuePair actualState = logRecord.StateValues[0]; - Assert.Equal(string.Empty, actualState.Key); - Assert.Same(state, actualState.Value); - } - finally - { - this.options.ParseStateValues = false; - } + Assert.Equal(string.Empty, actualState.Key); + Assert.Same(state, actualState.Value); } - public void Dispose() + private static void InitializeLoggerFactory(out ILogger logger, out List exportedItems, Action configure = null) { - this.loggerFactory?.Dispose(); + exportedItems = new List(); + var exporter = new InMemoryExporter(exportedItems); + var processor = new TestLogRecordProcessor(exporter); + var loggerFactory = LoggerFactory.Create(builder => + { + builder.AddOpenTelemetry(options => + { + configure?.Invoke(options); + options.AddProcessor(processor); + }); + builder.AddFilter(typeof(LogRecordTest).FullName, LogLevel.Trace); + }); + + logger = loggerFactory.CreateLogger(); } internal struct Food From 797f991b35b5507b85e41a157db0b58a6a25a732 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Tue, 15 Mar 2022 14:57:34 -0700 Subject: [PATCH 05/15] REMOVE --- docs/logs/customizing-the-sdk/Program.cs | 8 ------ docs/logs/customizing-the-sdk/README.md | 33 +----------------------- 2 files changed, 1 insertion(+), 40 deletions(-) diff --git a/docs/logs/customizing-the-sdk/Program.cs b/docs/logs/customizing-the-sdk/Program.cs index eac8cf998de..a88618e6494 100644 --- a/docs/logs/customizing-the-sdk/Program.cs +++ b/docs/logs/customizing-the-sdk/Program.cs @@ -26,7 +26,6 @@ public static void Main() { builder.AddOpenTelemetry(options => { - options.IncludeScopes = true; options.AddConsoleExporter(); }); }); @@ -36,12 +35,5 @@ public static void Main() logger.LogInformation("Hello Information"); logger.LogWarning("Hello Warning"); logger.LogError("Hello Error"); - - // TESTING options.IncludeScopes - using (logger.BeginScope("My Scope 1")) - using (logger.BeginScope("My Scope 2")) - { - logger.LogInformation("Hello Information within scope"); - } } } diff --git a/docs/logs/customizing-the-sdk/README.md b/docs/logs/customizing-the-sdk/README.md index 532b8fbd617..bd69902b331 100644 --- a/docs/logs/customizing-the-sdk/README.md +++ b/docs/logs/customizing-the-sdk/README.md @@ -14,38 +14,7 @@ TODO ### IncludeScopes -A "scope" is an ILogger concept that can group a set of logical operations and -attach data to each log created as part of a set. - -`IncludeScopes` will include all scopes with the exported `LogRecord`. - -The snippet below demonstrates `IncludeScopes = true` using the -`ConsoleExporter`: - -``` -using(logger.BeginScope("My Scope 1")) -using(logger.BeginScope("My Scope 2")) -{ - logger.LogInformation("Hello Information within scope"); -} -``` - -``` -LogRecord.TraceId: 00000000000000000000000000000000 -LogRecord.SpanId: 0000000000000000 -LogRecord.Timestamp: 0001-01-01T00:00:00.0000000Z -LogRecord.EventId: 0 -LogRecord.EventName: -LogRecord.CategoryName: Program -LogRecord.LogLevel: Information -LogRecord.TraceFlags: None -LogRecord.State: Hello Information within scope -LogRecord.ScopeValues (Key:Value): -[Scope.0]: My Scope 1 -[Scope.1]: My Scope 2 -``` - -See [Program.cs](Program.cs) for complete example. +TODO ### IncludeFormattedMessage From 4a29ff1a41486b252ef6b3cbadef98032b5ce15c Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Tue, 15 Mar 2022 17:08:10 -0700 Subject: [PATCH 06/15] copy the values out of the Options instance --- src/OpenTelemetry/Logs/OpenTelemetryLogger.cs | 10 ++++------ src/OpenTelemetry/Logs/OpenTelemetryLoggerProvider.cs | 9 +++++++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/OpenTelemetry/Logs/OpenTelemetryLogger.cs b/src/OpenTelemetry/Logs/OpenTelemetryLogger.cs index 574dfbefb55..c8f063de0b7 100644 --- a/src/OpenTelemetry/Logs/OpenTelemetryLogger.cs +++ b/src/OpenTelemetry/Logs/OpenTelemetryLogger.cs @@ -49,18 +49,16 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except var processor = this.provider.Processor; if (processor != null) { - var options = this.provider.Options; - var record = new LogRecord( - options.IncludeScopes ? this.ScopeProvider : null, + this.provider.IncludeScopes ? this.ScopeProvider : null, DateTime.UtcNow, this.categoryName, logLevel, eventId, - options.IncludeFormattedMessage ? formatter?.Invoke(state, exception) : null, - options.ParseStateValues ? null : state, + this.provider.IncludeFormattedMessage ? formatter?.Invoke(state, exception) : null, + this.provider.ParseStateValues ? null : state, exception, - options.ParseStateValues ? this.ParseState(state) : null); + this.provider.ParseStateValues ? this.ParseState(state) : null); processor.OnEnd(record); diff --git a/src/OpenTelemetry/Logs/OpenTelemetryLoggerProvider.cs b/src/OpenTelemetry/Logs/OpenTelemetryLoggerProvider.cs index 77d9d6206bb..372c3d04ed5 100644 --- a/src/OpenTelemetry/Logs/OpenTelemetryLoggerProvider.cs +++ b/src/OpenTelemetry/Logs/OpenTelemetryLoggerProvider.cs @@ -25,7 +25,9 @@ namespace OpenTelemetry.Logs [ProviderAlias("OpenTelemetry")] public class OpenTelemetryLoggerProvider : BaseProvider, ILoggerProvider, ISupportExternalScope { - internal readonly OpenTelemetryLoggerOptions Options; + internal readonly bool IncludeScopes; + internal readonly bool IncludeFormattedMessage; + internal readonly bool ParseStateValues; internal BaseProcessor Processor; internal Resource Resource; private readonly Hashtable loggers = new(); @@ -48,7 +50,10 @@ internal OpenTelemetryLoggerProvider(OpenTelemetryLoggerOptions options) { Guard.ThrowIfNull(options); - this.Options = options.DeepCopy(); + this.IncludeScopes = options.IncludeScopes; + this.IncludeFormattedMessage = options.IncludeFormattedMessage; + this.ParseStateValues = options.ParseStateValues; + this.Resource = options.ResourceBuilder.Build(); foreach (var processor in options.Processors) From c6f2133f9d98c08db2c6d4357f0679ec41b32485 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Tue, 15 Mar 2022 17:08:49 -0700 Subject: [PATCH 07/15] remove --- src/OpenTelemetry/Logs/OpenTelemetryLoggerOptions.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/OpenTelemetry/Logs/OpenTelemetryLoggerOptions.cs b/src/OpenTelemetry/Logs/OpenTelemetryLoggerOptions.cs index 9dc48025f1a..002fe742ae6 100644 --- a/src/OpenTelemetry/Logs/OpenTelemetryLoggerOptions.cs +++ b/src/OpenTelemetry/Logs/OpenTelemetryLoggerOptions.cs @@ -78,7 +78,5 @@ public OpenTelemetryLoggerOptions SetResourceBuilder(ResourceBuilder resourceBui this.ResourceBuilder = resourceBuilder; return this; } - - internal OpenTelemetryLoggerOptions DeepCopy() => (OpenTelemetryLoggerOptions)this.MemberwiseClone(); } } From 9a9b7c0af399caa7433339d1fe33d8220fc5e0ff Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Thu, 17 Mar 2022 09:22:30 -0700 Subject: [PATCH 08/15] new test to verify options cannot be changed --- .../Logs/LoggerOptionsTest.cs | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 test/OpenTelemetry.Tests/Logs/LoggerOptionsTest.cs diff --git a/test/OpenTelemetry.Tests/Logs/LoggerOptionsTest.cs b/test/OpenTelemetry.Tests/Logs/LoggerOptionsTest.cs new file mode 100644 index 00000000000..7ede4cc9b20 --- /dev/null +++ b/test/OpenTelemetry.Tests/Logs/LoggerOptionsTest.cs @@ -0,0 +1,54 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +#if !NET461 + +using Xunit; + +namespace OpenTelemetry.Logs.Tests +{ + public sealed class LoggerOptionsTest + { + [Theory] + [InlineData(true)] + [InlineData(false)] + public void VerifyOptionsCannotBeChangedAfterInit(bool initialValue) + { + var options = new OpenTelemetryLoggerOptions + { + IncludeFormattedMessage = initialValue, + IncludeScopes = initialValue, + ParseStateValues = initialValue, + }; + var processor = new OpenTelemetryLoggerProvider(options); + + // Verify initial set + Assert.Equal(initialValue, processor.IncludeFormattedMessage); + Assert.Equal(initialValue, processor.IncludeScopes); + Assert.Equal(initialValue, processor.ParseStateValues); + + // Attempt to change value + options.IncludeFormattedMessage = !initialValue; + options.IncludeScopes = !initialValue; + options.ParseStateValues = !initialValue; + + // Verify processor is unchanged + Assert.Equal(initialValue, processor.IncludeFormattedMessage); + Assert.Equal(initialValue, processor.IncludeScopes); + Assert.Equal(initialValue, processor.ParseStateValues); + } + } +} +#endif From b3ab1f943f96b4a55ff7f8b520e9de48f4a651a5 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Fri, 18 Mar 2022 09:45:14 -0700 Subject: [PATCH 09/15] changelog --- src/OpenTelemetry/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 99271b782cd..5faddb591c0 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -20,6 +20,10 @@ to `-1`. ([#3038](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3038)) +* `OpenTelemetryLoggerProvider` now copies values from `OpenTelemetryLoggerOptions` + to prevent options from being changed after the Provider is initialized. + ([#3055](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3055)) + ## 1.2.0-rc3 Released 2022-Mar-04 From dbb9645be1172ba227e2da8b03f8009fcf2a152f Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Fri, 18 Mar 2022 09:56:40 -0700 Subject: [PATCH 10/15] update changelog --- src/OpenTelemetry/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 5faddb591c0..599bd5bb4f9 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -20,8 +20,8 @@ to `-1`. ([#3038](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3038)) -* `OpenTelemetryLoggerProvider` now copies values from `OpenTelemetryLoggerOptions` - to prevent options from being changed after the Provider is initialized. +* `OpenTelemetryLoggerProvider` now copies values from `OpenTelemetryLoggerOptions`. + This prevents changes to the Provider after being initialized. ([#3055](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3055)) ## 1.2.0-rc3 From c659f74415f83c3eaa0bc11b108f5eae0d49dd1a Mon Sep 17 00:00:00 2001 From: Timothy Mothra Date: Fri, 18 Mar 2022 10:31:48 -0700 Subject: [PATCH 11/15] Update src/OpenTelemetry/CHANGELOG.md Co-authored-by: Cijo Thomas --- src/OpenTelemetry/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 582c3146528..79d2fbe5be7 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -25,7 +25,7 @@ ([#3065](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3065)) * `OpenTelemetryLoggerProvider` now copies values from `OpenTelemetryLoggerOptions`. - This prevents changes to the Provider after being initialized. + Provider will not respect changes made after the LoggerFactory is built. ([#3055](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3055)) ## 1.2.0-rc3 From 767beafc9eac7b32a1d0c7cc554004ab985cc917 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Mon, 28 Mar 2022 14:53:55 -0700 Subject: [PATCH 12/15] update --- src/OpenTelemetry/Logs/OpenTelemetryLogger.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/OpenTelemetry/Logs/OpenTelemetryLogger.cs b/src/OpenTelemetry/Logs/OpenTelemetryLogger.cs index c8f063de0b7..9fc41beac90 100644 --- a/src/OpenTelemetry/Logs/OpenTelemetryLogger.cs +++ b/src/OpenTelemetry/Logs/OpenTelemetryLogger.cs @@ -46,19 +46,20 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except return; } - var processor = this.provider.Processor; + var provider = this.provider; + var processor = provider.Processor; if (processor != null) { var record = new LogRecord( - this.provider.IncludeScopes ? this.ScopeProvider : null, + provider.IncludeScopes ? this.ScopeProvider : null, DateTime.UtcNow, this.categoryName, logLevel, eventId, - this.provider.IncludeFormattedMessage ? formatter?.Invoke(state, exception) : null, - this.provider.ParseStateValues ? null : state, + provider.IncludeFormattedMessage ? formatter?.Invoke(state, exception) : null, + provider.ParseStateValues ? null : state, exception, - this.provider.ParseStateValues ? this.ParseState(state) : null); + provider.ParseStateValues ? this.ParseState(state) : null); processor.OnEnd(record); From 5aef46089e3c988e35df05974003aded7299f5e1 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Tue, 29 Mar 2022 12:01:28 -0700 Subject: [PATCH 13/15] addressing pr comments. --- .../OpenTelemetry.Tests/Logs/LogRecordTest.cs | 49 ++++++++++--------- .../Logs/LoggerOptionsTest.cs | 14 +++--- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/test/OpenTelemetry.Tests/Logs/LogRecordTest.cs b/test/OpenTelemetry.Tests/Logs/LogRecordTest.cs index 5ae6d1d300f..4bb4c30dc52 100644 --- a/test/OpenTelemetry.Tests/Logs/LogRecordTest.cs +++ b/test/OpenTelemetry.Tests/Logs/LogRecordTest.cs @@ -34,7 +34,7 @@ public sealed class LogRecordTest [Fact] public void CheckCateogryNameForLog() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); logger.LogInformation("Log"); var categoryName = exportedItems[0].CategoryName; @@ -51,7 +51,7 @@ public void CheckCateogryNameForLog() [InlineData(LogLevel.Critical)] public void CheckLogLevel(LogLevel logLevel) { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); var message = $"Log {logLevel}"; logger.Log(logLevel, message); @@ -63,7 +63,7 @@ public void CheckLogLevel(LogLevel logLevel) [Fact] public void CheckStateForUnstructuredLog() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); var message = "Hello, World!"; logger.LogInformation(message); @@ -78,7 +78,7 @@ public void CheckStateForUnstructuredLog() [Fact] public void CheckStateForUnstructuredLogWithStringInterpolation() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); var message = $"Hello from potato {0.99}."; logger.LogInformation(message); @@ -93,7 +93,7 @@ public void CheckStateForUnstructuredLogWithStringInterpolation() [Fact] public void CheckStateForStructuredLogWithTemplate() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); var message = "Hello from {name} {price}."; logger.LogInformation(message, "tomato", 2.99); @@ -120,7 +120,7 @@ public void CheckStateForStructuredLogWithTemplate() [Fact] public void CheckStateForStructuredLogWithStrongType() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); var food = new Food { Name = "artichoke", Price = 3.99 }; logger.LogInformation("{food}", food); @@ -146,7 +146,7 @@ public void CheckStateForStructuredLogWithStrongType() [Fact] public void CheckStateForStructuredLogWithAnonymousType() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); var anonymousType = new { Name = "pumpkin", Price = 5.99 }; logger.LogInformation("{food}", anonymousType); @@ -172,7 +172,7 @@ public void CheckStateForStructuredLogWithAnonymousType() [Fact] public void CheckStateForStrucutredLogWithGeneralType() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); var food = new Dictionary { @@ -210,7 +210,7 @@ public void CheckStateForStrucutredLogWithGeneralType() [Fact] public void CheckStateForExceptionLogged() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); var exceptionMessage = "Exception Message"; var exception = new Exception(exceptionMessage); @@ -233,7 +233,7 @@ public void CheckStateForExceptionLogged() [Fact] public void CheckTraceIdForLogWithinDroppedActivity() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); logger.LogInformation("Log within a dropped activity"); var logRecord = exportedItems[0]; @@ -247,7 +247,7 @@ public void CheckTraceIdForLogWithinDroppedActivity() [Fact] public void CheckTraceIdForLogWithinActivityMarkedAsRecordOnly() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); var sampler = new RecordOnlySampler(); var exportedActivityList = new List(); @@ -274,7 +274,7 @@ public void CheckTraceIdForLogWithinActivityMarkedAsRecordOnly() [Fact] public void CheckTraceIdForLogWithinActivityMarkedAsRecordAndSample() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); var sampler = new AlwaysOnSampler(); var exportedActivityList = new List(); @@ -301,7 +301,7 @@ public void CheckTraceIdForLogWithinActivityMarkedAsRecordAndSample() [Fact] public void VerifyIncludeFormattedMessage_False() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeFormattedMessage = false); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeFormattedMessage = false); logger.LogInformation("OpenTelemetry!"); var logRecord = exportedItems[0]; @@ -311,7 +311,7 @@ public void VerifyIncludeFormattedMessage_False() [Fact] public void VerifyIncludeFormattedMessage_True() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeFormattedMessage = true); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeFormattedMessage = true); logger.LogInformation("OpenTelemetry!"); var logRecord = exportedItems[0]; @@ -325,7 +325,7 @@ public void VerifyIncludeFormattedMessage_True() [Fact] public void IncludeFormattedMessageTestWhenFormatterNull() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeFormattedMessage = true); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeFormattedMessage = true); logger.Log(LogLevel.Information, default, "Hello World!", null, null); var logRecord = exportedItems[0]; @@ -345,7 +345,7 @@ public void IncludeFormattedMessageTestWhenFormatterNull() [Fact] public void VerifyIncludeScopes_False() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeScopes = false); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeScopes = false); using var scope = logger.BeginScope("string_scope"); @@ -360,7 +360,7 @@ public void VerifyIncludeScopes_False() [Fact] public void VerifyIncludeScopes_True() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeScopes = true); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeScopes = true); using var scope = logger.BeginScope("string_scope"); @@ -456,7 +456,7 @@ public void VerifyIncludeScopes_True() [Fact] public void VerifyParseStateValues_False_UsingStandardExtensions() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = false); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = false); // Tests state parsing with standard extensions. @@ -470,7 +470,7 @@ public void VerifyParseStateValues_False_UsingStandardExtensions() [Fact] public void VerifyParseStateValues_True_UsingStandardExtensions() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); // Tests state parsing with standard extensions. @@ -504,7 +504,7 @@ public void VerifyParseStateValues_True_UsingStandardExtensions() [Fact] public void ParseStateValuesUsingStructTest() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); // Tests struct IReadOnlyList> parse path. @@ -525,7 +525,7 @@ public void ParseStateValuesUsingStructTest() [Fact] public void ParseStateValuesUsingListTest() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); // Tests ref IReadOnlyList> parse path. @@ -546,7 +546,7 @@ public void ParseStateValuesUsingListTest() [Fact] public void ParseStateValuesUsingIEnumerableTest() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); // Tests IEnumerable> parse path. @@ -567,7 +567,7 @@ public void ParseStateValuesUsingIEnumerableTest() [Fact] public void ParseStateValuesUsingCustomTest() { - InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); + using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); // Tests unknown state parse path. @@ -594,7 +594,7 @@ public void ParseStateValuesUsingCustomTest() Assert.Same(state, actualState.Value); } - private static void InitializeLoggerFactory(out ILogger logger, out List exportedItems, Action configure = null) + private static ILoggerFactory InitializeLoggerFactory(out ILogger logger, out List exportedItems, Action configure = null) { exportedItems = new List(); var exporter = new InMemoryExporter(exportedItems); @@ -610,6 +610,7 @@ private static void InitializeLoggerFactory(out ILogger logger, out List(); + return loggerFactory; } internal struct Food diff --git a/test/OpenTelemetry.Tests/Logs/LoggerOptionsTest.cs b/test/OpenTelemetry.Tests/Logs/LoggerOptionsTest.cs index 7ede4cc9b20..14ccaf92db6 100644 --- a/test/OpenTelemetry.Tests/Logs/LoggerOptionsTest.cs +++ b/test/OpenTelemetry.Tests/Logs/LoggerOptionsTest.cs @@ -32,12 +32,12 @@ public void VerifyOptionsCannotBeChangedAfterInit(bool initialValue) IncludeScopes = initialValue, ParseStateValues = initialValue, }; - var processor = new OpenTelemetryLoggerProvider(options); + var provider = new OpenTelemetryLoggerProvider(options); // Verify initial set - Assert.Equal(initialValue, processor.IncludeFormattedMessage); - Assert.Equal(initialValue, processor.IncludeScopes); - Assert.Equal(initialValue, processor.ParseStateValues); + Assert.Equal(initialValue, provider.IncludeFormattedMessage); + Assert.Equal(initialValue, provider.IncludeScopes); + Assert.Equal(initialValue, provider.ParseStateValues); // Attempt to change value options.IncludeFormattedMessage = !initialValue; @@ -45,9 +45,9 @@ public void VerifyOptionsCannotBeChangedAfterInit(bool initialValue) options.ParseStateValues = !initialValue; // Verify processor is unchanged - Assert.Equal(initialValue, processor.IncludeFormattedMessage); - Assert.Equal(initialValue, processor.IncludeScopes); - Assert.Equal(initialValue, processor.ParseStateValues); + Assert.Equal(initialValue, provider.IncludeFormattedMessage); + Assert.Equal(initialValue, provider.IncludeScopes); + Assert.Equal(initialValue, provider.ParseStateValues); } } } From 99f6b9ed7fe71c4d46136a3fdad744281885cf42 Mon Sep 17 00:00:00 2001 From: TimothyMothra Date: Tue, 29 Mar 2022 13:20:06 -0700 Subject: [PATCH 14/15] remove CreateLogger from InitializeLoggerFactory --- .../OpenTelemetry.Tests/Logs/LogRecordTest.cs | 76 ++++++++++++------- 1 file changed, 48 insertions(+), 28 deletions(-) diff --git a/test/OpenTelemetry.Tests/Logs/LogRecordTest.cs b/test/OpenTelemetry.Tests/Logs/LogRecordTest.cs index a5b4d078670..5b2052b2ddf 100644 --- a/test/OpenTelemetry.Tests/Logs/LogRecordTest.cs +++ b/test/OpenTelemetry.Tests/Logs/LogRecordTest.cs @@ -35,7 +35,8 @@ public sealed class LogRecordTest [Fact] public void CheckCateogryNameForLog() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: null); + var logger = loggerFactory.CreateLogger(); logger.LogInformation("Log"); var categoryName = exportedItems[0].CategoryName; @@ -52,7 +53,8 @@ public void CheckCateogryNameForLog() [InlineData(LogLevel.Critical)] public void CheckLogLevel(LogLevel logLevel) { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: null); + var logger = loggerFactory.CreateLogger(); var message = $"Log {logLevel}"; logger.Log(logLevel, message); @@ -64,7 +66,8 @@ public void CheckLogLevel(LogLevel logLevel) [Fact] public void CheckStateForUnstructuredLog() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: null); + var logger = loggerFactory.CreateLogger(); var message = "Hello, World!"; logger.LogInformation(message); @@ -79,7 +82,8 @@ public void CheckStateForUnstructuredLog() [Fact] public void CheckStateForUnstructuredLogWithStringInterpolation() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: null); + var logger = loggerFactory.CreateLogger(); var message = $"Hello from potato {0.99}."; logger.LogInformation(message); @@ -94,7 +98,8 @@ public void CheckStateForUnstructuredLogWithStringInterpolation() [Fact] public void CheckStateForStructuredLogWithTemplate() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: null); + var logger = loggerFactory.CreateLogger(); var message = "Hello from {name} {price}."; logger.LogInformation(message, "tomato", 2.99); @@ -121,7 +126,8 @@ public void CheckStateForStructuredLogWithTemplate() [Fact] public void CheckStateForStructuredLogWithStrongType() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: null); + var logger = loggerFactory.CreateLogger(); var food = new Food { Name = "artichoke", Price = 3.99 }; logger.LogInformation("{food}", food); @@ -147,7 +153,8 @@ public void CheckStateForStructuredLogWithStrongType() [Fact] public void CheckStateForStructuredLogWithAnonymousType() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: null); + var logger = loggerFactory.CreateLogger(); var anonymousType = new { Name = "pumpkin", Price = 5.99 }; logger.LogInformation("{food}", anonymousType); @@ -173,7 +180,8 @@ public void CheckStateForStructuredLogWithAnonymousType() [Fact] public void CheckStateForStrucutredLogWithGeneralType() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: null); + var logger = loggerFactory.CreateLogger(); var food = new Dictionary { @@ -211,7 +219,8 @@ public void CheckStateForStrucutredLogWithGeneralType() [Fact] public void CheckStateForExceptionLogged() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: null); + var logger = loggerFactory.CreateLogger(); var exceptionMessage = "Exception Message"; var exception = new Exception(exceptionMessage); @@ -234,7 +243,8 @@ public void CheckStateForExceptionLogged() [Fact] public void CheckTraceIdForLogWithinDroppedActivity() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: null); + var logger = loggerFactory.CreateLogger(); logger.LogInformation("Log within a dropped activity"); var logRecord = exportedItems[0]; @@ -248,7 +258,8 @@ public void CheckTraceIdForLogWithinDroppedActivity() [Fact] public void CheckTraceIdForLogWithinActivityMarkedAsRecordOnly() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: null); + var logger = loggerFactory.CreateLogger(); var sampler = new RecordOnlySampler(); var exportedActivityList = new List(); @@ -275,7 +286,8 @@ public void CheckTraceIdForLogWithinActivityMarkedAsRecordOnly() [Fact] public void CheckTraceIdForLogWithinActivityMarkedAsRecordAndSample() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: null); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: null); + var logger = loggerFactory.CreateLogger(); var sampler = new AlwaysOnSampler(); var exportedActivityList = new List(); @@ -302,7 +314,8 @@ public void CheckTraceIdForLogWithinActivityMarkedAsRecordAndSample() [Fact] public void VerifyIncludeFormattedMessage_False() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeFormattedMessage = false); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: options => options.IncludeFormattedMessage = false); + var logger = loggerFactory.CreateLogger(); logger.LogInformation("OpenTelemetry!"); var logRecord = exportedItems[0]; @@ -312,7 +325,8 @@ public void VerifyIncludeFormattedMessage_False() [Fact] public void VerifyIncludeFormattedMessage_True() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeFormattedMessage = true); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: options => options.IncludeFormattedMessage = true); + var logger = loggerFactory.CreateLogger(); logger.LogInformation("OpenTelemetry!"); var logRecord = exportedItems[0]; @@ -326,7 +340,8 @@ public void VerifyIncludeFormattedMessage_True() [Fact] public void IncludeFormattedMessageTestWhenFormatterNull() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeFormattedMessage = true); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: options => options.IncludeFormattedMessage = true); + var logger = loggerFactory.CreateLogger(); logger.Log(LogLevel.Information, default, "Hello World!", null, null); var logRecord = exportedItems[0]; @@ -346,7 +361,8 @@ public void IncludeFormattedMessageTestWhenFormatterNull() [Fact] public void VerifyIncludeScopes_False() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeScopes = false); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: options => options.IncludeScopes = false); + var logger = loggerFactory.CreateLogger(); using var scope = logger.BeginScope("string_scope"); @@ -361,7 +377,8 @@ public void VerifyIncludeScopes_False() [Fact] public void VerifyIncludeScopes_True() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.IncludeScopes = true); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: options => options.IncludeScopes = true); + var logger = loggerFactory.CreateLogger(); using var scope = logger.BeginScope("string_scope"); @@ -457,7 +474,8 @@ public void VerifyIncludeScopes_True() [Fact] public void VerifyParseStateValues_False_UsingStandardExtensions() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = false); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: options => options.ParseStateValues = false); + var logger = loggerFactory.CreateLogger(); // Tests state parsing with standard extensions. @@ -471,7 +489,8 @@ public void VerifyParseStateValues_False_UsingStandardExtensions() [Fact] public void VerifyParseStateValues_True_UsingStandardExtensions() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: options => options.ParseStateValues = true); + var logger = loggerFactory.CreateLogger(); // Tests state parsing with standard extensions. @@ -505,7 +524,8 @@ public void VerifyParseStateValues_True_UsingStandardExtensions() [Fact] public void ParseStateValuesUsingStructTest() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: options => options.ParseStateValues = true); + var logger = loggerFactory.CreateLogger(); // Tests struct IReadOnlyList> parse path. @@ -526,7 +546,8 @@ public void ParseStateValuesUsingStructTest() [Fact] public void ParseStateValuesUsingListTest() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: options => options.ParseStateValues = true); + var logger = loggerFactory.CreateLogger(); // Tests ref IReadOnlyList> parse path. @@ -547,7 +568,8 @@ public void ParseStateValuesUsingListTest() [Fact] public void ParseStateValuesUsingIEnumerableTest() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: options => options.ParseStateValues = true); + var logger = loggerFactory.CreateLogger(); // Tests IEnumerable> parse path. @@ -568,7 +590,8 @@ public void ParseStateValuesUsingIEnumerableTest() [Fact] public void ParseStateValuesUsingCustomTest() { - using var loggerFactory = InitializeLoggerFactory(out ILogger logger, out List exportedItems, configure: options => options.ParseStateValues = true); + using var loggerFactory = InitializeLoggerFactory(out List exportedItems, configure: options => options.ParseStateValues = true); + var logger = loggerFactory.CreateLogger(); // Tests unknown state parse path. @@ -595,12 +618,12 @@ public void ParseStateValuesUsingCustomTest() Assert.Same(state, actualState.Value); } - private static ILoggerFactory InitializeLoggerFactory(out ILogger logger, out List exportedItems, Action configure = null) + private static ILoggerFactory InitializeLoggerFactory(out List exportedItems, Action configure = null) { exportedItems = new List(); var exporter = new InMemoryExporter(exportedItems); var processor = new TestLogRecordProcessor(exporter); - var loggerFactory = LoggerFactory.Create(builder => + return LoggerFactory.Create(builder => { builder.AddOpenTelemetry(options => { @@ -609,9 +632,6 @@ private static ILoggerFactory InitializeLoggerFactory(out ILogger logger, out Li }); builder.AddFilter(typeof(LogRecordTest).FullName, LogLevel.Trace); }); - - logger = loggerFactory.CreateLogger(); - return loggerFactory; } internal struct Food From fbf6ac54bd06991b157c8ddde63c86e50cc6effc Mon Sep 17 00:00:00 2001 From: Timothy Mothra Date: Tue, 29 Mar 2022 15:27:45 -0700 Subject: [PATCH 15/15] Update CHANGELOG.md --- src/OpenTelemetry/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 79d2fbe5be7..49c6e5d8191 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -24,8 +24,8 @@ `readonly` ([#3065](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3065)) -* `OpenTelemetryLoggerProvider` now copies values from `OpenTelemetryLoggerOptions`. - Provider will not respect changes made after the LoggerFactory is built. +* [Bug fix] OpenTelemetryLoggerProvider is now unaffected by changes to + OpenTelemetryLoggerOptions after the LoggerFactory is built. ([#3055](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3055)) ## 1.2.0-rc3