open-telemetry / opentelemetry-dotnet-contrib

This repository contains set of components extending functionality of the OpenTelemetry .NET SDK. Instrumentation libraries, exporters, and other components can find their home here.
https://opentelemetry.io
Apache License 2.0
474 stars 283 forks source link

No Distinct CustomFields option for different ILogger Categories #1188

Open sunildatla opened 1 year ago

sunildatla commented 1 year ago

Issue with OpenTelemetry.Exporter.Geneva

List of all OpenTelemetry NuGet packages and version that you are using (e.g. OpenTelemetry 1.3.2):

Runtime version (e.g. net462, net48, net6.0, net7.0 etc. You can find this information from the *.csproj file):

Is this a feature request or a bug?

Feature Request and/or Bug

What is the expected behavior?

What do you expect to see?

*There should be different custom field sections for different ILogger categories like how we have TableNameMappings dictionary .

What is the actual behavior?

What did you see instead? If you are reporting a bug, create a self-contained project using the template of your choice and apply the minimum required code to result in the issue you're observing. We will close this issue if:

Additional Context

Add any other context about the feature request here.

cijothomas commented 1 year ago

Related/same https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/757

sunildatla commented 1 year ago

@cijothomas does this mean this support will not be considered for future as well? We have a lot of legacy loggers /tables with lot of different columns like the above #757 issue , its hard to have one customfields option for all the tables together as it can be 100s of columns and not everything is related to one table. This was addressed in later tables by standardizing the schema but we need to still support the legacy tables before they migrate away at some point to later standardized schemas and they are 100s of tables .

sunildatla commented 1 year ago

@cijothomas any update on this

jeffreyrivor commented 1 year ago

One other thing to consider is when an ILogger category is coming from another NuGet package, where you're given the column names by whatever the library hard-coded (in practice, no library has a configuration point where it can be told to remap the names in its ILogger message template). I also think the notion of "standardizing the schema" across every library a developer may use will not scale.

A concrete case I'm running into is LoggingHttpMessageHandler from Microsoft.Extensions.Http. It seems logical for it to log "HttpMethod" and "Uri" as column names, but I may not necessarily want something as generic as "Uri" to be a column in every other table.

Another concrete case I have involves user code logging with Log scopes that has a dependency on HttpClient (and subsequently is setup with LoggingHttpMessageHandler in the pipeline by the implementation of IHttpClientFactory.

public class MyBusinessLogicClass
{
    private readonly ILogger<MyBusinessLogicClass> _logger;
    private readonly HttpClient _httpClient;

    public MyBusinessLogicClass(ILogger<MyBusinessLogicClass> logger, HttpClient httpClient)
    {
        _logger = logger;
        _httpClient = httpClient;
    }

    public async Task<string> DoWorkAsync(string logicParameter, CancellationToken ct)
    {
        using (_logger.BeginScope("Beginning work for {logicParameter}", logicParameter))
        using (var httpRequest = new HttpRequestMessage(
            HttpMethod.GET,
            $"https://something.com/api/do-work?logicParameter={logicParameter}")
        {
            using var response = await _httpClient.SendAsync(httpRequest, ct);
            response.EnsureSuccessStatusCode();
            var responseBodyText = await response.Content.ReadAsStringAsync(ct);
            _logger.LogTrace("Response is {response}", responseBodyText);
            return responseBodyText;
        }
    }
}

In this case, the HttpClient logs end up with the "logicParameter" column as well. If I add "logicParameter" for the CustomFields and set separate tables for "HttpClient" and "MyBusinessLogicClass", both tables will end up with the "logicParameter", and if HttpClient is used by other classes in the same way, then the "logicParameter" column will be sparsely populated in the "HttpClient" table anyhow; ideally, it wouldn't be a separate column in the "HttpClient" table.