Skip to content

fix: escape periods in attribute keys during flatten/unflatten #3975

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
deepshekhardas wants to merge 1 commit into from
+32 −5
Closed

fix: escape periods in attribute keys during flatten/unflatten #3975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions packages/core/src/v3/utils/flattenAttributes.ts
View file
Open in desktop

@devin-ai-integration devin-ai-integration Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 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)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Attributes } from "@opentelemetry/api";

export const NULL_SENTINEL = "$@null((";
export const CIRCULAR_REFERENCE_SENTINEL = "$@circular((";
const DOT_ESCAPE = "$@dot";

@devin-ai-integration devin-ai-integration Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


const DEFAULT_MAX_DEPTH = 128;

Expand Down Expand Up @@ -200,7 +201,8 @@ class AttributeFlattener {
break;
}

const newPrefix = `${prefix ? `${prefix}.` : ""}${Array.isArray(obj) ? `[${key}]` : key}`;
const escapedKey = Array.isArray(obj) ? `[${key}]` : key.replace(/\./g, DOT_ESCAPE);
const newPrefix = `${prefix ? `${prefix}.` : ""}${escapedKey}`;

if (Array.isArray(value)) {
for (let i = 0; i < value.length; i++) {
Expand Down Expand Up @@ -280,17 +282,18 @@ export function unflattenAttributes(

const parts = key.split(".").reduce(
(acc, part) => {
if (part.startsWith("[") && part.endsWith("]")) {
const unescaped = part.split(DOT_ESCAPE).join(".");

@devin-ai-integration devin-ai-integration Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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 $@dot back 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.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

if (unescaped.startsWith("[") && unescaped.endsWith("]")) {
// Handle array indices more precisely
const match = part.match(/^\[(\d+)\]$/);
const match = unescaped.match(/^\[(\d+)\]$/);
if (match && match[1]) {
acc.push(parseInt(match[1]));
} else {
// Remove brackets for non-numeric array keys
acc.push(part.slice(1, -1));
acc.push(unescaped.slice(1, -1));
}
} else {
acc.push(part);
acc.push(unescaped);
}
return acc;
},
Expand Down
24 changes: 24 additions & 0 deletions packages/core/test/flattenAttributes.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,30 @@
import { flattenAttributes, unflattenAttributes } from "../src/v3/utils/flattenAttributes.js";

describe("flattenAttributes", () => {
it("escapes periods in object keys", () => {
const obj = { "Key 0.002mm": 31.4 };
const flattened = flattenAttributes(obj);
expect(flattened).toEqual({ "Key 0$@dot002mm": 31.4 });
expect(unflattenAttributes(flattened)).toEqual(obj);
});

it("handles multiple periods in a single key", () => {
const obj = { "a.b.c": "value" };
const flattened = flattenAttributes(obj);
expect(flattened).toEqual({ "a$@dotb$@dotc": "value" });
expect(unflattenAttributes(flattened)).toEqual(obj);
});

it("handles period keys nested inside regular objects", () => {
const obj = { meta: { "Key 0.002mm": 31.4, "version": 2 } };
const flattened = flattenAttributes(obj);
expect(flattened).toEqual({
"meta.Key 0$@dot002mm": 31.4,
"meta.version": 2,
});
expect(unflattenAttributes(flattened)).toEqual(obj);
});

it("handles number keys correctly", () => {
expect(flattenAttributes({ bar: { "25": "foo" } })).toEqual({ "bar.25": "foo" });
expect(unflattenAttributes({ "bar.25": "foo" })).toEqual({ bar: { "25": "foo" } });
Expand Down