Skip to content

Commit bcb9a84

Browse files
committed
Centralize relaxed JSON writer; add Format-path and label escaping tests
The relaxed-encoder JsonWriterOptions was defined twice (formatter and serializer) with drifting comments, and each Utf8JsonWriter opted in per construction site: a future writer that forgot the options argument would silently revert to HTML-safe \uXXXX escaping with no compiler warning. Extract a single Infrastructure.JsonWriterDefaults.createWriter as the one way the sink builds a writer, so escaping cannot drift between sites. The shared comment also corrects the description: the relaxed encoder still escapes U+2028/U+2029 and DEL, so it is not "only what JSON mandates". Add the two missing wire-format guards: - the public LokiJsonTextFormatter.Format entry point (the sink fast path bypasses it, so its writer site had no test coverage); - promoted label values (the envelope writer escapes label keys/values too). The escaping tests sample neutral, multi-script non-ASCII (accented Latin plus CJK) rather than a single script.
1 parent 39f3274 commit bcb9a84

5 files changed

Lines changed: 78 additions & 28 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright 2020-2026 Mykhailo Shevchuk & Contributors
2+
//
3+
// Licensed under the MIT license;
4+
// you may not use this file except in compliance with the License.
5+
//
6+
// Unless required by applicable law or agreed to in writing, software
7+
// distributed under the License is distributed on an "AS IS" BASIS,
8+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
9+
// See LICENSE file in the project root for full license information.
10+
11+
namespace Serilog.Sinks.Grafana.Loki.Infrastructure
12+
13+
open System.Buffers
14+
open System.Text.Encodings.Web
15+
open System.Text.Json
16+
17+
// Single source of truth for every Utf8JsonWriter the sink creates.
18+
//
19+
// The default Utf8JsonWriter encoder is HTML-safe and renders every '"', '<', '>', '&', '\'' and
20+
// all non-ASCII as \uXXXX, which makes the stored Loki log line unreadable. The relaxed encoder
21+
// escapes the JSON-mandatory set (quote, backslash, control chars) plus the line/paragraph
22+
// separators U+2028/U+2029 and DEL, and emits printable non-ASCII verbatim. Output stays valid
23+
// JSON (and valid UTF-8), so consumers are unaffected.
24+
//
25+
// Centralised behind createWriter so escaping cannot drift between writer sites: a writer built
26+
// the default way (`new Utf8JsonWriter(buffer)`) would silently re-introduce \uXXXX over part of
27+
// the payload, and no compiler warning catches the missing options argument.
28+
[<RequireQualifiedAccess>]
29+
module internal JsonWriterDefaults =
30+
31+
let relaxedOptions =
32+
JsonWriterOptions(Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping)
33+
34+
/// Creates a Utf8JsonWriter over the buffer using the relaxed (readable) encoder.
35+
let createWriter (buffer: PooledByteBufferWriter) =
36+
new Utf8JsonWriter(buffer :> IBufferWriter<byte>, relaxedOptions)

src/Serilog.Sinks.Grafana.Loki/LokiJsonTextFormatter.fs

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111
namespace Serilog.Sinks.Grafana.Loki
1212

