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

Add Tags and Baggage to LogScope using ActivityTrackingOptions #46571

Closed
msallin opened this issue Jan 5, 2021 · 16 comments
Closed

Add Tags and Baggage to LogScope using ActivityTrackingOptions #46571

msallin opened this issue Jan 5, 2021 · 16 comments
Labels
api-approved API was approved in API review, it can be implemented area-Extensions-Logging
Milestone

Comments

@msallin
Copy link
Contributor

msallin commented Jan 5, 2021

Background and Motivation

Since .NET 5 the LoggerFactoryScopeProvider can be configured using the ActivityTrackingOptions, to add the SpanId, TraceId, ParentId, TraceStateString and TraceFlags to the scope. The SpanId, TraceId and ParentId are added by default when using Host.CreateDefaultBuilder().

In our use case, we implement a own ConsoleFormatter which writes the log lines as key-value-pairs, to be able to easy search them with Splunk. We want to add the SpandId, TraceId, ParentId and all items from baggage and tags to each log line.
2021-55-05 08:01:01.075 level=INFO eventName=RequestFinished eventId=2 sourceContext="Microsoft.AspNetCoreHosting.Diagnostics" message="Request finished HTTP/1.1 GET http://localhost/ - - - 200- - 101.8717ms" ElapsedMilliseconds="101.8717" StatusCode="200" ContentType="" ContentLength="" Protocol="HTTP/1.1" Method="GET" Scheme="http" Host="localhost" PathBase="" Path="/" QueryString="" SpanId="976b6fa436d88a4e" TraceId="a23f2582c9522c42bce68662015ed2f1" ParentId="0000000000000000" RequestId="0HM5HAUAVSI9J" RequestPath="/"

added some more text to the proposal code by @tarekgh

Proposed API

using System;

namespace Microsoft.Extensions.Logging
{
    [Flags]
    public enum ActivityTrackingOptions
    {
        None        = 0x0000,
        SpanId      = 0x0001,
        TraceId     = 0x0002,
        ParentId    = 0x0004,
        TraceState  = 0x0008,
        TraceFlags  = 0x0010,

+       Tags        = 0x0020,
+       Baggage     = 0x0040,
    }
}

Usage Examples

namespace Microsoft.Extensions.Hosting
{
    public static class Host
    {
        public static IHostBuilder CreateDefaultBuilder(string[] args)
        {
            var builder = new HostBuilder();
 
            builder.UseContentRoot(Directory.GetCurrentDirectory());

            builder.ConfigureAppConfiguration((hostingContext, config) =>
            {
                //...
            })
            .ConfigureLogging((hostingContext, logging) =>
            {
                logging.AddConfiguration(hostingContext.Configuration.GetSection("Logging"));
                logging.AddConsole();

                logging.Configure(options =>
                {
                    options.ActivityTrackingOptions = ActivityTrackingOptions.SpanId
                                                        | ActivityTrackingOptions.TraceId
                                                        | ActivityTrackingOptions.ParentId
+                                                        | ActivityTrackingOptions.Baggage
+                                                        | ActivityTrackingOptions.Tags;
                });
 
            })

            return builder;
        }
    }
}

Alternative Designs

  • Make it possible to subclass LoggerFactoryScopeProvider and add the behavior this way.
  • Use another Enum because ActivityTrackingOptions currently represents only parts of the W3C spec

Risks

No risks.

@msallin msallin added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 5, 2021
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Jan 5, 2021
@msallin
Copy link
Contributor Author

msallin commented Jan 5, 2021

area label: area-Extensions-Logging

@msallin
Copy link
Contributor Author

msallin commented Jan 5, 2021

@ghost
Copy link

ghost commented Jan 5, 2021

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and Motivation

Since .NET 5 the LoggerFactoryScopeProvider can be configured using the ActivityTrackingOptions, to add the SpanId, TraceId, ParentId, TraceStateString and TraceFlags to the scope. The SpanId, TraceId and ParentId are added by default when using Host.CreateDefaultBuilder().

