Add NLog instrumentation for OpenTelemetry .NET Auto-Instrumentation

[danifitz]

This commit implements a comprehensive NLog instrumentation that automatically bridges NLog logging events to OpenTelemetry without requiring code changes.

Features:

• Automatic target injection into NLog's target collection
• Complete log event bridging with level mapping
• Structured logging support with message templates
• Trace context integration
• Custom properties forwarding with filtering
• Comprehensive test coverage
• End-to-end test application
• Extensive documentation

The implementation follows the same patterns as the existing Log4Net instrumentation and supports NLog versions 4.0.0 through 6...

Configuration:

• OTEL_DOTNET_AUTO_LOGS_ENABLED=true
• OTEL_DOTNET_AUTO_LOGS_ENABLE_NLOG_BRIDGE=true
• OTEL_DOTNET_AUTO_LOGS_INCLUDE_FORMATTED_MESSAGE=true (optional)

Files added:

• src/OpenTelemetry.AutoInstrumentation/Instrumentations/NLog/
• test/OpenTelemetry.AutoInstrumentation.Tests/NLogTests.cs
• test/test-applications/integrations/TestApplication.NLog/

Files modified:

• Configuration classes to support NLog bridge
• Public API files to include new integration class

Why

https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/issues/3938

Fixes #3938

What

This PR implements a comprehensive NLog instrumentation that automatically bridges NLog logging events to OpenTelemetry without requiring any code changes from users.

Tests

• Unit Tests: 16 tests covering level mapping and edge cases
• Integration Tests: Complete end-to-end test application
• Compatibility: Verified on .NET 8.0 and .NET 9.0 with NLog 5.3.2

Checklist

• [x] CHANGELOG.md is updated.
• [x] Documentation is updated.
• [X] New features are covered by tests.

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: Kielek / name: Piotr Kiełkowicz (79a7cacff711f415c7e1898e4d8ed3aa54416a80, e0e9a38653caa607e2ec3f5bba510c17eae05143)

[danifitz]

I will update the changelog and documentation once we've verified that the code itself looks okay

[danifitz]

I took onboard the comments from @snakefoot and @Kielek and undertook a refactor to align more with the NLog way of doing things.

The key aspects of this refactor:

• Architecture Change: From pure CallTarget interception to a hybrid approach using a standard NLog Target
• New Project: OpenTelemetry.AutoInstrumentation.NLogTarget with OpenTelemetryTarget
• Auto-injection: NLogAutoInjector provides zero-config experience
• Standards Compliance: Uses NLog.Targets.TargetWithContext following NLog conventions
• Test Updates: Integration tests now follow the project's standard pattern
• Documentation: Updated to reflect the new dual-path approach

206be87 represents an improvement in the NLog instrumentation's architecture, making it more maintainable and aligned with NLog's intended usage patterns.

[danifitz]

Thank you @snakefoot for your speedy and detailed feedback, so invaluable. I've made some edits (all in one commit) 4401757 to address your feedback.

Summary of Changes

Some Architectural Improvements

• AsyncWrapper Compatibility: Replaced Activity.Current with Layout-based trace context resolution using TraceIdLayout and SpanIdLayout properties
• Performance Optimization: Added HasProperties check to avoid unnecessary dictionary allocation
• Standards Compliance: Removed custom Attributes property in favor of base class ContextProperties
• Layout Consistency: All rendering operations now use RenderLogEvent() pattern
• Property Logic Refinement: Enhanced parameter vs. properties handling logic
• Memory Efficiency: Eliminated dummy object creation in favor of null returns

[danifitz]

@zacharycmontoya @snakefoot @Kielek @matt-hensley whew this has been a journey - I believe I've taken onboard all your feedback and it's been through a few ups and downs and refactors. I hope we've arrived at a solution that pleases everyone and I've certainly learnt a lot. Please all have a review and let me know what you think

[danifitz]

@Kielek I made those changes in b078f85

[danifitz]

@Kielek @snakefoot @matt-hensley @zacharycmontoya I believe all comments/feedback resolved.

[danifitz]

Thanks for cleaning this up @Kielek - any further action needed from me?

[Kielek]

@danifitz, still reviewing. In general looks good, for applied small changes, it was easier to make commits directly than writing a comments. I hope you are fine with this.

[danifitz]

@danifitz, still reviewing. In general looks good, for applied small changes, it was easier to make commits directly than writing a comments. I hope you are fine with this.

I'm so fine with that :)

[danifitz]

I'm a bit busy over the next few days but will try and find some time to look into these comments next week or the weekend

[danifitz]

Still planning to resolve this but have been busy recently

[danifitz]

Hi @Kielek had a very busy period but found some time to review it! Please see latest changes

[ysolomchenko]

@danifitz, please add support for file-based configuration.

[danifitz]

@danifitz, please add support for file-based configuration.

Hi @ysolomchenko Just so I'm clear, do you mean support for adding an OTLP NLog target via Nlog.config in XML?

[ysolomchenko]

@danifitz, please add support for file-based configuration.

Hi @ysolomchenko Just so I'm clear, do you mean support for adding an OTLP NLog target via Nlog.config in XML?

No, there is a method OnLoadFile in LogSettings.cs - it needs to enable the bridge as well.

