Skip to content

Commit

Permalink
Ensure we also always globally log when using .NET core IServiceColle…
Browse files Browse the repository at this point in the history
…ction
  • Loading branch information
Mpdreamz committed Jun 3, 2024
1 parent 70edc92 commit 734172a
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public static IApplicationBuilder UseElasticApm(
params IDiagnosticsSubscriber[] subscribers
)
{
var logger = NetCoreLogger.GetApmLogger(builder.ApplicationServices);
var logger = ApmExtensionsLogger.GetApmLogger(builder.ApplicationServices);

var configReader = configuration == null
? new EnvironmentConfiguration(logger)
Expand All @@ -70,7 +70,7 @@ params IDiagnosticsSubscriber[] subscribers
internal static IApplicationBuilder UseElasticApm(
this IApplicationBuilder builder,
ApmAgent agent,
IApmLogger logger,
Logging.IApmLogger logger,
params IDiagnosticsSubscriber[] subscribers
)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@

namespace Elastic.Apm.Extensions.Hosting;

internal sealed class NetCoreLogger : IApmLogger
internal sealed class ApmExtensionsLogger : IApmLogger
{
private readonly ILogger _logger;

public NetCoreLogger(ILoggerFactory loggerFactory) => _logger = loggerFactory?.CreateLogger("Elastic.Apm") ?? throw new ArgumentNullException(nameof(loggerFactory));
public ApmExtensionsLogger(ILoggerFactory loggerFactory) => _logger = loggerFactory?.CreateLogger("Elastic.Apm") ?? throw new ArgumentNullException(nameof(loggerFactory));

public bool IsEnabled(LogLevel level) => _logger.IsEnabled(Convert(level));

Expand All @@ -34,6 +34,6 @@ private static Microsoft.Extensions.Logging.LogLevel Convert(LogLevel logLevel)

internal static IApmLogger GetApmLogger(IServiceProvider serviceProvider) =>
serviceProvider.GetService(typeof(ILoggerFactory)) is ILoggerFactory loggerFactory
? new NetCoreLogger(loggerFactory)
? new ApmExtensionsLogger(loggerFactory)
: ConsoleLogger.Instance;
}
35 changes: 35 additions & 0 deletions src/integrations/Elastic.Apm.Extensions.Hosting/CompositeLogger.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Licensed to Elasticsearch B.V under one or more agreements.
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
// See the LICENSE file in the project root for more information

using System;
using Elastic.Apm.Logging;
using LogLevel = Elastic.Apm.Logging.LogLevel;

namespace Elastic.Apm.Extensions.Hosting;

internal sealed class CompositeLogger(TraceLogger traceLogger, IApmLogger logger) : IDisposable , IApmLogger
{
public TraceLogger TraceLogger { get; } = traceLogger;
public IApmLogger ApmLogger { get; } = logger;

private bool _isDisposed;

public void Dispose() => _isDisposed = true;

public void Log<TState>(LogLevel level, TState state, Exception e, Func<TState, Exception, string> formatter)
{
if (_isDisposed)
return;

if (TraceLogger.IsEnabled(level))
TraceLogger.Log(level, state, e, formatter);

if (ApmLogger.IsEnabled(level))
ApmLogger.Log(level, state, e, formatter);
}

public bool IsEnabled(LogLevel logLevel) => ApmLogger.IsEnabled(logLevel) || TraceLogger.IsEnabled(logLevel);


}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ internal class ApmConfiguration : FallbackToEnvironmentConfigurationBase
{
private const string ThisClassName = nameof(ApmConfiguration);

public ApmConfiguration(IConfiguration configuration, IApmLogger logger, string defaultEnvironmentName)
public ApmConfiguration(IConfiguration configuration, Apm.Logging.IApmLogger logger, string defaultEnvironmentName)
: base(logger,
new ConfigurationDefaults { EnvironmentName = defaultEnvironmentName, DebugName = ThisClassName },
new ConfigurationKeyValueProvider(configuration)) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
using LogLevel = Elastic.Apm.Logging.LogLevel;

namespace Elastic.Apm.Extensions.Hosting
{
Expand Down Expand Up @@ -54,7 +55,13 @@ public static IHostBuilder UseElasticApm(this IHostBuilder builder, params IDiag
//If the static agent doesn't exist, we create one here. If there is already 1 agent created, we reuse it.
if (!Agent.IsConfigured)
{
services.AddSingleton<IApmLogger, NetCoreLogger>();
services.AddSingleton(sp =>
{
var netCoreLogger = ApmExtensionsLogger.GetApmLogger(sp);
var globalLogger = AgentComponents.GetGlobalLogger(netCoreLogger, LogLevel.Error);
var logger = globalLogger is TraceLogger g ? new CompositeLogger(g, netCoreLogger) : netCoreLogger;
return logger;
});
services.AddSingleton<IConfigurationReader>(sp =>
new ApmConfiguration(ctx.Configuration, sp.GetService<IApmLogger>(), GetHostingEnvironmentName(ctx, sp.GetService<IApmLogger>())));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,8 @@
using Elastic.Apm.Logging;
using Elastic.Apm.NetCoreAll;
using Elastic.Apm.Report;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
using static Microsoft.Extensions.DependencyInjection.ServiceDescriptor;

namespace Microsoft.Extensions.DependencyInjection;

Expand Down Expand Up @@ -49,28 +47,25 @@ public static IServiceCollection AddElasticApm(this IServiceCollection services,
if (agentConfigured)
return Agent.Instance;
var logger = NetCoreLogger.GetApmLogger(sp);
var environmentName = GetEnvironmentName(sp);
if (environmentName is null)
{
logger?.Warning()?.Log("Failed to retrieve hosting environment name");
environmentName = "Undetermined";
}
var netCoreLogger = ApmExtensionsLogger.GetApmLogger(sp);
var configuration = sp.GetService<Configuration.IConfiguration>();
var environmentName = GetDefaultEnvironmentName(sp);
IConfigurationReader configurationReader = configuration is null
? new EnvironmentConfiguration(logger)
: new ApmConfiguration(configuration, logger, environmentName);
? new EnvironmentConfiguration(netCoreLogger)
: new ApmConfiguration(configuration, netCoreLogger, environmentName ?? "Undetermined");
var globalLogger = AgentComponents.GetGlobalLogger(netCoreLogger, configurationReader.LogLevel);
var logger = globalLogger is TraceLogger g ? new CompositeLogger(g, netCoreLogger) : netCoreLogger;
if (environmentName is null)
logger?.Warning()?.Log("Failed to retrieve default hosting environment name");
// This may be null, which is fine
var payloadSender = sp.GetService<IPayloadSender>();
var components = agentConfigured
? Agent.Components
: new AgentComponents(logger, configurationReader, payloadSender);
var components = new AgentComponents(logger, configurationReader, payloadSender);
HostBuilderExtensions.UpdateServiceInformation(components.Service);
Agent.Setup(components);
Expand Down Expand Up @@ -110,7 +105,7 @@ public static IServiceCollection AddElasticApm(this IServiceCollection services,
return services;
}

private static string GetEnvironmentName(IServiceProvider serviceProvider) =>
private static string GetDefaultEnvironmentName(IServiceProvider serviceProvider) =>
#if NET6_0_OR_GREATER
(serviceProvider.GetService(typeof(IHostEnvironment)) as IHostEnvironment)?.EnvironmentName; // This is preferred since 3.0
#else
Expand Down
1 change: 1 addition & 0 deletions src/startuphook/Elastic.Apm.StartupHook.Loader/Loader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Elastic.Apm.GrpcClient;
using Elastic.Apm.Instrumentations.SqlClient;
using Elastic.Apm.Logging;
using IApmLogger = Elastic.Apm.Logging.IApmLogger;

namespace Elastic.Apm.StartupHook.Loader
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,17 @@ public void UseElasticApmShouldUseAspNetLoggerWhenLoggingIsConfigured()
var services = new ServiceCollection()
.AddLogging();

var logger = NetCoreLogger.GetApmLogger(services.BuildServiceProvider());
var logger = ApmExtensionsLogger.GetApmLogger(services.BuildServiceProvider());

Assert.IsType<NetCoreLogger>(logger);
Assert.IsType<ApmExtensionsLogger>(logger);
}

[Fact]
public void UseElasticApmShouldUseConsoleLoggerInstanceWhenLoggingIsNotConfigured()
{
var services = new ServiceCollection();

var logger = NetCoreLogger.GetApmLogger(services.BuildServiceProvider());
var logger = ApmExtensionsLogger.GetApmLogger(services.BuildServiceProvider());

Assert.IsType<ConsoleLogger>(logger);
Assert.Same(ConsoleLogger.Instance, logger);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
using SampleAspNetCoreApp;
using Xunit;
using Xunit.Abstractions;
using IApmLogger = Elastic.Apm.Logging.IApmLogger;

namespace Elastic.Apm.AspNetCore.Tests
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@
namespace Elastic.Apm.AspNetCore.Tests
{
/// <summary>
/// Tests the <see cref="Elastic.Apm.Extensions.Hosting.NetCoreLogger" /> type.
/// Tests the <see cref="ApmExtensionsLogger" /> type.
/// </summary>
public class AspNetCoreLoggerTests
{
[Fact]
public void AspNetCoreLoggerShouldThrowExceptionWhenLoggerFactoryIsNull()
=> Assert.Throws<ArgumentNullException>(() => new NetCoreLogger(null));
=> Assert.Throws<ArgumentNullException>(() => new ApmExtensionsLogger(null));

[Fact]
public void AspNetCoreLoggerShouldGetLoggerFromFactoryWithProperCategoryName()
Expand All @@ -27,7 +27,7 @@ public void AspNetCoreLoggerShouldGetLoggerFromFactoryWithProperCategoryName()
.Returns(() => Mock.Of<ILogger>());

// ReSharper disable UnusedVariable
var logger = new NetCoreLogger(loggerFactoryMock.Object);
var logger = new ApmExtensionsLogger(loggerFactoryMock.Object);
// ReSharper restore UnusedVariable

loggerFactoryMock.Verify(x => x.CreateLogger(It.Is<string>(s => s.Equals("Elastic.Apm"))), Times.Once);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Microsoft.AspNetCore.Mvc.Testing;
using SampleAspNetCoreApp;
using Xunit;
using IApmLogger = Elastic.Apm.Logging.IApmLogger;

namespace Elastic.Apm.AspNetCore.Tests
{
Expand Down

0 comments on commit 734172a

Please sign in to comment.