Skip to content

Commit

Permalink
StackOverflowException if tracer needs ILoggerFactory (resolves #14)
Browse files Browse the repository at this point in the history
  • Loading branch information
cwe1ss committed Apr 22, 2018
1 parent 048ec67 commit 6930f02
Show file tree
Hide file tree
Showing 7 changed files with 99 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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>());
Expand Down
12 changes: 12 additions & 0 deletions src/OpenTracing.Contrib.NetCore/Internal/GlobalTracerAccessor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
using OpenTracing.Util;

namespace OpenTracing.Contrib.NetCore.Internal
{
public class GlobalTracerAccessor : IGlobalTracerAccessor
{
public ITracer GetGlobalTracer()
{
return GlobalTracer.Instance;
}
}
}
10 changes: 10 additions & 0 deletions src/OpenTracing.Contrib.NetCore/Internal/IGlobalTracerAccessor.cs
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using Microsoft.Extensions.Logging;
using OpenTracing.Contrib.NetCore.Internal;

namespace OpenTracing.Contrib.NetCore.Logging
{
Expand All @@ -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/>
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
}
}
}
8 changes: 8 additions & 0 deletions test/OpenTracing.Contrib.NetCore.Tests/Logging/LoggingTest.cs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down

0 comments on commit 6930f02

Please sign in to comment.