Skip to content

fix(v3): handle dot keys in flattenAttributes (#1510) #3185

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 2 commits into from
+80 −20
Closed

fix(v3): handle dot keys in flattenAttributes (#1510) #3185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9377289
fix(cli): respect --profile flag by lazily evaluating default profile…
Mar 2, 2026
926f2b1
fix(core): escape dots in log attribute keys to prevent incorrect unf…
Mar 4, 2026
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/cli-v3/src/cli/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,17 @@ export const CommonCommandOptions = z.object({
apiUrl: z.string().optional(),
logLevel: z.enum(["debug", "info", "log", "warn", "error", "none"]).default("log"),
skipTelemetry: z.boolean().default(false),
profile: z.string().default(readAuthConfigCurrentProfileName()),
profile: z
.string()
.optional()
.transform((v) => v ?? readAuthConfigCurrentProfileName()),
});

export type CommonCommandOptions = z.infer<typeof CommonCommandOptions>;

export function commonOptions(command: Command) {
return command
.option("--profile <profile>", "The login profile to use", readAuthConfigCurrentProfileName())
.option("--profile <profile>", "The login profile to use")
.option("-a, --api-url <value>", "Override the API URL", CLOUD_API_URL)
.option(
"-l, --log-level <level>",
Expand All @@ -30,9 +33,9 @@ export function commonOptions(command: Command) {
.option("--skip-telemetry", "Opt-out of sending telemetry");
}

export class SkipLoggingError extends Error {}
export class SkipCommandError extends Error {}
export class OutroCommandError extends SkipCommandError {}
export class SkipLoggingError extends Error { }
export class SkipCommandError extends Error { }
export class OutroCommandError extends SkipCommandError { }

export async function handleTelemetry(action: () => Promise<void>) {
try {
Expand Down
30 changes: 22 additions & 8 deletions packages/core/src/v3/utils/flattenAttributes.ts
View file
Open in desktop
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot Mar 5, 2026

Choose a reason for hiding this comment

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

🚩 Missing changeset for public package changes

Per CLAUDE.md and CONTRIBUTING.md, changes to packages under packages/ require a changeset (pnpm run changeset:add). This PR modifies packages/core and packages/cli-v3 (both public packages) but no .changeset/ file appears in the diff. This may be intentional if changesets are added separately, but it's worth confirming before merge.

Open in Devin Review

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

Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
import { Attributes } from "@opentelemetry/api";

function escapeKey(key: string): string {
return key.replace(/\./g, "\\.");
}
Comment on lines +3 to +5
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot Mar 5, 2026

Choose a reason for hiding this comment

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

🟡 Incomplete escape scheme: keys containing backslashes cause incorrect unflattening (regression)

The new escapeKey only escapes dots (. → \.) but does not escape existing backslash characters in keys. This means unflattenAttributes cannot distinguish between an escaped dot \. (intended as a literal dot in a key) and a literal backslash at the end of a key followed by the path separator dot. This is a regression from the old behavior.

Reproduction scenario

Consider flattenAttributes({ "a\\": { b: "value" } }) (key is literal a\):

  1. escapeKey("a\\") → "a\\" (no dots, no change)
  2. Nested key b produces flattened key "a\\.b" (chars: a, , ., b)
  3. Old unflattenAttributes used split(".") → ["a\\", "b"] → correctly creates { "a\\": { b: "value" } }
  4. New unflattenAttributes uses split(/(?<!\\)\./) → the dot IS preceded by \, so lookbehind prevents splitting → ["a\\.b"] → unescapeKey converts \. to . → incorrectly creates { "a.b": "value" }

The fix requires also escaping backslashes in escapeKey (\ → \\) and updating unescapeKey and the split regex accordingly.

Prompt for agents 
In packages/core/src/v3/utils/flattenAttributes.ts, the escapeKey/unescapeKey functions at lines 3-9 need to handle backslash escaping in addition to dot escaping to avoid ambiguity. The fix:

1. escapeKey (line 3-5): First escape backslashes, then escape dots:
   function escapeKey(key: string): string {
     return key.replace(/\\/g, "\\\\").replace(/\./g, "\\.");
   }

2. unescapeKey (line 7-9): Reverse the escaping in opposite order:
   function unescapeKey(key: string): string {
     return key.replace(/\\\./g, ".").replace(/\\\\\\\\/g, "\\");
   }

3. The split regex at line 293 needs to be updated to split on dots preceded by an even number of backslashes (including zero). A simple negative lookbehind (?<!\\) is insufficient. Consider using a custom split function or a regex like split(/(?<!(?:^|[^\\])(?:\\\\)*\\)\./) or implement a character-by-character split function that tracks escape state.

All three changes must be coordinated. Update the tests in packages/core/test/flattenAttributes.test.ts to cover keys containing literal backslashes.
Open in Devin Review

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


function unescapeKey(key: string): string {
return key.replace(/\\\./g, ".");
}
Comment on lines +3 to +9
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot Mar 5, 2026

Choose a reason for hiding this comment

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

🚩 Version mismatch risk: new flatten with old unflatten

If a newer SDK client uses the new flattenAttributes (which escapes dots in keys, e.g. "a\\.b"), but the webapp server is still running the old unflattenAttributes (which uses split(".")), the old code would split "a\\.b" into ["a\\", "b"] — creating a key a\ nested into b instead of the intended single key a.b. This creates a brief window during rolling deploys where data could be incorrectly unflattened. The impact depends on whether the SDK and webapp packages are always deployed in lockstep.

Open in Devin Review

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



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

Expand All @@ -24,7 +33,7 @@ class AttributeFlattener {
constructor(
private maxAttributeCount?: number,
private maxDepth: number = DEFAULT_MAX_DEPTH
) {}
) { }

get attributes(): Attributes {
return this.result;
Expand Down Expand Up @@ -117,7 +126,8 @@ class AttributeFlattener {
if (!this.canAddMoreAttributes()) break;
// Use the key directly if it's a string, otherwise convert it
const keyStr = typeof key === "string" ? key : String(key);
this.#processValue(value, `${prefix || "map"}.${keyStr}`, depth);
this.#processValue(value, `${prefix || "map"}.${escapeKey(keyStr)}`, depth);

}
return;
}
Expand Down Expand Up @@ -200,7 +210,9 @@ class AttributeFlattener {
break;
}

const newPrefix = `${prefix ? `${prefix}.` : ""}${Array.isArray(obj) ? `[${key}]` : key}`;
const escapedKey = Array.isArray(obj) ? `[${key}]` : escapeKey(key);
const newPrefix = `${prefix ? `${prefix}.` : ""}${escapedKey}`;


if (Array.isArray(value)) {
for (let i = 0; i < value.length; i++) {
Expand Down Expand Up @@ -278,25 +290,27 @@ export function unflattenAttributes(
continue;
}

const parts = key.split(".").reduce(
const parts = key.split(/(?<!\\)\./).reduce(
(acc, part) => {
if (part.startsWith("[") && part.endsWith("]")) {
const unescapedPart = unescapeKey(part);
if (unescapedPart.startsWith("[") && unescapedPart.endsWith("]")) {
// Handle array indices more precisely
const match = part.match(/^\[(\d+)\]$/);
const match = unescapedPart.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(unescapedPart.slice(1, -1));
}
} else {
acc.push(part);
acc.push(unescapedPart);
}
return acc;
},
[] as (string | number)[]
);


// Skip keys that exceed max depth to prevent memory exhaustion
if (parts.length > maxDepth) {
continue;
Expand Down
57 changes: 50 additions & 7 deletions packages/core/test/flattenAttributes.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { describe, it, expect } from "vitest";
import { flattenAttributes, unflattenAttributes } from "../src/v3/utils/flattenAttributes.js";


describe("flattenAttributes", () => {
it("handles number keys correctly", () => {
expect(flattenAttributes({ bar: { "25": "foo" } })).toEqual({ "bar.25": "foo" });
Expand All @@ -15,6 +17,19 @@ describe("flattenAttributes", () => {
expect(unflattenAttributes({ "bar.25": "foo" })).toEqual({ bar: { 25: "foo" } });
});

it("handles keys with periods correctly", () => {
const obj = { "Key 0.002mm": 31.4 };
const flattened = flattenAttributes(obj);
expect(flattened).toEqual({ "Key 0\\.002mm": 31.4 });
expect(unflattenAttributes(flattened)).toEqual(obj);

const nestedObj = { parent: { "child.key": "value" } };
const nestedFlattened = flattenAttributes(nestedObj);
expect(nestedFlattened).toEqual({ "parent.child\\.key": "value" });
expect(unflattenAttributes(nestedFlattened)).toEqual(nestedObj);
});


it("handles null correctly", () => {
expect(flattenAttributes(null)).toEqual({ "": "$@null((" });
expect(unflattenAttributes({ "": "$@null((" })).toEqual(null);
Expand Down Expand Up @@ -297,9 +312,9 @@ describe("flattenAttributes", () => {
});

it("handles function values correctly", () => {
function namedFunction() {}
const anonymousFunction = function () {};
const arrowFunction = () => {};
function namedFunction() { }
const anonymousFunction = function () { };
const arrowFunction = () => { };

const result = flattenAttributes({
named: namedFunction,
Expand All @@ -317,7 +332,7 @@ describe("flattenAttributes", () => {
it("handles mixed problematic types", () => {
const complexObj = {
error: new Error("Mixed error"),
func: function testFunc() {},
func: function testFunc() { },
date: new Date("2023-01-01"),
normal: "string",
number: 42,
Expand Down Expand Up @@ -415,10 +430,10 @@ describe("flattenAttributes", () => {
it("handles Promise objects correctly", () => {
const resolvedPromise = Promise.resolve("value");
const rejectedPromise = Promise.reject(new Error("failed"));
const pendingPromise = new Promise(() => {}); // Never resolves
const pendingPromise = new Promise(() => { }); // Never resolves

// Catch the rejection to avoid unhandled promise rejection warnings
rejectedPromise.catch(() => {});
rejectedPromise.catch(() => { });

const result = flattenAttributes({
resolved: resolvedPromise,
Expand Down Expand Up @@ -481,7 +496,7 @@ describe("flattenAttributes", () => {
it("handles complex mixed object with all special types", () => {
const complexObj = {
error: new Error("Test error"),
func: function testFunc() {},
func: function testFunc() { },
date: new Date("2023-01-01"),
mySet: new Set([1, 2, 3]),
myMap: new Map([["key", "value"]]),
Expand Down Expand Up @@ -629,6 +644,34 @@ describe("unflattenAttributes", () => {
});
});

it("handles keys with periods correctly (literal keys vs. nested paths)", () => {
const flattened = {
"user.name": "John Doe",
"user\\.email": "john.doe@example.com",
"data.version": "1.0",
"file.name.with\\.dots": "document.pdf",
};

const expected = {
user: {
name: "John Doe",
},
"user.email": "john.doe@example.com",
data: {
version: "1.0",
},
file: {
name: {
"with.dots": "document.pdf",
},
},
};


expect(unflattenAttributes(flattened)).toEqual(expected);
});


it("respects maxDepth limit and skips overly deep keys", () => {
// Create a flattened object with keys at various depths
const flattened = {
Expand Down