0

Below my logging works fine with the extension method I added. However, when I use this with DI, it does not work.... Why?

using ConsoleUI.Services;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
using Serilog;
using Serilog.Context;
using System;
using System.IO;
using System.Runtime.CompilerServices;

/// <summary>
/// 1. We want to use Dependenxy Injection or DI.
/// 2. We want to use logging, but with Serilog.
/// 3. We want to use our appsettings.json file.
/// </summary>
namespace ConsoleUI
{
    partial class Program
    {
        /// <summary>
        /// This is the main application.
        /// </summary>
        /// <param name="args">These are the arguments for the main application.</param>
        static void Main(string[] args)
        {
            //Used to get the method name for logging. This is extremely slow and should not be used.
            var stackTrace = new System.Diagnostics.StackTrace();
            var methodName = stackTrace.GetFrame(0).GetMethod().Name;

            //Used to setup your application settings configuration.
            var builder = new ConfigurationBuilder();
            BuildConfig(builder); //With objects we are passing a reference to that instance. When it is modified here, it is modfied for everyone!

            //There is a way to get the method name out for Serilog, by extendeding it, but this does not work for DI when using a general type for logging.
            //https://stackoverflow.com/questions/29470863/serilog-output-enrich-all-messages-with-methodname-from-which-log-entry-was-ca

            //Setup logging for Serilog.
            Log.Logger = new LoggerConfiguration()
                    .ReadFrom.Configuration(builder.Build())
                    .MinimumLevel.Verbose()
                    .Enrich.FromLogContext()
                    .Enrich.WithMachineName()
                    .CreateLogger();

            //The first thing to gett logged in the application.
            Log.Logger.Information("{name} --> Application Starting.", methodName);

            //This will help setup Depedency Injection. It also has our logger and appsettings.json, it has everything.
            var host = Host.CreateDefaultBuilder()
                .ConfigureServices((context, services) =>
                {
                    services.AddTransient<IGreetingService,GreetingService>(); //Give me a new instance of the GreetingService every time I ask for it.
                })
                .UseSerilog()
                .Build();

            //From all the services you have, create an instance of GreetingService.
            var svc = ActivatorUtilities.CreateInstance<GreetingService>(host.Services); //Normally, you should use the interface here and not the concrete class. Research this.
            svc.Run();

            //The first thing to gett logged in the application.
            Log.Logger.Information("{name} --> Application Finished Normally.", methodName);

            //!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
            //THIS WORKS FINE!!!!
            //!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
            Log.Logger.Here<Serilog.ILogger>().Information("Application Finished Normally.");


        }

        #region BuildConfig

        /// <summary>
        /// This will allow us to do logging, before we work with our actual configuration.
        /// </summary>
        /// <param name="builder">Our Configuration Builder.</param>
        static void BuildConfig(IConfigurationBuilder builder)
        {
            //Wherever the execitable is running, get the current directory, find file appsettings.json, and this is not option and if it changes, reload it!
            //Also - Get environment settings for the environment we are in as well. This second appsettings file will override the first one here.
            //       If APSNETCORE_ENVIRONMENT does not exist, assume this is Production and add .json at the end. If this file is not there, than that's cool, it's optional.
            //Also - If you have environmental variables, they can override what they are in appsettings.json.
            builder.SetBasePath(Directory.GetCurrentDirectory())
                .AddJsonFile("appsettings.json", optional: false, reloadOnChange: true)
                .AddJsonFile($"appsettings.{Environment.GetEnvironmentVariable("APSNETCORE_ENVIRONMENT") ?? "Production"}.json", optional: true)
                .AddEnvironmentVariables();
        }

        #endregion

    }

    public static class Extensions
    {
        public static Serilog.ILogger Here<T>(
            this Serilog.ILogger logger,
            [CallerMemberName] string memberName = "",
            [CallerFilePath] string sourceFilePath = "",
            [CallerLineNumber] int sourceLineNumber = 0)
        {
            return logger
            .ForContext("MemberName", memberName)
            .ForContext("FilePath", sourceFilePath)
            .ForContext("LineNumber", sourceLineNumber)
            .ForContext<T>();

        }
    }
}

This is what I am trying to use in my class by using DI. This is not working and I cannot figure out why. I nee the extension method to add additional context to my logging.

Error I am getting.: Severity Code Description Project File Line Suppression State Error CS1929 'ILogger' does not contain a definition for 'Here' and the best extension method overload 'Extensions.Here(ILogger, string, string, int)' requires a receiver of type 'ILogger' ConsoleUI D:\Code\Powerful Console App\BetterConsoleApp\ConsoleUI\Services\GreetingService.cs 42 Active

namespace ConsoleUI.Services
{
    public class GreetingService : IGreetingService
    {
        private readonly ILogger<GreetingService> _log;
        private readonly IConfiguration _config;

        public static string GetActualAsyncMethodName([CallerMemberName] string name = null) => name;

