pieceofsummer / Hangfire.Console

Job console extension for Hangfire
MIT License
436 stars 80 forks source link

Add support for ILoggin<> in asp.net core #58

Open danielmeza opened 6 years ago

danielmeza commented 6 years ago

This issue is for traking the support for ILoggin 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

pieceofsummer commented 6 years ago

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.

danielmeza commented 6 years ago

@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.

pieceofsummer commented 6 years ago

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.

TimmyGilissen commented 6 years ago

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?

mpetito commented 5 years ago

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.

migajek commented 5 years ago

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

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

danielmeza commented 5 years ago

@migajek I never thought it was so easy

danielmeza commented 4 years ago

@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 commented 4 years ago

@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

pieceofsummer commented 4 years ago

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.

danielmeza commented 4 years ago

@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

artworkad commented 2 years ago

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