Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Implement an official FileLogger #441

Closed
muratg opened this issue May 27, 2016 · 27 comments
Closed

Implement an official FileLogger #441

muratg opened this issue May 27, 2016 · 27 comments
Milestone

Comments

@muratg
Copy link

muratg commented May 27, 2016

A very simple file based logger would be useful.

@muratg muratg added this to the 1.0.1 milestone May 27, 2016
@grahamehorner
Copy link

personally I like the enterprise library semantic logging aka SLAB & feel that should/if this be made coreclr compatible that this would provide a lot of features/benefit for the logging story which could be used by non aspnetcore projects

@davidfowl
Copy link
Member

@grahamehorner sure but that has little to do with this issue.

@grahamehorner
Copy link

@davidFlow not really given SLAB has many sinks including a simple file and rolling file sink, why reinvent the wheel? also others have asked for EventSource support; killing two birds with one stone. 😀 I've already started a fork of SLAB with a view of making a

@davidfowl
Copy link
Member

davidfowl commented May 28, 2016

Because even if slab is ported, we're not going to delete this library. Serilog/NLog/log4Net already has a file logger as well but that won't influence our decision to have a basic file logger out of the box. There's also an event source logger in the works for RTM #424

@Yves57
Copy link
Contributor

Yves57 commented Jul 8, 2016

  • With only one file for all ILogger<> instances, or one file for each ILogger<> (with for instance the logger name as file name)?
  • I think that a queue is necessary to buffer the messages (ILogger.Log() is synchronous so it has to be fast). But it is maybe too much for a "basic" file logger?

@davidfowl
Copy link
Member

  • Single file for all Logger instances that rolls over when it hits a threshold.
  • Yes the writing should be buffered and delegated to a worker thread writing to disk.

@victorhurdugaci
Copy link
Contributor

Here's the plan, let me know what you think:

By default:

  1. The file logger will log in a predefined location (based on app name) and will number-stamp files. A new log file will be created when the previous log file reaches a certain size. So let's say the size limit is 100mb, then you'll end up with MyApp1.txt (100mb), MyApp2.txt (100mb), MyApp3.txt(42mb)
  2. It has a background thread that does the file logging with a queue of a limited size
  3. If the queue reaches it's capacity, then the callers will be slowed down

You can configure:

  1. The factory that generates the log file name (you could use a time based file name instead for example)
  2. The default name factory file size
  3. The folder where to logs are store
  4. The size of the background queue

Things to consider:

  1. What happens when you get slowed down by a full queue. Should we append a message in the log? If yes, how often?

Rough API:

// API:
// Might consider grouping the configuration params in Options

AddFile();
AddFile(folderName = null, queueSize = 100, Func<string, string> nameFactory = defaultnamefactory);

// Examples:

// Default file name, location, queue size, file size
loggerFactory.AddFile(); 

// New naming convention
loggerFactory.AddFile(nameFactory: previousFileName => {
     if (someCondition) {
         return newFileName;
     }
     return previousFileName
});

// Custom folder and log file size (default name)
loggerFactory.AddFile("/someFolder", 1024);

Thoughts?

@cesarblum
Copy link
Contributor

I think the default should be to name log files by date (UTC).

@Yves57
Copy link
Contributor

Yves57 commented Jul 9, 2016

  • What about text encoding? It think the best it to have an option.
  • What happens in case of error when writing in file (disk full,...)? Because the messages are added in the queue during theILogger.Log, the error will be thrown later in a different thread.

@nblumhardt
Copy link
Contributor

Is the goal here just to simplify the few lines of code needed in order to get file logging up and running? If so, why not open a discussion with the community implementers of Microsoft.Extensions.Logging about how we can do that without the framework sucking the air out of the room?

Having complete implementations of m.e.Logging and m.e.DependencyInjection in the framework is a short-term win and huge long-term loss. We (me, the community of people writing open source components for .NET, and others) have supported the vision of a lean, core, cross-platform modular platform on which the community can build from the beginning. Serilog, and Autofac, for example, have been there at every -beta and -rc you've put out. The 1.0 is awesome progress, but there's still heaps for everyone to do and we need to know you're still committed to that vision.

As the core team, you're in a unique position: you can bring the community together, and you can fill gaps in the platform, in a way that no one else can. Do we need better file logging in .NET? Exposing a good API for atomic append writes would mean we can all have awesome file loggers. Reinventing the wheel will just mean we have yet more mediocre components and less investment into .NET open source.

