Decode escaped counter tags at output boundaries - #5964
Open
hoyosjs wants to merge 2 commits into
Open
Conversation
MetricsEventSource escapes counter tag keys/values in newer event
versions ('\'->'\\', ','->'\,', '='->'\='). Add CounterTagFormatter to
mirror the runtime encoder: Normalize re-escapes legacy payloads at
ingestion, Decode unescapes at each output boundary.
- Fix ConsoleWriter.RenderTagSetsInColumnMode IndexOutOfRange on tags
containing '=' or ','.
- JSON/CSV/Console exporters decode tags to their real key/value pairs.
- CSVExporter is now RFC 4180 compliant: every field is quoted when it
contains ',', '"', CR or LF, so a comma in a tag value, provider name
or display name no longer spills into the next column.
- JSONExporter escapes all control chars below U+0020 as \u00XX so a
control char in a tag value cannot produce invalid JSON.
- Decode has a no-backslash fast path that slices key/values from the
source string, avoiding StringBuilders on the common (unescaped) case.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52f40a78-0e0a-49cd-a98d-5970e0935969
Contributor
There was a problem hiding this comment.
Pull request overview
Version-gates the MetricsEventSource tag-escaping transport format at ingestion, keeps tags canonical internally, and decodes them at dotnet-counters output boundaries so tags containing \, ,, and = render correctly (and no longer crash/misparse in column mode).
Changes:
- Normalize/escape meter tag payloads at ingestion based on TraceEvent version, and add a shared
CounterTagFormatterfor normalize/decode. - Decode tags for console column mode, CSV, and JSON exporters; add RFC 4180 CSV quoting and stricter JSON string escaping for control characters.
- Add focused unit tests covering decoding behavior and ensuring EventCounters metadata remains verbatim.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Tools/dotnet-counters/Exporters/JSONExporter.cs | Decodes canonical meter tags at output and escapes previously-invalid JSON control characters. |
| src/Tools/dotnet-counters/Exporters/CSVExporter.cs | Decodes tags before emitting and RFC 4180-quotes fields so commas/quotes/CR/LF can’t corrupt CSV rows. |
| src/Tools/dotnet-counters/Exporters/ConsoleWriter.cs | Fixes column-mode tag rendering by decoding escaped meter tags (and parsing EventCounters metadata appropriately). |
| src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs | Normalizes tag transport format at ingestion using event-version gating. |
| src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterTagFormatter.cs | Adds canonical tag normalize/encode and robust decode logic for escaped meter tags. |
| src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterTriggerTests.cs | Verifies EventCounters metadata parsing preserves backslashes/equals verbatim. |
| src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/CounterTagFormatterTests.cs | New unit tests for decode/normalize behaviors and edge cases. |
| src/tests/dotnet-counters/JSONExporterTests.cs | Ensures JSON exporter decodes tags and escapes control characters while leaving EventCounters metadata unchanged. |
| src/tests/dotnet-counters/CSVExporterTests.cs | Ensures decoded commas don’t create extra columns and validates CSV quoting behavior. |
| src/tests/dotnet-counters/ConsoleExporterTests.cs | Ensures console column mode decodes escaped tags and preserves EventCounters metadata verbatim. |
| AGENTS.md | Documents comment-style guidance (ASCII-only, avoid historical narration, explain “why”). |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
noahfalk
reviewed
Aug 13, 2026
| // With no backslash there are no escape sequences, so every '=' and ',' is a separator and | ||
| // each key/value is a verbatim substring of the input. This is the common case (most tags | ||
| // contain no special characters), and slicing it avoids the two StringBuilders and the | ||
| // per-character copy the escaped path needs. |
Member
There was a problem hiding this comment.
I'd optimize for code simplicity here, not minimal allocations. These tools aren't designed for the volume of metrics where a few extra allocations per metric would have measurable impact. I'd suggest don't dual version the decode, just write it once including the escape sequence handling.
| // Counter tags arrive from MetricsEventSource as a single string. Newer event versions escape | ||
| // each key and value so a ',' or '=' inside a key/value is distinguishable from the ',' that | ||
| // separates pairs and the '=' that separates a key from its value: | ||
| // '\' -> '\\' ',' -> '\,' '=' -> '\=' |
Member
There was a problem hiding this comment.
I suggested an alternative encoding scheme in the runtime PR so if use that we'd need to update the decoder to match.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5935.
Newer
MetricsEventSourceevent versions escape\,,, and=inside meter tag keys and values. This change version-gates that transport format at ingestion, keeps it canonical internally, and decodes it at dotnet-counters console, CSV, and JSON output boundaries. It fixes column-mode crashes/misparsing, preserves literal delimiters, keeps legacy EventCounters metadata verbatim, makes CSV fields RFC 4180 compliant, and prevents raw JSON control characters.Older tools remain compatible with ordinary tags, but do not understand escaped delimiters from newer runtimes. Tags containing commas may be split or crash older dotnet-counters output, equals may be truncated or misparsed, and backslashes may appear doubled.
CSV fields containing commas, quotes, CR, or LF are now quoted. This is an intentional output formatting change, not a schema change.
Known pre-existing limitation: bare
NaN,Infinity, and-Infinitymetric values remain invalid under strict JSON parsing. Choosingnullversus quoted strings requires a separate schema decision.Validation:
CounterTagFormatterTests: 17 passed.EventCounterTriggerTests: 10 passed, 1 existing pipeline test skipped.CSVExporterTests: 15 passed.JSONExporterTests: 13 passed.ConsoleExporterTests: 26 passed.AI-generated-content disclosure: This change was developed with GitHub Copilot. The implementation and tests were reviewed and validated by the contributor.