Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infrastructure: Improve experiece for filtering log data #7217

Closed
rowanmiller opened this issue Dec 8, 2016 · 6 comments
Closed

Infrastructure: Improve experiece for filtering log data #7217

rowanmiller opened this issue Dec 8, 2016 · 6 comments
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@rowanmiller
Copy link
Contributor

rowanmiller commented Dec 8, 2016

At the moment you need to filter by category name, which has some issues:

  • It's hard to work out the full set of categories you want
  • You are potentially referencing internal types
  • Type names can change (i.e. you want Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommandBuilderFactory in 1.0 and Microsoft.EntityFrameworkCore.Storage.IRelationalCommandBuilderFactory in 1.1)

Even once you do this, it is hard to filter to the events that you want. Here is an attempt to filter to commands being executed, but EventIds are not globally unique... so there is no guarantee that I just get RelationalEventId.ExecutedCommand events. Even in this example, where I am just logging for one component, there is no guarantee that the command builder factory won't log something related to the EventId in core that collides with the id of RelationalEventId.ExecutedCommand.

The types we pass to state also make is pretty cumbersome to get the data I want (command text).

    public class SqlLoggerProvider : ILoggerProvider
    {
        public ILogger CreateLogger(string categoryName)
        {
            if (categoryName == typeof(Microsoft.EntityFrameworkCore.Storage.IRelationalCommandBuilderFactory).FullName)
            {
                return new SqlLogger(categoryName);
            }

            return new NullLogger();
        }

        public void Dispose()
        { }

        private class SqlLogger : ILogger
        {
            private string _test;

            public SqlLogger(string test)
            {
                _test = test;
            }

            public bool IsEnabled(LogLevel logLevel)
            {
                return true;
            }

            public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
            {
                if (eventId.Id == (int)RelationalEventId.ExecutedCommand)
                {
                    var data = state as IEnumerable<KeyValuePair<string, object>>;
                    if (data != null)
                    {
                        var commandText = data.Single(p => p.Key == "CommandText").Value;
                        Console.WriteLine(commandText);
                    }
                }
            }

            public IDisposable BeginScope<TState>(TState state)
            {
                return null;
            }
        }

        private class NullLogger : ILogger
        {
            public bool IsEnabled(LogLevel logLevel)
            {
                return false;
            }

            public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
            { }

            public IDisposable BeginScope<TState>(TState state)
            {
                return null;
            }
        }
    }
@rowanmiller
Copy link
Contributor Author

BTW the quickest win would be to set EventId.Name to the name of the enum member (currently they are all null).

@Ponant
Copy link

Ponant commented Dec 8, 2016

@rowanmiller I think the example you gave in the docs has the advantage of returning more info, e.g. execution time.

@rowanmiller
Copy link
Contributor Author

@Ponant you can do that by calling the formatter, same as the docs. I just thought you were specifically after the command text 😄

@Ponant
Copy link

Ponant commented Dec 12, 2016

Yes, you are right. At least I learned that the exec command is not part of the command text. In any case I implemented your code above and will follow up this issue. Cheers

@divega divega added this to the 2.0.0 milestone Jan 3, 2017
ajcvickers added a commit that referenced this issue Apr 12, 2017
Issues #7217 #218

This change:
* Consolidates interception and sensitive logging into one service and uses that service everywhere in the EF code.
* Adds a LoggerCategory class which contains nested classes that define EF logging categories.
* Used together, this means any EF service can depend on `IInterceptingLogger<TLoggingCategory>` and have D.I. fill create the correct service using its open generics feature. The TLoggingCategory is constrained so that only types specifically defined as LoggingCategories can be used. For example, `IInterceptingLogger<LoggingCategory.Database.Sql>`.
* Application code can get logging categories using, for example, `LoggingCategory.Database.Sql.Name`. This means that any application can immediately see in code what logging categories there are to do appropriate filtering.
* All services depend on the specific generic `IInterceptingLogger<TLoggingCategory>` that they will use--never ILogger. This makes it very clear which category is being logged to and makes it safe to share loggers where safe, and impossible where the categories should be different.
ajcvickers added a commit that referenced this issue Apr 13, 2017
Issues #7217 #218

This change:
* Consolidates interception and sensitive logging into one service and uses that service everywhere in the EF code.
* Adds a LoggerCategory class which contains nested classes that define EF logging categories.
* Used together, this means any EF service can depend on `IInterceptingLogger<TLoggingCategory>` and have D.I. fill create the correct service using its open generics feature. The TLoggingCategory is constrained so that only types specifically defined as LoggingCategories can be used. For example, `IInterceptingLogger<LoggingCategory.Database.Sql>`.
* Application code can get logging categories using, for example, `LoggingCategory.Database.Sql.Name`. This means that any application can immediately see in code what logging categories there are to do appropriate filtering.
* All services depend on the specific generic `IInterceptingLogger<TLoggingCategory>` that they will use--never ILogger. This makes it very clear which category is being logged to and makes it safe to share loggers where safe, and impossible where the categories should be different.
ajcvickers added a commit that referenced this issue Apr 13, 2017
Issues #7217 #218

This change:
* Consolidates interception and sensitive logging into one service and uses that service everywhere in the EF code.
* Adds a LoggerCategory class which contains nested classes that define EF logging categories.
* Used together, this means any EF service can depend on `IInterceptingLogger<TLoggingCategory>` and have D.I. fill create the correct service using its open generics feature. The TLoggingCategory is constrained so that only types specifically defined as LoggingCategories can be used. For example, `IInterceptingLogger<LoggingCategory.Database.Sql>`.
* Application code can get logging categories using, for example, `LoggingCategory.Database.Sql.Name`. This means that any application can immediately see in code what logging categories there are to do appropriate filtering.
* All services depend on the specific generic `IInterceptingLogger<TLoggingCategory>` that they will use--never ILogger. This makes it very clear which category is being logged to and makes it safe to share loggers where safe, and impossible where the categories should be different.
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Apr 13, 2017
@ajcvickers ajcvickers changed the title Improve experiece for filtering log data Infrastructure: Improve experiece for filtering log data May 9, 2017
@demodav
Copy link

demodav commented Aug 30, 2018

How does one use this code? How do you execute it please provide an example.

@ajcvickers
Copy link
Member

@demodav This is old code used to describe an issue--it is not intended to run on recent releases.

@ajcvickers ajcvickers modified the milestones: 2.0.0-preview1, 2.0.0 Oct 15, 2022
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

No branches or pull requests

5 participants