fix: escape periods in attribute keys during flatten/unflatten#3975
fix: escape periods in attribute keys during flatten/unflatten#3975deepshekhardas wants to merge 1 commit into
Conversation
Object keys containing periods (e.g. 'Key 0.002mm') were incorrectly split on the '.' delimiter during flatten/unflatten, causing data corruption in the dashboard logs view. Escape dots with \$@dot sentinel during flattening and unescape during unflattening. Fixes triggerdotdev#1510
|
|
Hi @deepshekhardas, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚩 Map keys with dots are not escaped, inconsistent with object key handling
The Map handling code at packages/core/src/v3/utils/flattenAttributes.ts:120-121 constructs the flattened key without escaping dots:
const keyStr = typeof key === "string" ? key : String(key);
this.#processValue(value, `${prefix || "map"}.${keyStr}`, depth);
Meanwhile, object keys ARE escaped at line 204: key.replace(/\./g, DOT_ESCAPE). This means a Map with a string key like "a.b" will still have the dot treated as a hierarchy separator, producing an incorrect unflattened result. This is not a regression (Map keys were never escaped before this PR), but it is an inconsistency — the PR fixes the problem for plain objects but leaves Map objects with the same original issue. If the intent is to fully support dots in keys, Map keys should receive the same treatment.
(Refers to lines 120-121)
Was this helpful? React with 👍 or 👎 to provide feedback.
| const parts = key.split(".").reduce( | ||
| (acc, part) => { | ||
| if (part.startsWith("[") && part.endsWith("]")) { | ||
| const unescaped = part.split(DOT_ESCAPE).join("."); |
There was a problem hiding this comment.
🟡 Escape sentinel $@dot is not itself escaped, corrupting keys that naturally contain $@dot
The escaping mechanism replaces . with $@dot during flatten (line 204) but doesn't escape pre-existing $@dot occurrences. During unflatten, part.split(DOT_ESCAPE).join(".") (line 285) unconditionally converts ALL $@dot substrings back to ., including ones that were present in the original key.
For example, flattenAttributes({ "x$@doty": 1 }) produces { "x$@doty": 1 } (no dots to replace), but unflattenAttributes({ "x$@doty": 1 }) produces { "x.y": 1 } — silently corrupting the key. This means unflattenAttributes(flattenAttributes(obj)) is not a faithful round-trip for objects whose keys contain the sentinel substring.
Minimal reproduction and suggested fix approach
A proper escape scheme needs to escape the escape character first, e.g.:
- Flatten: first replace
$@dot→$@dot$@dot(escape the escape), then replace.→$@dot - Unflatten: first replace
$@dotback to., then replace$@dot$@dot→$@dot(unescape the escape)
Alternatively, choose a longer/more distinctive sentinel like $@__dot__@$ to minimize collision risk.
Prompt for agents
The DOT_ESCAPE sentinel ($@dot) is used as a substring replacement in both flattenAttributes and unflattenAttributes, but neither direction escapes pre-existing occurrences of the sentinel string itself. This means keys containing the literal string $@dot will be corrupted during round-tripping.
In flattenAttributes.ts line 204, only . is replaced with $@dot, but existing $@dot substrings are left as-is. In unflattenAttributes line 285, split(DOT_ESCAPE).join(.) converts ALL $@dot substrings including ones that were originally present in the key.
To fix: implement proper bi-directional escaping. For example:
- In doFlatten (around line 204): First escape literal $@dot in the key (e.g. to $$@dot), then replace . with $@dot
- In unflattenAttributes (around line 285): First replace $@dot with ., then unescape $$@dot back to $@dot
Alternatively, use a more distinctive and longer sentinel that is extremely unlikely to appear in real data.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| export const NULL_SENTINEL = "$@null(("; | ||
| export const CIRCULAR_REFERENCE_SENTINEL = "$@circular(("; | ||
| const DOT_ESCAPE = "$@dot"; |
There was a problem hiding this comment.
🚩 Backward compatibility: old unflatten code won't understand $@dot in keys
This change alters the wire format of flattened attributes. Keys that previously contained literal dots (e.g., "a.b" for a key with a dot) now contain $@dot (e.g., "a$@dotb"). This is a cross-boundary format used between the SDK (running in user task containers) and the webapp. If a newer SDK (with dot escaping) sends OTEL attributes to an older webapp (without $@dot unescaping), the webapp will see literal $@dot in reconstructed keys instead of the intended dots. The reverse direction (old SDK → new webapp) is safe since old data contains no $@dot sequences. In practice, the webapp is typically upgraded before users update their SDK, so the risky direction is uncommon — but self-hosted users upgrading their SDK independently could hit this. Consider whether this warrants a version negotiation mechanism or a note in the changelog.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Object keys containing periods (e.g. 'Key 0.002mm') were incorrectly
split on the '.' delimiter during flatten/unflatten, causing data
corruption in the dashboard logs view. Escape dots with $@dot sentinel
during flattening and unescape during unflattening.
Fixes #1510