From f5889ec694ebecdb8688ce3e1b987569caf41348 Mon Sep 17 00:00:00 2001
From: Tarek Mahmoud Sayed <tarekms@microsoft.com>
Date: Tue, 8 Aug 2023 12:20:25 -0700
Subject: [PATCH] Fix Activity Baggage logging scope (#90127)

---
 .../src/LoggerFactoryScopeProvider.cs              | 32 +++++---
 .../tests/Common/LoggerFactoryTest.cs              | 88 ++++++++++++++++++++++
 2 files changed, 111 insertions(+), 9 deletions(-)

diff --git a/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactoryScopeProvider.cs b/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactoryScopeProvider.cs
index 9795a46..5e2bfeb 100644
--- a/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactoryScopeProvider.cs
+++ b/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactoryScopeProvider.cs
@@ -220,7 +220,7 @@ namespace Microsoft.Extensions.Logging
             }
         }
 
-        private sealed class ActivityBaggageLogScopeWrapper : IEnumerable<KeyValuePair<string, string?>>
+        private sealed class ActivityBaggageLogScopeWrapper : IEnumerable<KeyValuePair<string, object?>>
         {
             private readonly IEnumerable<KeyValuePair<string, string?>> _items;
 
@@ -231,15 +231,9 @@ namespace Microsoft.Extensions.Logging
                 _items = items;
             }
 
-            public IEnumerator<KeyValuePair<string, string?>> GetEnumerator()
-            {
-                return _items.GetEnumerator();
-            }
+            public IEnumerator<KeyValuePair<string, object?>> GetEnumerator() => new BaggageEnumerator(_items.GetEnumerator());
 
-            IEnumerator IEnumerable.GetEnumerator()
-            {
-                return _items.GetEnumerator();
-            }
+            IEnumerator IEnumerable.GetEnumerator() => new BaggageEnumerator(_items.GetEnumerator());
 
             public override string ToString()
             {
@@ -269,6 +263,26 @@ namespace Microsoft.Extensions.Logging
                     return result;
                 }
             }
+
+            private struct BaggageEnumerator : IEnumerator<KeyValuePair<string, object?>>
+            {
+                private readonly IEnumerator<KeyValuePair<string, string?>> _enumerator;
+
+                public BaggageEnumerator(IEnumerator<KeyValuePair<string, string?>> enumerator)
+                {
+                    _enumerator = enumerator;
+                }
+
+                public KeyValuePair<string, object?> Current => new KeyValuePair<string, object?>(_enumerator.Current.Key, _enumerator.Current.Value);
+
+                object? IEnumerator.Current => Current;
+
+                public void Dispose() => _enumerator.Dispose();
+
+                public bool MoveNext() => _enumerator.MoveNext();
+
+                public void Reset() => _enumerator.Reset();
+            }
         }
     }
 
diff --git a/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerFactoryTest.cs b/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerFactoryTest.cs
index 7c2f397..48b9eb2 100644
--- a/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerFactoryTest.cs
+++ b/src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerFactoryTest.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.IO;
 using System.Text;
 using System.Diagnostics;
 using System.Collections.Generic;
@@ -407,6 +408,46 @@ namespace Microsoft.Extensions.Logging.Test
         }
 
         [Fact]
+        public void BaggageFormattedOutput()
+        {
+            var loggerProvider = new ExternalScopeLoggerWithFormatterProvider();
+            var loggerFactory = LoggerFactory.Create(builder =>
+            {
+                builder
+                    .Configure(o => o.ActivityTrackingOptions = ActivityTrackingOptions.TraceId | ActivityTrackingOptions.Baggage | ActivityTrackingOptions.Tags)
+                    .AddProvider(loggerProvider);
+            });
+
+            var logger = loggerFactory.CreateLogger("Logger");
+
+            Activity activity = new Activity("ScopeActivity");
+            activity.Start();
+
+            activity.AddTag("Tag1", "1");
+            activity.AddBaggage("Baggage1", "1");
+
+            using (logger.BeginScope("Scope1"))
+            {
+                activity.AddTag("Tag2", "2");
+                activity.AddBaggage("Baggage2", "2");
+                logger.LogInformation("Inside Scope Info!");
+            }
+            activity.Stop();
+
+            string [] loggerOutput = new string[]
+            {
+                $"Inside Scope Info!",
+                $"[TraceId, {activity.GetTraceId()}]",
+                $"[Tag1, 1]",
+                $"[Tag2, 2]",
+                $"[Baggage2, 2]",
+                $"[Baggage1, 1]",
+                $"Scope1",
+            };
+            Assert.Equal(loggerOutput, loggerProvider.LogText);
+        }
+
+        [Fact]
         public void CallsSetScopeProvider_OnSupportedProviders()
         {
             var loggerProvider = new ExternalScopeLoggerProvider();
@@ -580,6 +621,53 @@ namespace Microsoft.Extensions.Logging.Test
             }
         }
 
+        // Support formatting IEnumerable<KeyValuePair<string, object?>> scopes
+        private class ExternalScopeLoggerWithFormatterProvider : ILoggerProvider, ISupportExternalScope, ILogger
+        {
+            public void SetScopeProvider(IExternalScopeProvider scopeProvider)
+            {
+                ScopeProvider = scopeProvider;
+            }
+
+            public IExternalScopeProvider ScopeProvider { get; set; }
+
+            public int BeginScopeCalledTimes { get; set; }
+
+            public List<string> LogText { get; set; } = new List<string>();
+
+            public void Dispose() { }
+
+            public ILogger CreateLogger(string categoryName) => this;
+
+            public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
+            {
+                LogText.Add(formatter(state, exception));
+
+                ScopeProvider.ForEachScope((scope, builder) =>
+                {
+                    if (scope is IEnumerable<KeyValuePair<string, object>> scopeItems)
+                    {
+                        foreach (KeyValuePair<string, object> item in scopeItems)
+                        {
+                            builder.Add(item.ToString());
+                        }
+                    }
+                    else
+                    {
+                        builder.Add(scope.ToString());
+                    }
+                }, LogText);
+            }
+
+            public bool IsEnabled(LogLevel logLevel) => true;
+
+            public IDisposable BeginScope<TState>(TState state)
+            {
+                BeginScopeCalledTimes++;
+                return null;
+            }
+        }
+
         private class Dummy
         {
             public override string ToString()
-- 
2.7.4