feat(logs): add NLog integration - #5176
1 issue
code-review: Found 1 issue (1 medium)
Medium
NLog CaptureStructuredLog passes SdkVersion where SetDefaultAttributes expects Scope - `src/Sentry.NLog/SentryTarget.cs:354`
In SentryTarget.Structured.cs, CaptureStructuredLog calls log.SetDefaultAttributes(options, Sdk), passing the static SdkVersion Sdk field as the second argument. However, SentryLog.SetDefaultAttributes has the signature (SentryOptions options, Scope? scope, SdkVersion? sdk = null), so the second positional argument must be a Scope?. There is no implicit conversion from SdkVersion to Scope, so this is a type mismatch. The other structured-logging integrations retrieve and pass the scope explicitly (Serilog: log.SetDefaultAttributes(options, scope, Sdk); Extensions.Logging: log.SetDefaultAttributes(_options, scope, _sdk)). Because the NLog path never obtains a Scope (no hub.GetScope() call), scope-derived attributes set by SetDefaultAttributes — user.id, user.name, user.email, and the PII/server.address fallback — cannot be populated on structured logs, and the SdkVersion is passed into the wrong parameter.
⏱ 5m 40s · 1.6M in / 50.6k out · $3.51
Annotations
Check warning on line 354 in src/Sentry.NLog/SentryTarget.cs
sentry-warden / warden: code-review
NLog CaptureStructuredLog passes SdkVersion where SetDefaultAttributes expects Scope
In `SentryTarget.Structured.cs`, `CaptureStructuredLog` calls `log.SetDefaultAttributes(options, Sdk)`, passing the static `SdkVersion Sdk` field as the second argument. However, `SentryLog.SetDefaultAttributes` has the signature `(SentryOptions options, Scope? scope, SdkVersion? sdk = null)`, so the second positional argument must be a `Scope?`. There is no implicit conversion from `SdkVersion` to `Scope`, so this is a type mismatch. The other structured-logging integrations retrieve and pass the scope explicitly (Serilog: `log.SetDefaultAttributes(options, scope, Sdk)`; Extensions.Logging: `log.SetDefaultAttributes(_options, scope, _sdk)`). Because the NLog path never obtains a `Scope` (no `hub.GetScope()` call), scope-derived attributes set by `SetDefaultAttributes` — `user.id`, `user.name`, `user.email`, and the PII/`server.address` fallback — cannot be populated on structured logs, and the SdkVersion is passed into the wrong parameter.