juliuskoval / NLog.Targets.OpenTelemetryProtocol

1 stars 4 forks source link

Inherit from TargetWithContext and use NLog Layouts #2

Closed snakefoot closed 6 months ago

snakefoot commented 6 months ago

Updated to use the TargetWithContext that makes it easier to capture NLog-context:

Replaced the following properites with ArrayParameter:

Change the following properties to use NLog Layout (Allow target-configuration using ${configsetting})

Added the following new properties:

snakefoot commented 6 months ago

But really weird that one have to go through LogRecordData to be able to publish the actual LogRecord.

Because of this translation, then one suddenly looses the ability to assign properties:

juliuskoval commented 6 months ago

Hi so the thing is that OpenTelemetry logging was originally made to work with Microsoft's ILogger and I'm using the bridge API, which is supposed to connect OpenTelemetry logging to existing logging frameworks. This API is only intended for backend developers, so it kind of feels like an afterthought. It also isn't stable yet.

I created a pull request which would allow us to set CategoryName, but it was denied.

Out of curiosity, how did you find the repo so quickly?

snakefoot commented 6 months ago

Out of curiosity, how did you find the repo so quickly?

I sometimes use GitHub-Repository-search to check for new interesting NLog extensions. The "OpenTelemetryProtocol" caught my eye, as others have requested such an integration: https://github.com/NLog/NLog/issues/5352

snakefoot commented 6 months ago

I created a https://github.com/open-telemetry/opentelemetry-dotnet/pull/5275 which would allow us to set CategoryName, but it was denied.

If they accept that the Logger-name is mapped to Category-Name. Then I guess this NLog Target could maintain a dictionary of OpenTelemetry-Loggers. And lookup/create Logger based on LogEventInfo.Logger-string-value.

juliuskoval commented 6 months ago

Yes, that's what I was thinking. Something like this https://github.com/dotnet/runtime/blob/e3ff66bf5afdcb85a642e69962ed5f6575751a35/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactory.cs#L139

juliuskoval commented 6 months ago

Alright, the PR looks good. Thank you for the contribution.