You need to add NLOG as an instrumentation option with NlogBridge (similar to how it’s done for Log4Net) in the DotNetLogs.cs file. You can use the same class for this (just rename it) from Log4NetBridgeEnabled -> BridgeEnabled

Also, update the tests to include NLOG:

• ParserInstrumentationTests.cs
• FilebasedInstrumentationSettingsTests.cs

And finally, update the documentation for file-based configuration:

• file-based-configuration.md
• and the example files under examples/file-based-configuration-files.

Maybe you need to resolve the conflicts first before making these changes.

[zacharycmontoya]

@ysolomchenko can we defer updates for file-based configuration to a later PR? I'd like to avoid adding scope to this PR that's already quite large.

[danifitz]

Hi - I found a few issues with my implementation so I performed another refactor.

Summary

Refactors the NLog instrumentation to intercept WriteToTargets/WriteLogEventToTargets instead of Logger.Log. The previous approach only captured explicit Logger.Log() calls, missing all convenience methods (log.Info(), log.Debug(), etc.) which bypass Logger.Log internally.

Changes

New integrations targeting internal NLog methods:

• WriteToTargetsIntegration - NLog 5.3.0+
• WriteToTargetsLegacyIntegration - NLog 5.0.0-5.2.x
• WriteLogEventToTargetsIntegration - NLog 6.x
• NLogIntegrationHelper - shared logic to reduce duplication

Removed LoggerIntegration - no longer needed

Fixed log record attributes:

• Exception handling now uses RecordException for proper exception.type, exception.message, exception.stacktrace attributes
• Parameter keys no longer have arg_ prefix
• GlobalDiagnosticsContext properties are now captured

Updated tests to match new output format and verify trace context injection

All integration tests pass on net9.0.

[danifitz]

Thanks to @lmolkova I found an issue with .net framework (I haven't been able to test it).

The issue was that .NET Framework's expression tree compiler doesn't support TryExpression when the result is passed directly to a method with ref parameters.

The fix: Assign the logRecordExpression and attributesExpression results to local variables first Then pass those local variables by reference to the EmitLog method

[Kielek]

@danifitz, I have merged main plus adjusted couple of places/

Common issues in the CI - most of .NET/NETFx/system combination:

  [ERR] RunManagedIntegratio: [xUnit.net 00:05:19.50]     IntegrationTests.NLogBridgeTests.TraceContext_IsInjectedIntoCurrentNLogLogsDestination(packageVersion: "6.0.0") [FAIL]
  [ERR] RunManagedIntegratio: [xUnit.net 00:05:20.43]     IntegrationTests.NLogBridgeTests.TraceContext_IsInjectedIntoCurrentNLogLogsDestination(packageVersion: "6.0.6") [FAIL]
  [ERR] RunManagedIntegratio: [xUnit.net 00:05:21.37]     IntegrationTests.NLogBridgeTests.TraceContext_IsInjectedIntoCurrentNLogLogsDestination(packageVersion: "5.3.4") [FAIL]
  [ERR] RunManagedIntegratio: [xUnit.net 00:05:22.24]     IntegrationTests.NLogBridgeTests.TraceContext_IsInjectedIntoCurrentNLogLogsDestination(packageVersion: "5.0.0") [FAIL]

[danifitz]

@danifitz, I have merged main plus adjusted couple of places/

Common issues in the CI - most of .NET/NETFx/system combination:

  [ERR] RunManagedIntegratio: [xUnit.net 00:05:19.50]     IntegrationTests.NLogBridgeTests.TraceContext_IsInjectedIntoCurrentNLogLogsDestination(packageVersion: "6.0.0") [FAIL]
  [ERR] RunManagedIntegratio: [xUnit.net 00:05:20.43]     IntegrationTests.NLogBridgeTests.TraceContext_IsInjectedIntoCurrentNLogLogsDestination(packageVersion: "6.0.6") [FAIL]
  [ERR] RunManagedIntegratio: [xUnit.net 00:05:21.37]     IntegrationTests.NLogBridgeTests.TraceContext_IsInjectedIntoCurrentNLogLogsDestination(packageVersion: "5.3.4") [FAIL]
  [ERR] RunManagedIntegratio: [xUnit.net 00:05:22.24]     IntegrationTests.NLogBridgeTests.TraceContext_IsInjectedIntoCurrentNLogLogsDestination(packageVersion: "5.0.0") [FAIL]

@Kielek should be fixed now - wasn't handling different formats correctly in the tests

[danifitz]

Hi @Kielek the failing CI test seems to be unrelated to the NLog changes:

<Message>Assert.InRange() Failure: Value not in range
Range:  (50 - 70)
Actual: 0</Message>
          <StackTrace>   at IntegrationTests.SelectiveSamplerTests.ExportThreadSamples() in /home/runner/work/opentelemetry-dotnet-instrumentation/opentelemetry-dotnet-instrumentation/test/IntegrationTests/SelectiveSamplerTests.cs:line 51
   at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** arguments, ObjectHandleOnStack sig, BOOL isConstructor, ObjectHandleOnStack result)
   at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** arguments, ObjectHandleOnStack sig, BOOL isConstructor, ObjectHandleOnStack result)
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)</StackTrace>

[Kielek]

Reviewed your changes (and pushed some fixes). The scope of the instrumented methods should be fine, but you missing coverage for them. You need to modify test application and test assertion to cover methods with Type wrapperType.