When you started m.e.Logging by building NLog and Serilog adapters, and subsequently donating them to the respective projects, it felt like the beginning of a whole new chapter in .NET to me. Let's keep that alive! It's harder to organize the community than it is to ship a few thousand lines of code, but over the long-term it's the best thing for the ecosystem.

User experience also doesn't have to be compromised if we go this way. If people want to write:

loggerConfiguration.AddFile("logs/app.txt");

...there's no material differnce whether that AddFile() extension method lives in Microsoft.Extensions.Logging.File or Serilog.Extensions.Logging.File. The user will still need to search for a configuration example, they'll still need to install a NuGet package, and so-on.

However, if you build the APIs to make it possible, there is a big difference in the ecosystem. Instead of one AddFile() implementation, there will be many, and out of those, better and better options will grow.

If we set off down the path of trying to ship good-enough implementations of everything in the box, that's all we'll have in a couple of years.

I hope this is receieved in the spirit that it is written - I know my position seems biased by my involvement in a bunch of these community projects - but ultimately the most important thing to me is to see .NET grow and thrive.

@cwe1ss
Copy link
Contributor

cwe1ss commented Jul 12, 2016

I fully agree with what @nblumhardt is saying. I think this is very similar to the current situation in DependencyInjection.

I'm very afraid that supporting more providers in this logging implementation will lead to a lack of usage and innovation in well-established and feature-rich logging frameworks like Serilog, NLog etc! People will use this logging implementation because it's included by default, not realizing that there are many better solutions out there.