        /// <summary>
        /// Constructor for the GreetingService Class. This is bring in information from the Depedency Injection System.
        /// </summary>
        /// <param name="log">So the logger knows from what type of class we are going to call from. Don't modify this, just use it.</param>
        /// <param name="config">The configuration of the application. Don't modify this, just use it.</param>
        public GreetingService(ILogger<GreetingService> log, IConfiguration config)
        {
            _log = log;
            _config = config;
        }

        public void Run()
        {
            //Used to get the method name for logging.
            var stackTrace = new System.Diagnostics.StackTrace();
            var name = stackTrace.GetFrame(0).GetMethod().Name;

            for (int i = 0; i < _config.GetValue<int>("LoopTimes"); i++)
            {
                _log.LogInformation("{name} Run number {runNumber}", name, i); //Log "i" seperately under ther varianle name "runNumber". Can do query on run number 3.
            }

            //THIS DOES NOT WORK!?!?! Why?
            _log.Here<Serilog.ILogger>().Information("Application Finished Normally.");
        }
    }
}
Bill Blair
  • 61
  • 1
  • 2
  • 10
  • When you pass ILogger to your service, it will be the hooked up serilog one, you don't need an extension method. – PmanAce Nov 16 '20 at 21:03
  • Please elaborate. I do need the extension method to get the extra members to add to the logger context. – Bill Blair Nov 16 '20 at 21:48
  • @BillBlair Looks like you edited my answer instead of your own post. Edit your answer with the latest version of your code where you're using `ILogger` everywhere and *not* `ILogger` – C. Augusto Proiete Nov 17 '20 at 01:20

3 Answers3

1

It doesn't work because you are using the "wrong" ILogger in your service. Your extension method applies to instances of Serilog.ILogger and yet you are trying to call it on a Microsoft.Extensions.Logging.ILogger<T>.

Either you change your GreetingService to use Serilog.ILogger everywhere, or you have to change your extension method to target Microsoft.Extensions.Logging.ILogger<T> instead.

public class GreetingService
{
    private readonly Serilog.ILogger _log; // <#<#<#<#<#<#<#<#<#<#
    private readonly IConfiguration _config;

    public static string GetActualAsyncMethodName([CallerMemberName] string name = null) => name;

    public GreetingService(Serilog.ILogger log, IConfiguration config) // <#<#<#<#<#<#<#<#<#<#
    {
        _log = log;
        _config = config;
    }

    public void Run()
    {
        // ...

        _log.Here<Serilog.ILogger>().Information("Application Finished Normally.");
    }
}

You might also be interested in this question & answer:

Serilog DI in ASP.NET Core, which ILogger interface to inject?

C. Augusto Proiete
  • 24,684
  • 2
  • 63
  • 91
0

I need to register the GreetingService with the logger. This is what I had to do for the constructor of the GreetingService.

        //Passing in the configuration throgh depedency injection.
        private readonly Serilog.ILogger _log;
        private readonly IConfiguration _config;

        /// <summary>
        /// Constructor for the GreetingService Class. This is bring in information from the Depedency Injection System.
        /// </summary>
        /// <param name="config">The configuration of the application. Don't modify this, just use it.</param>
        public GreetingService(Serilog.ILogger<GreetingService> log, IConfiguration config)
        {
            _log = log;
            _config = config;
        }

If I don't do that, then how do I register the GreetingService through DI for this class? If I remove from the constructor, I got the the following error:"Unable to resolve service for type 'Serilog.ILogger' while attempting to activate 'ConsoleUI.Services.GreetingService'."

Bill Blair
  • 61
  • 1
  • 2
  • 10
  • I also found this: https://stackoverflow.com/a/61413261 – Bill Blair Nov 17 '20 at 02:09
  • I read your post and you said, "I prefer not to inject a logger in every single constructor of every class". Is this ok? I thought you would want to inject the logger for every class. Why would someone not want to do this? – Bill Blair Nov 17 '20 at 02:17
  • There's nothing wrong with injecting the `ILogger` in the constructor. It's a matter of personal preference. IMO it becomes very repetitive to have `ILogger` in the constructor as most classes will have a logger... If you prefer to inject, that's fine... It doesn't matter (in the case of Serilog) – C. Augusto Proiete Nov 17 '20 at 04:33
0

So, after reading your post, that I mention in your comment. I guess you don't have to inject a logger into a constructor of every class. Assuming this is ok (And I want to find out why). I found two ways to implement this, along with the logger extension I created earlier to have [CallerMemberName] and those ways are below...

private static Serilog.ILogger Log => Serilog.Log.ForContext<GreetingService>();
private readonly Serilog.ILogger _log = Serilog.Log.ForContext<GreetingService>();

Which of these is recommended?

Bill Blair
  • 61
  • 1
  • 2
  • 10
  • The second option is safer and what I would recommend, first because it will use the current `Log.Logger` at the time the class is instantiated (in case you change it, say for a unit test) and also to avoid the risk of your `static` instance being created before the Serilog pipeline has been configured (and Log.Logger) – C. Augusto Proiete Nov 17 '20 at 04:28