1313
open System
14-
open System.Buffers
15-
open System.Text.Encodings.Web
1614
open System.Text.Json
1715
open Serilog.Events
1816
open Serilog.Formatting
@@ -31,12 +29,6 @@ type LokiJsonTextFormatter(exceptionFormatter: ILokiExceptionFormatter, enrichTr
3129
static let pTraceId = JsonEncodedText.Encode "TraceId"
3230
static let pSpanId = JsonEncodedText.Encode "SpanId"
3331

34-
// Relaxed escaping keeps the body readable: only JSON-mandatory escapes, non-ASCII verbatim.
35-
// The default Utf8JsonWriter encoder is HTML-safe and would \uXXXX-escape every '"', '<', '>',
36-
// '&', '\'' and all non-ASCII. Output stays valid JSON, so consumers are unaffected.
37-
static let relaxedWriterOptions =
38-
JsonWriterOptions(Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping)
39-
4032
// Names that collide with top-level JSON keys; prefixed with '_' when seen as properties.
4133
static let reserved =
4234
Collections.Generic.HashSet<string>(
@@ -180,9 +172,7 @@ type LokiJsonTextFormatter(exceptionFormatter: ILokiExceptionFormatter, enrichTr
180172
use buffer = new PooledByteBufferWriter(256)
181173
// Throwaway writer + scratch for the message render. The internal sink path passes reused
182174
// instances instead; the public path stays allocation-light but fully thread-safe.
183-
use jsonWriter =
184-
new Utf8JsonWriter(buffer :> IBufferWriter<byte>, relaxedWriterOptions)
185-
175+
use jsonWriter = JsonWriterDefaults.createWriter buffer
186176
use messageBuffer = new PooledByteBufferWriter(128)
187177
use messageWriter = new Utf8TextWriter(messageBuffer)
188178
self.FormatToBuffer(logEvent, jsonWriter, messageWriter)

src/Serilog.Sinks.Grafana.Loki/Serialization.fs

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,12 @@ open System
1616
open System.Buffers
1717
open System.Buffers.Text
1818
open System.Collections.Generic
19-
open System.Text.Encodings.Web
2019
open System.Text.Json
2120
open Microsoft.FSharp.NativeInterop
2221
open Serilog.Events
2322
open Serilog.Formatting
2423
open Serilog.Sinks.Grafana.Loki.Infrastructure
2524

26-
/// Relaxed JSON escaping for the bytes the sink emits. The default Utf8JsonWriter encoder is
27-
/// HTML-safe and renders every '"', '<', '>', '&', '\'' and all non-ASCII as \uXXXX, which makes
28-
/// the stored log line unreadable; the relaxed encoder escapes only what JSON mandates and emits
29-
/// non-ASCII verbatim. Output stays valid JSON (and valid UTF-8), so consumers are unaffected.
30-
[<AutoOpen>]
31-
module private JsonWriterDefaults =
32-
let relaxedWriterOptions =
33-
JsonWriterOptions(Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping)
34-
3525
/// Reusable per-sink serialization scratch: the buffers and writers reused across every batch
3626
/// (cleared/reset between uses), bundled so the serializer takes one value instead of several
3727
/// same-typed positional args (easy to mis-order), and disposed as a unit by the sink.
@@ -41,9 +31,7 @@ type internal SerializationBuffers() =
4131
let body = new PooledByteBufferWriter(256)
4232
let message = new PooledByteBufferWriter(256)
4333

44-
let bodyWriter =
45-
new Utf8JsonWriter(body :> IBufferWriter<byte>, relaxedWriterOptions)
46-
34+
let bodyWriter = JsonWriterDefaults.createWriter body
4735
let messageWriter = new Utf8TextWriter(message)
4836

4937
/// Envelope buffer holding the full push payload; read by LokiPushContent after serialize.
@@ -100,8 +88,7 @@ module internal Serialization =
10088
| :? LokiJsonTextFormatter as fmt when fmt.GetType() = typeof<LokiJsonTextFormatter> -> fmt
10189
| _ -> Unchecked.defaultof<LokiJsonTextFormatter>
10290

103-
use jsonWriter =
104-
new Utf8JsonWriter(buffers.Main :> IBufferWriter<byte>, relaxedWriterOptions)
91+
use jsonWriter = JsonWriterDefaults.createWriter buffers.Main
10592

10693
// Reused stack scratch for the per-event Unix-nanosecond timestamp. Hoisted out of
10794
// the event loop so the localloc happens once per batch, not once per event. An

src/Serilog.Sinks.Grafana.Loki/Serilog.Sinks.Grafana.Loki.fsproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
<!-- Infrastructure (no Serilog dependency) -->
4949
<Compile Include="Infrastructure\PooledByteBufferWriter.fs"/>
5050
<Compile Include="Infrastructure\Utf8TextWriter.fs"/>
51+
<Compile Include="Infrastructure\JsonWriterDefaults.fs"/>
5152

5253
<!-- Public contract types -->
5354
<Compile Include="LokiLabel.fs"/>

tests/Serilog.Sinks.Grafana.Loki.UnitTests/WireFormatTests.fs

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,17 +257,53 @@ let ``body: quotes, markup and non-ASCII are not unicode-escaped`` () : Task =
257257
task {
258258
let handler, sink = makeSink id
259259
use _ = sink
260-
do! flush sink [ mkInfo [ "Note", box "a>b <c> & \"q\" Привет" ] ]
260+
do! flush sink [ mkInfo [ "Note", box "a>b <c> & \"q\" café 日本語" ] ]
261261
use doc = handler.LastBodyJson
262262
let body = bodyStringOf (streamAt 0 doc) 0
263263
// Relaxed escaping: '<' '>' '&' and non-ASCII stay verbatim and quotes use the JSON-standard
264264
// \", never \uXXXX. The default HTML-safe encoder would render all of these as \uXXXX.
265265
test <@ body.Contains("a>b <c> &") @>
266-
test <@ body.Contains("Привет") @>
266+
test <@ body.Contains("café 日本語") @>
267267
test <@ body.Contains("\\\"q\\\"") @>
268268
test <@ not (body.Contains("\\u0022")) @>
269269
}
270270

271+
[<Fact>]
272+
let ``body: public Format path keeps quotes, markup and non-ASCII readable`` () =
273+
// The sink fast path calls FormatToBuffer directly; this is the only coverage of the public
274+
// ITextFormatter.Format entry point, whose Utf8JsonWriter is a separate construction site.
275+
let formatter = LokiJsonTextFormatter() :> ITextFormatter
276+
let event = mkInfo [ "Note", box "a>b <c> & \"q\" café 日本語" ]
277+
use output = new System.IO.StringWriter()
278+
formatter.Format(event, output)
279+
let body = output.ToString()
280+
use _ = JsonDocument.Parse(body) // output must stay valid JSON
281+
test <@ body.Contains("a>b <c> &") @>
282+
test <@ body.Contains("café 日本語") @>
283+
test <@ body.Contains("\\\"q\\\"") @>
284+
test <@ not (body.Contains("\\u")) @>
285+
286+
[<Fact>]
287+
let ``labels: promoted label value keeps markup and non-ASCII verbatim`` () : Task =
288+
task {
289+
let handler, sink =
290+
makeSink (fun o ->
291+
{ o with
292+
PropertiesAsLabels = [| "app" |]
293+
})
294+
295+
use _ = sink
296+
do! flush sink [ mkInfo [ "app", box "<svc> café 日本語" ] ]
297+
use doc = handler.LastBodyJson
298+
let raw = handler.LastBodyText
299+
// The envelope writer escapes label keys/values too. The round-trip proves the label
300+
// decodes back to the original; the raw scan proves it ships verbatim, not \uXXXX (a
301+
// label-only regression would still decode correctly but leak escaped < or non-ASCII bytes).
302+
test <@ labelOf "app" (streamAt 0 doc) = Some "<svc> café 日本語" @>
303+
test <@ raw.Contains("<svc> café 日本語") @>
304+
test <@ not (raw.Contains("\\u")) @>
305+
}
306+
271307
[<Fact>]
272308
let ``body: reserved property "Message" sanitized to "_Message"`` () : Task =
273309
task {

0 commit comments

Comments
 (0)