In our use case, we implement a own ConsoleFormatter which writes the log lines as key-value-pairs, to be able to easy search them with Splunk. We want to add the SpandId, TraceId, ParentId and all items from baggage and tags to each log line.
2021-55-05 08:01:01.075 level=INFO eventName=RequestFinished eventId=2 sourceContext="Microsoft.AspNetCoreHosting.Diagnostics" message="Request finished HTTP/1.1 GET http://localhost/ - - - 200- - 101.8717ms" ElapsedMilliseconds="101.8717" StatusCode="200" ContentType="" ContentLength="" Protocol="HTTP/1.1" Method="GET" Scheme="http" Host="localhost" PathBase="" Path="/" QueryString="" SpanId="976b6fa436d88a4e" TraceId="a23f2582c9522c42bce68662015ed2f1" ParentId="0000000000000000" RequestId="0HM5HAUAVSI9J" RequestPath="/"

Proposed API

using System;

namespace Microsoft.Extensions.Logging
{
    [Flags]
    public enum ActivityTrackingOptions
    {
+       Tags  = ...
+       Baggage  = ...
    }
}

Usage Examples

namespace Microsoft.Extensions.Hosting
{
    public static class Host
    {
        public static IHostBuilder CreateDefaultBuilder(string[] args)
        {
            var builder = new HostBuilder();
 
            builder.UseContentRoot(Directory.GetCurrentDirectory());

            builder.ConfigureAppConfiguration((hostingContext, config) =>
            {
                //...
            })
            .ConfigureLogging((hostingContext, logging) =>
            {
                logging.AddConfiguration(hostingContext.Configuration.GetSection("Logging"));
                logging.AddConsole();

                logging.Configure(options =>
                {
                    options.ActivityTrackingOptions = ActivityTrackingOptions.SpanId
                                                        | ActivityTrackingOptions.TraceId
                                                        | ActivityTrackingOptions.ParentId
+                                                        | ActivityTrackingOptions.Baggage
+                                                        | ActivityTrackingOptions.Tags;
                });
 
            })

            return builder;
        }
    }
}

Alternative Designs

  • Make it possible to subclass LoggerFactoryScopeProvider and add the behavior this way.
  • Use another Enum because ActivityTrackingOptions currently represents only parts of the W3C spec

Risks

No risks.

Author: msallin
Assignees: -
Labels:

api-suggestion, area-Extensions-Hosting, untriaged

Milestone: -

@ghost
Copy link

ghost commented Jan 5, 2021

Tagging subscribers to this area: @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and Motivation

Since .NET 5 the LoggerFactoryScopeProvider can be configured using the ActivityTrackingOptions, to add the SpanId, TraceId, ParentId, TraceStateString and TraceFlags to the scope. The SpanId, TraceId and ParentId are added by default when using Host.CreateDefaultBuilder().

In our use case, we implement a own ConsoleFormatter which writes the log lines as key-value-pairs, to be able to easy search them with Splunk. We want to add the SpandId, TraceId, ParentId and all items from baggage and tags to each log line.
2021-55-05 08:01:01.075 level=INFO eventName=RequestFinished eventId=2 sourceContext="Microsoft.AspNetCoreHosting.Diagnostics" message="Request finished HTTP/1.1 GET http://localhost/ - - - 200- - 101.8717ms" ElapsedMilliseconds="101.8717" StatusCode="200" ContentType="" ContentLength="" Protocol="HTTP/1.1" Method="GET" Scheme="http" Host="localhost" PathBase="" Path="/" QueryString="" SpanId="976b6fa436d88a4e" TraceId="a23f2582c9522c42bce68662015ed2f1" ParentId="0000000000000000" RequestId="0HM5HAUAVSI9J" RequestPath="/"

Proposed API

using System;

namespace Microsoft.Extensions.Logging
{
    [Flags]
    public enum ActivityTrackingOptions
    {
+       Tags  = ...
+       Baggage  = ...
    }
}

