chore: Log New Relic collector response headers at debug level.#3655
chore: Log New Relic collector response headers at debug level.#3655jaffinito wants to merge 2 commits into
Conversation
| // Possibly combine these logs? makes parsing harder in tests... | ||
| Log.Debug("Request({0}): Invoked \"{1}\" with : {2}", requestGuid, method, serializedData); | ||
| Log.Debug("Request({0}): Invocation of \"{1}\" yielded response : {2}", requestGuid, method, responseContent); | ||
| if (Log.IsDebugEnabled) |
There was a problem hiding this comment.
Why not also put the two Log.Debug statements above this line inside the if block?
There was a problem hiding this comment.
I could have, but they don't need to be. My header line calls response.GetHeaders() — that executes regardless of log level (iterate headers, string.Join, allocate). Serilog/NoOp can drop the message, but it can't un-run the method that built the argument. The guard skips that work when Debug is off (the production default).
The existing lines pass serializedData / responseContent — strings that already exist. When Debug is off, the logger drops the message and those strings cost nothing extra.
|
Are there likely to be any headers carrying sensitive data that we should avoid logging or obfuscate if we do log? |
|
In the request headers, definitely. In the response headers, I would not expect there to be sensitive data. I will do a quick google survey to see if anything comes up. |
|
I added some basic protection for headers that might have data we don't want in logs. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3655 +/- ##
=======================================
Coverage 81.99% 82.00%
=======================================
Files 511 512 +1
Lines 34724 34753 +29
Branches 4134 4140 +6
=======================================
+ Hits 28473 28500 +27
+ Misses 5276 5275 -1
- Partials 975 978 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
4 participants
Description
What
Log the collector's HTTP response headers to the agent debug log, on both the success and error paths.
Why (NR-132421)
Only the response body was logged. Headers (cloudflare, request id, geo markers) are needed by support to diagnose 503s and Cloudflare failures.
How
IHttpResponse.GetHeaders(); implemented onHttpResponse(.NET Core) andWebRequestClientResponse(.NET Framework).HeadersonIHttpResponseMessageWrapperfor the Core mock seam.HttpResponseHeaderFormatter(pure helper) withIEnumerable<KeyValuePair<...>>and#if NETFRAMEWORK WebHeaderCollectionoverloads. Output:cf-ray=[abc-DFW]; x-request-id=[id-1], or(none).HttpCollectorWire.SendDatalogs headers at debug on success and error paths, guarded byLog.IsDebugEnabled(GetHeaders builds its string eagerly).Tests
HttpResponseHeaderFormatterTests,HttpResponseTests,WebRequestClientResponseTests,HttpCollectorWireTests(guard verified both ways).net10.0andnet481.Author Checklist
Reviewer Checklist
Fixes #112