Hangfire.Console icon indicating copy to clipboard operation
Hangfire.Console copied to clipboard

Add support for ILoggin<> in asp.net core

Open danielmeza opened this issue 6 years ago • 12 comments

This issue is for traking the support for ILoggin<T> in order to ovoid the double log writter, one to console and one to the application logger, the idea is to include a feacture for inject a ILogger of the current class from the current method on the hangfire job is executing

danielmeza avatar Mar 06 '18 15:03 danielmeza

Hi Daniel,

This is not going to be implemented for a couple of reasons:

  1. The main purpose of Console extension is tracking job progress, not extensive logging, so the data written to either of those is usually different.
  2. Writing to console requires PerformContext associated with the current job execution, which is inaccessible from ILogger for the current class.

However, you can implement your own extension method which would log to both and meet your particular needs.

pieceofsummer avatar Mar 06 '18 15:03 pieceofsummer

@pieceofsummer I understand, but with reflection from performe context we can get the information off the type, for the #1 point we can make it optional and configurable from the startup class. I will make a pull request this weekend and if yoy likeit you can do the merge.

danielmeza avatar Mar 06 '18 16:03 danielmeza

I'm not saying it is technically impossible, I just don't think it is necessary for everyone. There's a ton of things to consider here (what logging level to use when writing to the logger, or should it depend on text color, e.g. red for errors, yellow for warnings etc., or whether the progress should be logged somehow). Making all those things configurable through options looks like an overkill, and there would always be someone who wants it done in a different way. It's just impossible to please everyone :)

So I tend not to alter library code if things can be easily implemented as extension methods.

pieceofsummer avatar Mar 06 '18 17:03 pieceofsummer

I think that it should be possible to inject in a logger implementation. First of all I don't want a hangfire reference in my business or application layer. Hangfire is a third-party module that belongs in my infrastrucure layer. When I log some info I want it in my log files and in my hangfire console.

Are there plans for making it possible to inject the PerformContext in an implementation?

TimmyGilissen avatar Apr 19 '18 13:04 TimmyGilissen

When I log some info I want it in my log files and in my hangfire console.

This is absolutely correct and I'm sure a very common use-case.

Geez, my task failed and here are the info / warning messages printed out by my existing logging during execution, how useful!

If we have a common abstraction of ILogger and it's used throughout, I do not want to couple the usage of ILogger with PerformContext everywhere throughout my code.

I ended up using AsyncLocal to relay the applicable PerformContext instance to a logging sink to get around this for now but this deserves to be revisited.

mpetito avatar Mar 05 '19 22:03 mpetito

if anybody needs this, here's the simple workaround using AsyncLocal (as @mpetito did)

https://gist.github.com/migajek/ccb4d89d779c348f45f02a16d6179136

migajek avatar Oct 08 '19 10:10 migajek

@migajek I never thought it was so easy

danielmeza avatar Oct 09 '19 04:10 danielmeza

@migajek we can use a JobFilter to auto put the contex and cleand the dictionary the job need to be decorated with the filter

danielmeza avatar Nov 27 '19 22:11 danielmeza

@migajek here is the JobFilter

    [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method | AttributeTargets.Interface)]
    public class LogToConsoleAttribute : JobFilterAttribute, IServerFilter
    {
        private IDisposable _subscription;

        public void OnPerforming(PerformingContext filterContext)
        {
            _subscription = HangfireConsoleLogger.InContext(filterContext);
        }
        public void OnPerformed(PerformedContext filterContext)
        {
            _subscription?.Dispose();
        }
    }

    [LogToConsole]
    public class Job1
    {
        private readonly ILogger<Job1> _logger;

        public Job1(ILogger<Job1> logger)
        {
            _logger = logger;
        }

        public void Task1()
        {
            _logger.LogInformation("Hello to log and console");
        }
    }

@mpetito to avoid propagate hangfire throw our code by using the attribute we can add this

    public class LogToConsolFilterProvider : IJobFilterProvider
    {
        public IEnumerable<JobFilter> GetFilters(Job job)
        {
            yield return new JobFilter(new LogToConsoleAttribute(), JobFilterScope.Global, 0);
        }
    }

this will inject the filter to all jobs

danielmeza avatar Nov 27 '19 23:11 danielmeza

A quick follow up: Hangfire 1.7 now allows a custom JobActivator to access full PerformContext, so it is possible to inject it (and any other services depending on it) into job constructor. That way you can easily implement an ILogger<T> subclass writing to console.

pieceofsummer avatar Dec 03 '19 16:12 pieceofsummer

@pieceofsummer the solution proposed by @migajek looks better and we can easily implement it and it can be switch on/off I can make a pr

danielmeza avatar Apr 07 '20 19:04 danielmeza

There is actually a package for this problem: https://github.com/AnderssonPeter/Hangfire.Console.Extensions

artworkad avatar Dec 03 '21 10:12 artworkad