From 7144554598e1d75a1fce06ab9894e4f7397f4246 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ozan=20G=C3=B6ktan?= Date: Thu, 16 May 2024 12:22:08 +0200 Subject: [PATCH 1/4] add case-insensitive fallback for response header logging --- src/Logging.fs | 41 ++++++++++++++++++++++++++++++++++++----- test/Fetch.fs | 6 ++++-- version | 2 +- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/Logging.fs b/src/Logging.fs index 0a3e153..7e12032 100644 --- a/src/Logging.fs +++ b/src/Logging.fs @@ -24,12 +24,43 @@ module Logging = let private replacer (_: Match) : string = $"{{{PlaceHolder.ResponseHeader}__{Interlocked.Increment(&placeholderCounter)}}}" + let mutable private responseHeaders: Option>> = Option>>.None + + let private insertCaseInsensitive (headers: Map>) (key: string) (value: seq) :Map> = + let keyLower = key.ToLowerInvariant() + + let updated = + match headers.TryGetValue(keyLower) with + | true, v -> Seq.append v value + | false, _ -> value + + headers.Add(keyLower, updated) + + let private processHeadersMergeCase(headers: Map>) :Map> = + match responseHeaders with + | Some(x) -> x + | None -> + let mutable hdr: Map> = Map.empty + for key, value in headers |> Map.toList do + hdr <- insertCaseInsensitive hdr key value + + responseHeaders <- Some(hdr) + responseHeaders.Value + let private getHeaderValue (headers: Map>) (key: string) : string = - match headers.TryGetValue(key) with - | true, v -> - match Seq.tryHead v with - | first -> if first.IsSome then first.Value else String.Empty - | false, _ -> String.Empty + let readHeader (b: bool, v: seq) :Option = + match b with + | true -> + match Seq.tryHead v with + | first -> if first.IsSome then Some(first.Value) else None + | false -> None + + match headers.TryGetValue(key) |> readHeader with + | Some(x) -> x + | None -> + match processHeadersMergeCase(headers).TryGetValue(key.ToLowerInvariant()) |> readHeader with + | Some(x) -> x + | None -> String.Empty let private log' logLevel ctx content = match ctx.Request.Logger with diff --git a/test/Fetch.fs b/test/Fetch.fs index e1efcbe..513d400 100644 --- a/test/Fetch.fs +++ b/test/Fetch.fs @@ -132,6 +132,7 @@ let ``Get with logging is OK`` () = "Message" "ResponseHeader[missing-key]" "ResponseHeader[X-Request-ID]" + "ResponseHeader[x-request-id]" "ResponseHeader" "ResponseHeader[" "ResponseHeader]" @@ -163,7 +164,7 @@ let ``Get with logging is OK`` () = // Assert test <@ logger.Output.Contains "42" @> test <@ logger.Output.Contains "http://test.org" @> - test <@ logger.Output.Contains $"test-value\n← {msg}\n← \n← test-request-id\n← \n← \n← \n← \n← end" @> + test <@ logger.Output.Contains $"test-value\n← {msg}\n← \n← test-request-id\n← test-request-id\n← \n← \n← \n← \n← end" @> test <@ logger.Output.Contains "not-included-in-log" = false @> test <@ Result.isOk result @> test <@ metrics.Retries = 0L @> @@ -204,6 +205,7 @@ let ``Post with logging is OK`` () = "Message" "ResponseHeader[missing-key]" "ResponseHeader[X-Request-ID]" + "ResponseHeader[x-request-id]" "ResponseHeader" "ResponseHeader[" "ResponseHeader]" @@ -230,7 +232,7 @@ let ``Post with logging is OK`` () = test <@ logger.Output.Contains json @> test <@ logger.Output.Contains msg @> test <@ logger.Output.Contains "http://testing.org" @> - test <@ logger.Output.Contains $"test-value\n← {msg}\n← \n← test-request-id\n← \n← \n← \n← \n← end" @> + test <@ logger.Output.Contains $"test-value\n← {msg}\n← \n← test-request-id\n← test-request-id\n← \n← \n← \n← \n← end" @> test <@ logger.Output.Contains "not-included-in-log" = false @> test <@ Result.isOk result @> test <@ retries' = 1 @> diff --git a/version b/version index 6d54bbd..7a9f89d 100644 --- a/version +++ b/version @@ -1 +1 @@ -6.0.1 \ No newline at end of file +6.0.2 \ No newline at end of file From fea4307c04adf4b1690c59da979f880a4d5a911e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ozan=20G=C3=B6ktan?= Date: Thu, 16 May 2024 12:34:53 +0200 Subject: [PATCH 2/4] formatting --- src/Logging.fs | 27 ++++++++++++++++++--------- test/Fetch.fs | 16 ++++++++++++++-- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/Logging.fs b/src/Logging.fs index 7e12032..ac5715e 100644 --- a/src/Logging.fs +++ b/src/Logging.fs @@ -24,23 +24,29 @@ module Logging = let private replacer (_: Match) : string = $"{{{PlaceHolder.ResponseHeader}__{Interlocked.Increment(&placeholderCounter)}}}" - let mutable private responseHeaders: Option>> = Option>>.None - - let private insertCaseInsensitive (headers: Map>) (key: string) (value: seq) :Map> = + let mutable private responseHeaders: Option>> = + Option>>.None + + let private insertCaseInsensitive + (headers: Map>) + (key: string) + (value: seq) + : Map> = let keyLower = key.ToLowerInvariant() - let updated = + let updated = match headers.TryGetValue(keyLower) with | true, v -> Seq.append v value | false, _ -> value headers.Add(keyLower, updated) - let private processHeadersMergeCase(headers: Map>) :Map> = + let private processHeadersMergeCase (headers: Map>) : Map> = match responseHeaders with | Some(x) -> x | None -> let mutable hdr: Map> = Map.empty + for key, value in headers |> Map.toList do hdr <- insertCaseInsensitive hdr key value @@ -48,17 +54,20 @@ module Logging = responseHeaders.Value let private getHeaderValue (headers: Map>) (key: string) : string = - let readHeader (b: bool, v: seq) :Option = + let readHeader (b: bool, v: seq) : Option = match b with | true -> match Seq.tryHead v with | first -> if first.IsSome then Some(first.Value) else None - | false -> None - + | false -> None + match headers.TryGetValue(key) |> readHeader with | Some(x) -> x | None -> - match processHeadersMergeCase(headers).TryGetValue(key.ToLowerInvariant()) |> readHeader with + match + processHeadersMergeCase(headers).TryGetValue(key.ToLowerInvariant()) + |> readHeader + with | Some(x) -> x | None -> String.Empty diff --git a/test/Fetch.fs b/test/Fetch.fs index 513d400..973e0d1 100644 --- a/test/Fetch.fs +++ b/test/Fetch.fs @@ -164,7 +164,13 @@ let ``Get with logging is OK`` () = // Assert test <@ logger.Output.Contains "42" @> test <@ logger.Output.Contains "http://test.org" @> - test <@ logger.Output.Contains $"test-value\n← {msg}\n← \n← test-request-id\n← test-request-id\n← \n← \n← \n← \n← end" @> + + test + <@ + logger.Output.Contains + $"test-value\n← {msg}\n← \n← test-request-id\n← test-request-id\n← \n← \n← \n← \n← end" + @> + test <@ logger.Output.Contains "not-included-in-log" = false @> test <@ Result.isOk result @> test <@ metrics.Retries = 0L @> @@ -232,7 +238,13 @@ let ``Post with logging is OK`` () = test <@ logger.Output.Contains json @> test <@ logger.Output.Contains msg @> test <@ logger.Output.Contains "http://testing.org" @> - test <@ logger.Output.Contains $"test-value\n← {msg}\n← \n← test-request-id\n← test-request-id\n← \n← \n← \n← \n← end" @> + + test + <@ + logger.Output.Contains + $"test-value\n← {msg}\n← \n← test-request-id\n← test-request-id\n← \n← \n← \n← \n← end" + @> + test <@ logger.Output.Contains "not-included-in-log" = false @> test <@ Result.isOk result @> test <@ retries' = 1 @> From 9fc2ba1e5cd0d6f23177387df8c0e2579cbca13d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ozan=20G=C3=B6ktan?= <72358629+ozangoktan@users.noreply.github.com> Date: Thu, 16 May 2024 12:59:50 +0200 Subject: [PATCH 3/4] Update src/Logging.fs Co-authored-by: Dag Brattli --- src/Logging.fs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Logging.fs b/src/Logging.fs index ac5715e..5c22229 100644 --- a/src/Logging.fs +++ b/src/Logging.fs @@ -56,9 +56,7 @@ module Logging = let private getHeaderValue (headers: Map>) (key: string) : string = let readHeader (b: bool, v: seq) : Option = match b with - | true -> - match Seq.tryHead v with - | first -> if first.IsSome then Some(first.Value) else None + | true -> Seq.tryHead v | false -> None match headers.TryGetValue(key) |> readHeader with From 565fe630c82522e0b3a82681529eb38022b460f7 Mon Sep 17 00:00:00 2001 From: Dag Brattli Date: Thu, 16 May 2024 14:46:41 +0200 Subject: [PATCH 4/4] Simplify header logging --- src/Logging.fs | 54 +++++++++----------------------------------------- 1 file changed, 9 insertions(+), 45 deletions(-) diff --git a/src/Logging.fs b/src/Logging.fs index ac5715e..8cda504 100644 --- a/src/Logging.fs +++ b/src/Logging.fs @@ -24,52 +24,15 @@ module Logging = let private replacer (_: Match) : string = $"{{{PlaceHolder.ResponseHeader}__{Interlocked.Increment(&placeholderCounter)}}}" - let mutable private responseHeaders: Option>> = - Option>>.None - - let private insertCaseInsensitive - (headers: Map>) - (key: string) - (value: seq) - : Map> = - let keyLower = key.ToLowerInvariant() - - let updated = - match headers.TryGetValue(keyLower) with - | true, v -> Seq.append v value - | false, _ -> value - - headers.Add(keyLower, updated) - - let private processHeadersMergeCase (headers: Map>) : Map> = - match responseHeaders with - | Some(x) -> x - | None -> - let mutable hdr: Map> = Map.empty - - for key, value in headers |> Map.toList do - hdr <- insertCaseInsensitive hdr key value - - responseHeaders <- Some(hdr) - responseHeaders.Value + let private lowerCaseHeaders (headers: Map>) : Map> = + headers |> Seq.map (fun kv -> kv.Key.ToLowerInvariant(), kv.Value) |> Map.ofSeq let private getHeaderValue (headers: Map>) (key: string) : string = - let readHeader (b: bool, v: seq) : Option = - match b with - | true -> - match Seq.tryHead v with - | first -> if first.IsSome then Some(first.Value) else None - | false -> None - - match headers.TryGetValue(key) |> readHeader with - | Some(x) -> x - | None -> - match - processHeadersMergeCase(headers).TryGetValue(key.ToLowerInvariant()) - |> readHeader - with - | Some(x) -> x - | None -> String.Empty + headers + |> Map.tryFind key + |> Option.defaultValue Seq.empty + |> Seq.tryHead + |> Option.defaultValue String.Empty let private log' logLevel ctx content = match ctx.Request.Logger with @@ -77,6 +40,7 @@ module Logging = let format = ctx.Request.LogFormat let request = ctx.Request let matches = reqex.Matches format + let lowerCaseHeaders = lazy (lowerCaseHeaders ctx.Response.Headers) // Create an array with values in the same order as in the format string. Important to be lazy and not // stringify any values here. Only pass references to the objects themselves so the logger can stringify @@ -96,7 +60,7 @@ module Logging = | PlaceHolder.ResponseHeader -> // GroupCollection returns empty string values for indexes beyond what was captured, therefore // we don't cause an exception here if the optional second group was not captured - getHeaderValue ctx.Response.Headers match'.Groups[3].Value :> _ + getHeaderValue lowerCaseHeaders.Value (match'.Groups[3].Value.ToLowerInvariant()) :> _ | key -> // Look for the key in the extra info. This also enables custom HTTP handlers to add custom // placeholders to the format string.