GelfLayout - Align with SplunkLayout #5927
Conversation
WalkthroughThe changes update the naming convention of default GELF fields in the layout initialization to use lowercase and underscores, and adjust related test expectations accordingly. No public APIs or exported entity signatures are modified; all changes are internal to layout initialization logic and test string templates. Changes
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/NLog.Targets.Network.Tests/GelfLayoutTests.cs (1)
85-85: Test correctly updated but consider expanding coverage for other default fields.The test correctly reflects the field name change from "_logLevel" to "_loglevel". However, the test doesn't verify the other renamed default fields ("_exception_type", "_exception_msg", "_exception") that were also changed in the implementation.
Consider adding test coverage for exception-related fields to ensure they render correctly with the new naming convention:
[Fact] public void CanRenderGelfWithException() { var gelfLayout = new GelfLayout(); var logEvent = new LogEventInfo { LoggerName = "TestLogger", Level = LogLevel.Error, Message = "Error occurred", Exception = new InvalidOperationException("Test exception"), TimeStamp = DateTime.Now, }; var result = gelfLayout.Render(logEvent); // Verify new exception field names are present Assert.Contains("\"_exception_type\":", result); Assert.Contains("\"_exception_msg\":", result); Assert.Contains("\"_exception\":", result); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/NLog.Targets.Network/Layouts/GelfLayout.cs(1 hunks)tests/NLog.Targets.Network.Tests/GelfLayoutTests.cs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/NLog.Targets.Network/Layouts/GelfLayout.cs (1)
src/NLog/Targets/TargetPropertyWithContext.cs (2)
TargetPropertyWithContext(51-51)TargetPropertyWithContext(58-62)
| if (GelfFields.Count == 0) | ||
| { | ||
| GelfFields.Add(new TargetPropertyWithContext("_logLevel", "${level}")); | ||
| GelfFields.Add(new TargetPropertyWithContext("_loglevel", "${level}")); | ||
| GelfFields.Add(new TargetPropertyWithContext("_logger", "${logger}")); | ||
| GelfFields.Add(new TargetPropertyWithContext("_exceptionType", "${exception:Format=Type}") { IncludeEmptyValue = false }); | ||
| GelfFields.Add(new TargetPropertyWithContext("_exceptionMessage", "${exception:Format=Message}") { IncludeEmptyValue = false }); | ||
| GelfFields.Add(new TargetPropertyWithContext("_stackTrace", "${exception:Format=ToString}") { IncludeEmptyValue = false }); | ||
| GelfFields.Add(new TargetPropertyWithContext("_exception_type", "${exception:Format=Type}")); | ||
| GelfFields.Add(new TargetPropertyWithContext("_exception_msg", "${exception:Format=Message}")); | ||
| GelfFields.Add(new TargetPropertyWithContext("_exception", "${exception:Format=ToString}")); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Breaking change: Verify backward compatibility impact and alignment with standards.
The field name changes from camelCase to lowercase with underscores represent a breaking change for existing GELF consumers. Additionally, line 180 changes both the field name (from "_stackTrace" to "_exception") and the format (from individual exception components to ToString format).
Ensure that:
- This aligns with the intended GELF standard or SplunkLayout conventions
- Existing deployments can handle this change gracefully
- Consider providing a migration guide for users
🏁 Script executed:
#!/bin/bash
# Search for any documentation about GELF field naming standards
rg -i "gelf.*field.*standard|field.*naming.*convention" --type md
rg -i "splunk.*layout.*field" --type csLength of output: 109
🏁 Script executed:
#!/bin/bash
# Check for any remaining references to the old field names in code/tests
rg -n '"_logLevel"' -t cs
rg -n '"_exceptionType"' -t cs
rg -n '"_exceptionMessage"' -t cs
rg -n '"_stackTrace"' -t cs
# Inspect XML doc and surrounding code for default field description
sed -n '150,230p' src/NLog.Targets.Network/Layouts/GelfLayout.csLength of output: 3848
Breaking Change: Default GELF Field Names and Exception Format Updated
The InitializeLayout defaults now use lowercase-underscore field names and switch from a separate _stackTrace field to a combined _exception (ToString) value. This will break existing GELF consumers relying on the old camelCase names and individual exception components.
Please address the following before merging:
• Confirm these names and formats align with your intended GELF conventions (or SplunkLayout standards).
• Update any user documentation or examples to reflect the new defaults.
• Provide a clear migration guide or configuration option for users who need the previous field names (_logLevel, _exceptionType, _exceptionMessage, _stackTrace) and formats.
Affected location:
- src/NLog.Targets.Network/Layouts/GelfLayout.cs lines 174–181
🤖 Prompt for AI Agents
In src/NLog.Targets.Network/Layouts/GelfLayout.cs lines 174 to 181, the default
GELF field names have changed to lowercase with underscores and the exception
fields have been consolidated into a single _exception field using ToString
format. Review and confirm these new field names and formats match your GELF or
SplunkLayout conventions. Update all related user documentation and examples to
reflect these changes. Additionally, implement a migration guide or provide a
configuration option that allows users to revert to the previous camelCase field
names and separate exception fields (_logLevel, _exceptionType,
_exceptionMessage, _stackTrace) if needed.
|
No description provided.