From 6930f029acce308792a3344ba3b7a6460f8ac07e Mon Sep 17 00:00:00 2001
From: Christian Weiss <christian@chwe.at>
Date: Sun, 22 Apr 2018 13:43:44 +0200
Subject: [PATCH] StackOverflowException if tracer needs ILoggerFactory
 (resolves #14)

---
 .../ServiceCollectionExtensions.cs            |  1 +
 .../Internal/GlobalTracerAccessor.cs          | 12 ++++
 .../Internal/IGlobalTracerAccessor.cs         | 10 ++++
 .../Logging/OpenTracingLoggerProvider.cs      | 12 +++-
 .../Logging/LoggingDependencyInjectionTest.cs | 55 +++++++++++++++++++
 .../Logging/LoggingTest.cs                    |  8 +++
 .../OpenTracing.Contrib.NetCore.Tests.csproj  |  3 +
 7 files changed, 99 insertions(+), 2 deletions(-)
 create mode 100644 src/OpenTracing.Contrib.NetCore/Internal/GlobalTracerAccessor.cs
 create mode 100644 src/OpenTracing.Contrib.NetCore/Internal/IGlobalTracerAccessor.cs
 create mode 100644 test/OpenTracing.Contrib.NetCore.Tests/Logging/LoggingDependencyInjectionTest.cs

diff --git a/src/OpenTracing.Contrib.NetCore/Configuration/ServiceCollectionExtensions.cs b/src/OpenTracing.Contrib.NetCore/Configuration/ServiceCollectionExtensions.cs
index bfe3d16..8d04847 100644
--- a/src/OpenTracing.Contrib.NetCore/Configuration/ServiceCollectionExtensions.cs
+++ b/src/OpenTracing.Contrib.NetCore/Configuration/ServiceCollectionExtensions.cs
@@ -39,6 +39,7 @@ public static IServiceCollection AddOpenTracingCoreServices(this IServiceCollect
                 throw new ArgumentNullException(nameof(services));
 
             services.TryAddSingleton<ITracer>(GlobalTracer.Instance);
+            services.TryAddSingleton<IGlobalTracerAccessor, GlobalTracerAccessor>();
 
             services.TryAddSingleton<DiagnosticManager>();
             services.TryAddEnumerable(ServiceDescriptor.Singleton<IHostedService, InstrumentationService>());
diff --git a/src/OpenTracing.Contrib.NetCore/Internal/GlobalTracerAccessor.cs b/src/OpenTracing.Contrib.NetCore/Internal/GlobalTracerAccessor.cs
new file mode 100644
index 0000000..1d9bc21
--- /dev/null
+++ b/src/OpenTracing.Contrib.NetCore/Internal/GlobalTracerAccessor.cs
@@ -0,0 +1,12 @@
+using OpenTracing.Util;
+
+namespace OpenTracing.Contrib.NetCore.Internal
+{
+    public class GlobalTracerAccessor : IGlobalTracerAccessor
+    {
+        public ITracer GetGlobalTracer()
+        {
+            return GlobalTracer.Instance;
+        }
+    }
+}
diff --git a/src/OpenTracing.Contrib.NetCore/Internal/IGlobalTracerAccessor.cs b/src/OpenTracing.Contrib.NetCore/Internal/IGlobalTracerAccessor.cs
new file mode 100644
index 0000000..308a989
--- /dev/null
+++ b/src/OpenTracing.Contrib.NetCore/Internal/IGlobalTracerAccessor.cs
@@ -0,0 +1,10 @@
+namespace OpenTracing.Contrib.NetCore.Internal
+{
+    /// <summary>
+    /// Helper interface which allows unit tests to mock the <see cref="OpenTracing.Util.GlobalTracer"/>.
+    /// </summary>
+    public interface IGlobalTracerAccessor
+    {
+        ITracer GetGlobalTracer();
+    }
+}
diff --git a/src/OpenTracing.Contrib.NetCore/Logging/OpenTracingLoggerProvider.cs b/src/OpenTracing.Contrib.NetCore/Logging/OpenTracingLoggerProvider.cs
index a74f7dd..d657d9d 100644
--- a/src/OpenTracing.Contrib.NetCore/Logging/OpenTracingLoggerProvider.cs
+++ b/src/OpenTracing.Contrib.NetCore/Logging/OpenTracingLoggerProvider.cs
@@ -1,5 +1,6 @@
 using System;
 using Microsoft.Extensions.Logging;
+using OpenTracing.Contrib.NetCore.Internal;
 
 namespace OpenTracing.Contrib.NetCore.Logging
 {
@@ -11,9 +12,16 @@ internal class OpenTracingLoggerProvider : ILoggerProvider
     {
         private readonly ITracer _tracer;
 
-        public OpenTracingLoggerProvider(ITracer tracer)
+        public OpenTracingLoggerProvider(IGlobalTracerAccessor globalTracerAccessor)
         {
-            _tracer = tracer ?? throw new ArgumentNullException(nameof(tracer));
+            // HACK: We can't use ITracer directly here because this would lead to a StackOverflowException
+            // (due to a circular dependency) if the ITracer needs a ILoggerFactory.
+            // https://github.com/opentracing-contrib/csharp-netcore/issues/14
+
+            if (globalTracerAccessor == null)
+                throw new ArgumentNullException(nameof(globalTracerAccessor));
+
+            _tracer = globalTracerAccessor.GetGlobalTracer();
         }
 
         /// <inheritdoc/>
diff --git a/test/OpenTracing.Contrib.NetCore.Tests/Logging/LoggingDependencyInjectionTest.cs b/test/OpenTracing.Contrib.NetCore.Tests/Logging/LoggingDependencyInjectionTest.cs
new file mode 100644
index 0000000..878fcd2
--- /dev/null
+++ b/test/OpenTracing.Contrib.NetCore.Tests/Logging/LoggingDependencyInjectionTest.cs
@@ -0,0 +1,55 @@
+using System;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using OpenTracing.Propagation;
+using Xunit;
+
+namespace OpenTracing.Contrib.Netcore.Tests.Logging
+{
+    public class LoggingDependencyInjectionTest
+    {
+        [Fact]
+        public void Resolving_tracer_that_needs_ILoggerFactory_succeeds()
+        {
+            // https://github.com/opentracing-contrib/csharp-netcore/issues/14
+
+            var serviceProvider = new ServiceCollection()
+                .AddLogging()
+                .AddOpenTracingCoreServices(ot =>
+                {
+                    ot.AddLoggerProvider();
+                    ot.Services.AddSingleton<ITracer, TracerWithLoggerFactory>();
+                })
+                .BuildServiceProvider();
+
+            var tracer = serviceProvider.GetRequiredService<ITracer>();
+            Assert.IsType<TracerWithLoggerFactory>(tracer);
+        }
+
+        private class TracerWithLoggerFactory : ITracer
+        {
+            public TracerWithLoggerFactory(ILoggerFactory loggerFactory)
+            {
+            }
+
+            public IScopeManager ScopeManager => throw new NotSupportedException();
+
+            public ISpan ActiveSpan => throw new NotSupportedException();
+
+            public ISpanBuilder BuildSpan(string operationName)
+            {
+                throw new NotSupportedException();
+            }
+
+            public ISpanContext Extract<TCarrier>(IFormat<TCarrier> format, TCarrier carrier)
+            {
+                throw new NotSupportedException();
+            }
+
+            public void Inject<TCarrier>(ISpanContext spanContext, IFormat<TCarrier> format, TCarrier carrier)
+            {
+                throw new NotSupportedException();
+            }
+        }
+    }
+}
diff --git a/test/OpenTracing.Contrib.NetCore.Tests/Logging/LoggingTest.cs b/test/OpenTracing.Contrib.NetCore.Tests/Logging/LoggingTest.cs
index e390ee7..a926009 100644
--- a/test/OpenTracing.Contrib.NetCore.Tests/Logging/LoggingTest.cs
+++ b/test/OpenTracing.Contrib.NetCore.Tests/Logging/LoggingTest.cs
@@ -1,6 +1,8 @@
 using System;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using NSubstitute;
+using OpenTracing.Contrib.NetCore.Internal;
 using OpenTracing.Mock;
 using Xunit;
 
@@ -20,6 +22,12 @@ public LoggingTest()
                 {
                     ot.AddLoggerProvider();
                     ot.Services.AddSingleton<ITracer, MockTracer>();
+                    ot.Services.AddSingleton<IGlobalTracerAccessor>(sp =>
+                    {
+                        var globalTracerAccessor = Substitute.For<IGlobalTracerAccessor>();
+                        globalTracerAccessor.GetGlobalTracer().Returns(sp.GetRequiredService<ITracer>());
+                        return globalTracerAccessor;
+                    });
                 })
                 .BuildServiceProvider();
 
diff --git a/test/OpenTracing.Contrib.NetCore.Tests/OpenTracing.Contrib.NetCore.Tests.csproj b/test/OpenTracing.Contrib.NetCore.Tests/OpenTracing.Contrib.NetCore.Tests.csproj
index f13dba1..bfff346 100644
--- a/test/OpenTracing.Contrib.NetCore.Tests/OpenTracing.Contrib.NetCore.Tests.csproj
+++ b/test/OpenTracing.Contrib.NetCore.Tests/OpenTracing.Contrib.NetCore.Tests.csproj
@@ -2,6 +2,9 @@
 
   <PropertyGroup>
     <TargetFramework>netcoreapp2.0</TargetFramework>
+
+    <!-- Required for dotnet-xunit (https://github.com/xunit/xunit/issues/1573) -->
+    <RuntimeFrameworkVersion>2.0.0</RuntimeFrameworkVersion>
   </PropertyGroup>
 
   <ItemGroup>