That's why I think, these questions are really important:

  • Why do you want to invest time and money into competing with existing, well-established logging frameworks like Serilog, NLog, log4net, ...?
  • Will you invest enough time into this library to make it competitive (simpler providers, periodic batching, ...)?
  • Are you ok with killing most of the market for existing 3rd party loggers? (Most people will always use what's included in the default template)
  • Will you create more sinks for popular solutions like ElasticSearch, Application Insights, ... or do you want the community to come up with yet another set of adapters?
  • Will you continue to advise people to use 3rd party loggers?
  • Specific for this provider: will you support rolling files, filename patterns, message patterns, ...?

IMO, If you truly want to participate in the .NET OSS community, you have to acknowledge the role of the ASP.NET Core web framework as being only one component in a huge ecosystem - and this huge ecosystem already has well-established solutions for logging and Dependency Injection. Having this in mind, your framework libraries should be designed with interoperability as the first priority. IMO this means that the implementation packages Microsoft.Extensions.Logging and Microsoft.Extensions.DependencyInjection should be completely optional - very similar to the choice of a server like Kestrel, WebListener, ... - but of course, that's a different story.

I understand that you need your own basic logging implementation for people that don't want to use 3rd party tools, but I'm also sure that this group of people will become smaller and smaller over the next 5-10 years because using OSS libraries clearly is the way our industry is headed towards in this new era of .NET Core. So, having your own implementation should always be a secondary priority.

PS: I also want to say that I really like working with .NET Core and ASP.NET Core. You are doing a really great job and you had a great impact on my involvement in OSS! Logging and DI are the only areas I'm having architectural concerns with.

@benaadams
Copy link
Contributor

Makes me sad this doesn't exist. Am in a situation on a remote system where I can't get Console, can't get Debug, so file is a fallback - don't care if its rubbish, just need to see what's going on; so will have to write my own file provider.

Don't want to use a 3rd party as something is going wrong and don't want to bring in extra dependencies to try and find out what as that may muddy the waters.

How about keeping it very simple, not add rolling logs etc and have a extension called loggerFactory.AddDebugFile or similar so people know its not a production logger? Would that work?

@brockallen
Copy link

Makes me sad this doesn't exist.

Not true -- look into Serilog. And given the engineering that is going into this issue, they're just reinventing serilog's file sink.

@benaadams
Copy link
Contributor

benaadams commented Sep 25, 2016

Yeah, take your point...

@RickStrahl
Copy link

RickStrahl commented Oct 7, 2016

Don't agree with Brock on this - this is such a basic core feature - it belongs in the framework. I think they should keep it simple and leave the complex stuff to full logging libs like Serilog, but creating a simple file logger without having to add a third party library seems intuitive to me. After all you can already log to console and a few other things without having to add anything and a basic file log is probably the first thing you think of when you talk about logging.

Don't make people jump through hoops to get basic and expected functionality.

@brockallen
Copy link

brockallen commented Oct 7, 2016

Don't agree with Brock on this - this is such a basic core feature - it belongs in the framework. I think they should keep it simple and leave the complex stuff to full logging libs like Serilog

I think we do agree -- if they're going to do this then it should stay dirt simple. It needs to be developer focused at dev time, not production server focused. Let Serilog be the big boy logger.

@ToddThomson
Copy link

"..creating a simple file logger without having to add a third party library seems intuitive to me."

Me too.

I'd also add that having the file logger sink inherit from a base batching / buffering async sink would be nice ( In most cases the logger should enqueue the log event and let the sink do the usually asynchronous work ). Add common logging configuration for filtering and I'd have everything I need.

@nblumhardt
Copy link
Contributor

For an idea of what a Serilog-provided AddFile() would look like: https://github.com/serilog/serilog-extensions-logging-file. CC @merbla.

From the README:

1. Add the NuGet package to the "dependencies" section of your project.json file:

    "dependencies": {
        "Serilog.Extensions.Logging.File": "1.0.0"
    }

2. In your Startup class's Configure() method, call AddFile() on the provided loggerFactory.

    public void Configure(IApplicationBuilder app,
                          IHostingEnvironment env,
                          ILoggerFactory loggerFactory)
    {
        loggerFactory.AddFile("Logs/myapp-{Date}.txt");

It supports an opinionated set of defaults (e.g. background thread for writes, RequestId and computed event id stamped in each log line), it's compatible with the default JSON "Logging" configuration in the project template, and as a bonus it can emit events in newline-separated JSON if required.

We'll see how it fares and report back on the experience :-)

@colin-young
Copy link

I see the point that @brockallen and @RickStrahl are making about a default, included, stupidly simple file logger, for development use, but, while the idea of just throwing in a basic file logger sounds simple, once you get into it, it starts to get a lot more complex: do we put an upper limit on file size? Roll-over files? Naming? How do we format the entries? Can that be customized? How do you ensure it doesn't have a negative impact on performance?

A good compromise might be:

  • simple file logger with fixed date-based file name, daily rollover, fixed output format
  • in m.e.Logging assembly, but under e.g. m.e.Logging.DevelopmentLogging extending ILoggerFactory
  • require some sort of configuration setting to enable it to avoid accidental use in production

@nblumhardt 's example demonstrates how easy it can be to incorporate a third-party logger. The more I think about it, the more I really don't think there should be a huge effort around an out-of-the-box file logger, other than ensuring it can't bring your system down.

@brockallen
Copy link

And to add to what @colin-young just said, since it seems @nblumhardt is making it even easier now, the steps to add Serilog and the exact same to add this proposed file logger. Since everything is NuGet, there is no such thing as an "in the box" anything anymore.

@colin-young
Copy link

I was suggesting something along the lines of what @benaadams was looking for, to avoid introducing new dependencies to a misbehaving production system, although when you get to deploying modified code to production, that's really only a small step away from adding a single new dependency in terms of "muddying the waters."

By in-the-box I meant in the same assembly so you would get it automatically as soon as you take a dependency on m.e.Logging.

I'm not really a fan of adding a file logger to the framework. I think it's a lot more complex than many first assume, and it invites feature-creep.

@hishamco
Copy link
Contributor

@muratg it's a duplicate of the #201 which I requested long time back and closed by you 😄

@muratg
Copy link
Author

muratg commented May 12, 2017

We are closing this issue because no further action is planned for this issue. If you still have any issues or questions, please log a new issue with any additional details that you have.

@muratg muratg closed this as completed May 12, 2017
@VitaliyMF
Copy link

For everyone who are looking for simple FileLoggerProvider without dependencies on the logging frameworks (Serilog, NLog, log4net), here is an implementation that works very similar to standard ConsoleLogger but writes result to a file: https://github.com/nreco/logging

@adams85
Copy link

adams85 commented Sep 22, 2017

Another implementation without 3rd party dependencies.
https://github.com/adams85/filelogger/

Full feature set of ConsoleLogger is covered and some more.

@mbodm
Copy link

mbodm commented Nov 28, 2017

Rick Strahl, a great real world developer who safed my ass a thousand times in 10 years, with his asp blog infos, and a lot of other great ppl here, are 100% right in what they say, imo. THIS is the community. Your community. Plz listen to them.

@sgf
Copy link

sgf commented Sep 6, 2018

@muratg if u want the problem get solove,u shouldn't close this issues.
in fact,u see most of ppls hope that(has a default Implement an official FileLogger)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests