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

MicrosoftExtensionsLogging: Add each log argument/attribute to the context data #2519

Open
tylerohlsen opened this issue May 31, 2024 · 10 comments
Labels
community To tag external issues and PRs

Comments

@tylerohlsen
Copy link

tylerohlsen commented May 31, 2024

Feature Description

Currently, with MEL, only log scopes are included in the context data (using logger.BeginScope(...)). The log attributes that are passed as arguments to the standard logger.Log* methods are not in the context data. They are included in the formatted message, but then they lose a lot of value because they need to be parsed back out during ingestion.

For example, explicit calls to the logger (or logger extension methods) like this do not have MyData as a context data value:

logger.LogInformation("My log message with some data: {MyData}", myData);

or via the Source Generator (generated) calls like this do not have the RouteData, MethodInfo, etc as context data values:

[LoggerMessage(102, LogLevel.Information, "Route matched with {RouteData}. Executing controller action with signature {MethodInfo} on controller {Controller} ({AssemblyName}).", EventName = "ControllerActionExecuting", SkipEnabledCheck = true)]
private static partial void ControllerActionExecuting(ILogger logger, string routeData, MethodInfo methodInfo, string controller, string? assemblyName);

Describe Alternatives

None considered

Additional context

I believe this already works as described with other loggers (e.g. Serilog and NLog), but I could be wrong.

Priority

Really Want

@workato-integration
Copy link

@github-actions github-actions bot added the community To tag external issues and PRs label May 31, 2024
@nrcventura
Copy link
Member

@tylerohlsen thank you for submitting this feature request. I passed the information along to other members of the team so that it can be further reviewed.

@dolivanu
Copy link

dolivanu commented Jun 4, 2024

Very useful to get the message attributes parsed by New Relic.

@tylerohlsen
Copy link
Author

@nrcventura I'm not seeing a linked PR nor release notes of a related change that would include what was requested here. Could you point me to the PR or release that included this request? Or is there a different reason why this was closed?

@nrcventura
Copy link
Member

Unfortunately, the tooling that product management uses for managing feature requests does not update github with meaningful information. From what I can tell in the other system, the feature request went from the "gathering interest" step to the "closed" step with a "resolution" of future consideration. I did not see any more information about it.

@tylerohlsen
Copy link
Author

Well that's disappointing. I can't recommend using NewRelic without this feature. Too much very important contextual information is lost without it.

@dolivanu
Copy link

dolivanu commented Feb 20, 2025

I agree with @tylerohlsen, this feature is already present in Python APM Agent and is the basis of Structured/Semantic Logging. Some projects use only Microsoft.Extensions.Logging without a logging framework or implementation like Serilog/NLog/log4net due to not requiring persisting/storing/forwarding the logs to any system except NR1. This approach is lighter and removes one dependency and some configuration effort.

@chynesNR
Copy link
Member

Hi @tylerohlsen (and @dolivanu ) -- apologies for the lack of communication. We did some restructuring of our internal Jira board and some Feature Requests auto-closed, and I missed that this needed an update.

Unfortunately this is one of those issues that sounds simple but isn't in practice. Unlike e.g. Serilog, or even Serilog-backed-MEL, there's no practical place for us to grab the attributes (And even just the context data was a challenge!). If we had a path forward we would add this support but at the moment we don't.

Thank you for communicating that this is still important to you. I'll reopen this Feature Request and we'll take another stab at it.

@chynesNR chynesNR reopened this Feb 20, 2025
@dolivanu
Copy link

Thanks @chynesNR, would be great if can provide extra clarifications:

  1. To achieve the desired behaviour with NR, in your opinion, the best approach is to use MEL as Log API and Serilog as Log Framework, so the dependency with Serilog is minimized across the project.
  2. When using MEL backed with Serilog, NR is able to capture the log attributes from the Serilog in order to add them to the context data. As Serilog is Open Source, could you review the source code of Serilog to get the path to capture the log attributes from MEL directly?

@chynesNR
Copy link
Member

Hi @dolivanu -- It sounds like you might be better off using one of our other log forwarding options so you don't have to rely on in-Agent forwarding. If you do want to stick with in-Agent forwarding, I believe using Serilog.Extensions.Logging should work for you (i.e. giving you MEL and structured logging).
On your second point: yes, exactly. We are able to identify and pull out the necessary context data from Serilog (as well as NLog), but because MEL allows many implementations of ILogger we don't usually know where the context data is kept.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community To tag external issues and PRs
Projects
None yet
Development

No branches or pull requests

4 participants