Usage Examples

namespace Microsoft.Extensions.Hosting
{
    public static class Host
    {
        public static IHostBuilder CreateDefaultBuilder(string[] args)
        {
            var builder = new HostBuilder();
 
            builder.UseContentRoot(Directory.GetCurrentDirectory());

            builder.ConfigureAppConfiguration((hostingContext, config) =>
            {
                //...
            })
            .ConfigureLogging((hostingContext, logging) =>
            {
                logging.AddConfiguration(hostingContext.Configuration.GetSection("Logging"));
                logging.AddConsole();

                logging.Configure(options =>
                {
                    options.ActivityTrackingOptions = ActivityTrackingOptions.SpanId
                                                        | ActivityTrackingOptions.TraceId
                                                        | ActivityTrackingOptions.ParentId
+                                                        | ActivityTrackingOptions.Baggage
+                                                        | ActivityTrackingOptions.Tags;
                });
 
            })

            return builder;
        }
    }
}

Alternative Designs

  • Make it possible to subclass LoggerFactoryScopeProvider and add the behavior this way.
  • Use another Enum because ActivityTrackingOptions currently represents only parts of the W3C spec

Risks

No risks.

Author: msallin
Assignees: -
Labels:

api-suggestion, area-Extensions-Logging, untriaged

Milestone: -

@msallin
Copy link
Contributor Author

msallin commented Jan 28, 2021

CC: @tarekgh @shirhatti

@tarekgh tarekgh removed the untriaged New issue has not been triaged by the area owner label Jan 28, 2021
@tarekgh tarekgh added this to the 6.0.0 milestone Jan 28, 2021
@tarekgh
Copy link
Member

tarekgh commented Jan 28, 2021

@msallin

Tags now are string-object pairs and not string-string. What you suggest in formatting the tags? just call ToString on the value? or need to provide custom formatter for that?

CC @noahfalk @maryamariyan

@tarekgh
Copy link
Member

tarekgh commented Jan 28, 2021

CC @davidfowl

@msallin
Copy link
Contributor Author

msallin commented Jan 29, 2021

In our use case we attach only strings as tags. Thus, its fine to just use ToString. Using a formatter would be useful if one adds objects which one does not have control. Else one could still overwrite ToString .

@tarekgh
Copy link
Member

tarekgh commented Feb 12, 2021

@davidfowl @shirhatti the proposal looks reasonable to me. Do you have any concern bout it before we move forward?

@davidfowl
Copy link
Member

As long as it's not on by default, looks good.

@tarekgh tarekgh added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Feb 12, 2021
@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Feb 23, 2021
@terrajobst
Copy link
Member

terrajobst commented Feb 23, 2021

Video

  • Looks good as proposed
namespace Microsoft.Extensions.Logging
{
    public partial enum ActivityTrackingOptions
    {
        Tags        = 0x0020,
        Baggage     = 0x0040,
    }
}

@msallin
Copy link
Contributor Author

msallin commented Feb 23, 2021

I would love to contribute this tiny part to .NET
Let me know when/if you are ready to accept a PR.

@tarekgh
Copy link
Member

tarekgh commented Feb 23, 2021

@msallin go ahead and submit the PR and tag me there. Thanks for willing to help with that.

@tarekgh
Copy link
Member

tarekgh commented Feb 24, 2021

@msallin please let me know if you need any help or have any question. Thanks!

msallin added a commit to msallin/runtime that referenced this issue Mar 12, 2021
tarekgh pushed a commit that referenced this issue Mar 12, 2021
Co-authored-by: Sallin Marc, I212 <marc.sallin@post.ch>
@tarekgh
Copy link
Member

tarekgh commented Mar 12, 2021

Fixed by #48722

@tarekgh tarekgh closed this as completed Mar 12, 2021
ML, Extensions, Globalization, etc, POD. automation moved this from 6.0.0 to Done Mar 12, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-Extensions-Logging
Projects
None yet
Development

No branches or pull requests

7 participants