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

[API Proposal]: Make FormattedLogValues class public #79149

Closed
foreverstupid opened this issue Dec 2, 2022 · 3 comments
Closed

[API Proposal]: Make FormattedLogValues class public #79149

foreverstupid opened this issue Dec 2, 2022 · 3 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-Logging

Comments

@foreverstupid
Copy link

Background and motivation

Hello. I've decided to create a custom logging provider, that should support structure logging. In the most of the cases, my code uses logging of the format form, e.g.:

_logger.LogInformation("Service has started at {StartDateTime}", DateTime.UtcNow);

So, I wanted to use named format fields as additional properties to the log. But, the interface ILogger of course has only the method that logs a message with a generic state. I've decided to look, how other libraries (e.g. NLog or Serilog) had solved this issue, and it turned out, that they check the type of the state to be IEnumerable<KeyValuePair<string, object>> and more over they use a "magical" knowledge, that this collection has the format of the message as the last item (or the item, that has the key {OriginalFormat}). I don't think, that such shadow knowledge is reliable enough to be used.

After the deeper research into the Microsoft.Extensions.Logging assembly source code, I've found out that all extensions of the form Log***(string format, object[] args) create an instance of the special class FormattedLogValues as a message state. So my idea was to check a message state type to be FormattedLogValues. But unfortunately this class is internal. Therefore my proposal is to make this class public and moreover give it a special property that holds an original message format.

In sum, the motivation of this feature is to clarify hidden logic, that is used for creating formatted messages. So developers can rely on it and be sure that this logic will not change in the future.

API Proposal

using System;
using System.Collections;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Threading;

namespace Microsoft.Extensions.Logging
{
    /// <summary>
    /// LogValues to enable formatting options supported by <see cref="string.Format(IFormatProvider, string, object?)"/>.
    /// This also enables using {NamedformatItem} in the format string.
    /// </summary>
    public readonly struct FormattedLogValues : IReadOnlyList<KeyValuePair<string, object?>>
    {
        // We have to keep the original content as it is for backward compatibility. For example,
        // the original format is still the last item of the collection and has the key "{OriginalFormat}".
        // But we can extend the interface, giving appropriate ways to get the format and its arguments.

        /// Gets the original message format.
        public string OriginalFormat
        {
            get => this["{OriginalFormat}"];
        }

        /// Gets the collection of the format arguments with their names.
        public IEnumerable<KeyValuePair<string, object?>> FormatArgs
        {
            get
            {
                for (int i = 0; i < Count - 1; ++i)
                {
                    yield return this[i];
                }
            }
        }

        // the rest as it is
        ...
    }
}

API Usage

    public class CustomLogger : ILogger
    {
        public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
        {
            if (state is FormattedLogValues formattedValues)
            {
                var format = formattedValues.OriginalFormat;
                var args = formattedValues.FormatArgs;
                ...
            }
            else
            {
                ...
            }
        }

        ...
    }

Alternative Designs

No response

Risks

No response

@foreverstupid foreverstupid added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Dec 2, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Dec 2, 2022
@ghost
Copy link

ghost commented Dec 2, 2022

Tagging subscribers to this area: @dotnet/area-extensions-logging
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Hello. I've decided to create a custom logging provider, that should support structure logging. In the most of the cases, my code uses logging of the format form, e.g.:

_logger.LogInformation("Service has started at {StartDateTime}", DateTime.UtcNow);

So, I wanted to use named format fields as additional properties to the log. But, the interface ILogger of course has only the method that logs a message with a generic state. I've decided to look, how other libraries (e.g. NLog or Serilog) had solved this issue, and it turned out, that they check the type of the state to be IEnumerable<KeyValuePair<string, object>> and more over they use a "magical" knowledge, that this collection has the format of the message as the last item (or the item, that has the key {OriginalFormat}). I don't think, that such shadow knowledge is reliable enough to be used.

After the deeper research into the Microsoft.Extensions.Logging assembly source code, I've found out that all extensions of the form Log***(string format, object[] args) create an instance of the special class FormattedLogValues as a message state. So my idea was to check a message state type to be FormattedLogValues. But unfortunately this class is internal. Therefore my proposal is to make this class public and moreover give it a special property that holds an original message format.

In sum, the motivation of this feature is to clarify hidden logic, that is used for creating formatted messages. So developers can rely on it and be sure that this logic will not change in the future.

API Proposal

using System;
using System.Collections;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Threading;

namespace Microsoft.Extensions.Logging
{
    /// <summary>
    /// LogValues to enable formatting options supported by <see cref="string.Format(IFormatProvider, string, object?)"/>.
    /// This also enables using {NamedformatItem} in the format string.
    /// </summary>
    public readonly struct FormattedLogValues : IReadOnlyList<KeyValuePair<string, object?>>
    {
        // We have to keep the original content as it is for backward compatibility. For example,
        // the original format is still the last item of the collection and has the key "{OriginalFormat}".
        // But we can extend the interface, giving appropriate ways to get the format and its arguments.

        /// Gets the original message format.
        public string OriginalFormat
        {
            get => this["{OriginalFormat}"];
        }

        /// Gets the collection of the format arguments with their names.
        public IEnumerable<KeyValuePair<string, object?>> FormatArgs
        {
            get
            {
                for (int i = 0; i < Count - 1; ++i)
                {
                    yield return this[i];
                }
            }
        }

        // the rest as it is
        ...
    }
}

API Usage

    public class CustomLogger : ILogger
    {
        public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
        {
            if (state is FormattedLogValues formattedValues)
            {
                var format = formattedValues.OriginalFormat;
                var args = formattedValues.FormatArgs;
                ...
            }
            else
            {
                ...
            }
        }

        ...
    }

Alternative Designs

No response

Risks

No response

Author: foreverstupid
Assignees: -
Labels:

api-suggestion, area-Extensions-Logging

Milestone: -

@gfoidl
Copy link
Member

gfoidl commented Dec 2, 2022

Similar #67577

@tarekgh
Copy link
Member

tarekgh commented Dec 2, 2022

Closing as a duplicate of #67577.

@foreverstupid feel free to comment and add any thoughts there.

@tarekgh tarekgh closed this as completed Dec 2, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Dec 2, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jan 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-Logging
Projects
None yet
Development

No branches or pull requests

3 participants