Exposed public setters for LogRecord.State, LogRecord.StateValues, and LogRecord.FormattedMessage. #3217
Conversation
| internal class MyClassWithRedactionEnumerator : IReadOnlyList<KeyValuePair<string, object>> | ||
| { | ||
| private static readonly Regex Rgx = new("\\(?\\d{3}[\\.|\\)]?\\d{3}[\\.|\\-]\\d{4}"); | ||
| private readonly IReadOnlyList<KeyValuePair<string, object>> myList; |
There was a problem hiding this comment.
| private readonly IReadOnlyList<KeyValuePair<string, object>> myList; | |
| private readonly IReadOnlyList<KeyValuePair<string, object>> traits; |
There was a problem hiding this comment.
I've updated name of the private field to be state.
Not sure which one is better, trait or state; but both of them definitely are better than myList 🤓.
| return this.GetEnumerator(); | ||
| } | ||
|
|
||
| public override string ToString() |
There was a problem hiding this comment.
We'll need this for the compiler:
IEnumerator IEnumerable.GetEnumerator()
{
return this.GetEnumerator();
}
There was a problem hiding this comment.
The comment is about ToString() override. Enumeration is needed.
There was a problem hiding this comment.
Interesting - my browser is showing me code highlight as this:
|
|
||
| internal class MyClassWithRedactionEnumerator : IReadOnlyList<KeyValuePair<string, object>> | ||
| { | ||
| private static readonly Regex Rgx = new("\\(?\\d{3}[\\.|\\)]?\\d{3}[\\.|\\-]\\d{4}"); |
There was a problem hiding this comment.
for this tutorial, i dont think we need to show actual regex! its upto end user to make their own.. we can do simple match like value.Contains("secret"), as it is sufficient to demonstrate the approach.
There was a problem hiding this comment.
Or if the class is renamed to "PhoneNumberRedactionSomething" it sounds reasonable.
Yun-Ting
changed the title
Codecov Report
@@ Coverage Diff @@
## main #3217 +/- ##
==========================================
+ Coverage 85.47% 85.70% +0.22%
==========================================
Files 260 260
Lines 9366 9366
==========================================
+ Hits 8006 8027 +21
+ Misses 1360 1339 -21
|
| { | ||
| public override void OnEnd(LogRecord logRecord) | ||
| { | ||
| if (logRecord.State is IReadOnlyList<KeyValuePair<string, object>> listOfKvp) |
There was a problem hiding this comment.
just for completion, we could show how to use same approach for logRecord.StateValues as well. (not a blocker for this PR)
| @@ -64,6 +65,9 @@ public static void Main() | |||
| { | |||
| logger.LogError("{name} is broken.", "refrigerator"); | |||
| } | |||
|
|
|||
| // message will be redacted by MyRedactionProcessor | |||
| logger.LogInformation("OpenTelemetry {sensitiveString}.", "<secret>"); | |||
There was a problem hiding this comment.
also it'd be good to modify the readme.md for the extending-the-sdk to include the redaction part. Similar to this https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/trace/extending-the-sdk#filtering-processor
|
Merging. Couple of non-blocking doc improvements suggested. |
Fixes #2877.
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes