-
Notifications
You must be signed in to change notification settings - Fork 1.4k
ValueFormatter - Skip quotes for CaptureType.Stringify unless legacy mode #6013
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
Conversation
WalkthroughAdjusts Stringify capture handling across template formatting, JSON layout, and value formatting to consistently quote stringified values at appropriate layers, refine null handling, and prevent duplicate quotes. Updates unit tests to reflect new quoting behavior and add coverage for Stringify across types. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Application
participant Logger as Logger
participant Formatter as LogMessageTemplateFormatter
participant VF as ValueFormatter
App->>Logger: Log(message, args/properties)
Logger->>Formatter: Format template holes
alt Hole CaptureType = Stringify
Formatter->>Formatter: RenderHolePositional(...)
Formatter->>Formatter: RenderHole(value, Stringify)
note right of Formatter: Adds opening quote
Formatter->>VF: FormatValue(value, Stringify)
VF-->>Formatter: Stringified content
note right of Formatter: Adds closing quote
else Other CaptureType
Formatter->>VF: FormatValue(value, captureType)
VF-->>Formatter: Serialized content or "NULL"
end
Formatter-->>Logger: Formatted message
Logger-->>App: Output
sequenceDiagram
autonumber
participant App as Application
participant Json as JsonLayout
participant VF as ValueFormatter
App->>Json: Render(LogEvent with properties)
alt Property placeholder = {$name} (Stringify)
Json->>Json: Append '"'
Json->>VF: FormatValue(property, Stringify)
VF-->>Json: Stringified content
Json->>Json: Append '"'
Json->>Json: PerformJsonEscapeIfNeeded(startPos)
else Other properties
Json->>VF: FormatValue(property, captureType)
VF-->>Json: Content
end
Json-->>App: JSON string
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|



Followup to #5955 (Instead of magic-value logic around Format-parameter, then change the official contract)
CaptureType.Stringifyis kind of exotic, so not expecting much friction from the change (and legacy mode is available).