Unflatten attributes of spans#119
Conversation
Change-Id: I829a16958d63f2da330c373ae87ed8a00cc09ab2 Co-developed-by: Cursor <noreply@cursor.com>
|
|
1 similar comment
|
|
Summary of ChangesHello @Cirilla-zmh, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the trace system's ability to handle diverse span attribute formats. By ceasing the automatic unflattening of attributes on the backend, the system now retains the original OpenTelemetry Protocol structure, which can improve data fidelity and readability. Concurrently, frontend components have been updated to intelligently interpret these attributes, whether they are stored in a flat or nested manner, through an improved utility function. This change ensures broader compatibility and a more robust display of trace data. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully refactors the attribute handling across the application to support both flat and nested span attribute structures. By removing the automatic unflattening in the backend and enhancing the getNestedValue utility, the system now offers greater flexibility and compatibility with various OTLP attribute formats. The changes in SpanPanel and TraceTree correctly leverage the updated utility, and the SpanProcessor's decodeAnyValue method has been significantly improved for robust attribute decoding. The token extraction logic in Trace.ts is also more resilient to different data types.
| // Keep attributes in original structure (no unflatten) - preserve as-is from OTLP | ||
| return this.loadJsonStrings( | ||
| this.decodeKeyValues(attrs), | ||
| ) as unknown as Attributes; |
There was a problem hiding this comment.
The removal of this.unflattenAttributes directly addresses the PR's goal of preserving the original OTLP structure, which is a significant architectural change.
| /** | ||
| * Recursively parse JSON strings in attributes so nested values (e.g. from | ||
| * kvlist_value or SDK) are usable. Only plain objects are recursed; arrays | ||
| * and primitives are left as-is to avoid breaking OTLP array/bytes. | ||
| */ | ||
| private static loadJsonStrings( | ||
| attributes: Record<string, unknown>, | ||
| ): Record<string, unknown> { | ||
| const result: Record<string, unknown> = {}; | ||
| for (const [key, value] of Object.entries(attributes)) { | ||
| if (typeof value === 'string') { | ||
| try { | ||
| result[key] = JSON.parse(value); | ||
| } catch { | ||
| result[key] = value; | ||
| } | ||
| } else { | ||
| result[key] = value; | ||
| } | ||
| result[key] = this.parseJsonValue(value); | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| private static parseJsonValue(value: unknown): unknown { | ||
| if (typeof value === 'string') { | ||
| try { | ||
| return this.parseJsonValue(JSON.parse(value)); | ||
| } catch { | ||
| return value; | ||
| } | ||
| } | ||
| if ( | ||
| value !== null && | ||
| typeof value === 'object' && | ||
| !Array.isArray(value) && | ||
| !(value instanceof Uint8Array) | ||
| ) { | ||
| const obj = value as Record<string, unknown>; | ||
| const out: Record<string, unknown> = {}; | ||
| for (const [k, v] of Object.entries(obj)) { | ||
| out[k] = this.parseJsonValue(v); | ||
| } | ||
| return out; | ||
| } | ||
| return value; | ||
| } |
There was a problem hiding this comment.
The introduction of parseJsonValue and its recursive application within loadJsonStrings is a robust solution for deeply parsing JSON strings embedded in attributes. This ensures that nested JSON structures are correctly deserialized without inadvertently modifying arrays or byte values.
| // Convert path to string if it's an array | ||
| const pathString = Array.isArray(path) ? path.join(separator) : path; | ||
|
|
||
| // First, try direct access for flat structure (e.g., "gen_ai.input.messages" as a key) | ||
| if (pathString in obj) { | ||
| return obj[pathString]; | ||
| } |
There was a problem hiding this comment.
The getNestedValue utility now first attempts direct key access for flat structures before falling back to nested access. This is a crucial enhancement for supporting both attribute formats as described in the pull request, ensuring backward compatibility and flexibility.
| if (typeof value === 'number' && !Number.isNaN(value)) return value; | ||
| if (typeof value === 'string') { | ||
| const n = Number(value); | ||
| return Number.isNaN(n) ? undefined : n; |
There was a problem hiding this comment.
The logic for extracting input and output tokens has been improved to handle string values by attempting conversion to a number. This adds robustness to the data processing.
| const hasGenAi = | ||
| Object.keys(attributes).some((key) => key.startsWith('gen_ai.')) || | ||
| getNestedValue(attributes, 'gen_ai') !== undefined; |
There was a problem hiding this comment.
The hasGenAi check is now more comprehensive, supporting both flat and nested attribute structures. This ensures better compatibility and accurate detection of gen_ai attributes.
|
This PR is marked as stale because there has been no activity for 30 days. Remove stale label or add new comments or this PR will be closed in 5 day. |
|
Close this stale PR. |
1 participant
Change-Id: I829a16958d63f2da330c373ae87ed8a00cc09ab2
Co-developed-by: Cursor noreply@cursor.com
Description
This PR updates the trace system to support both flat and nested span attribute structures by removing automatic unflattening and enhancing attribute access utilities.
The previous implementation automatically unflattened span attributes, which could reduce readability. This change preserves the original structure while ensuring both formats are supported for better compatibility.
Changes
Backend: Removed
unflattenAttributes()to preserve original OTLP structure. Enhanced value decoding to prioritize complex types and improved token extraction to handle string values.Frontend: Updated
SpanPanelandTraceTreecomponents to usegetNestedValue()utility for accessing span attributes, supporting both flat (e.g.,attributes["gen_ai.operation.name"]) and nested (e.g.,attributes.gen_ai.operation.name) structures.Utilities: Enhanced
getNestedValue()to first attempt direct key access for flat structures, then fall back to nested access for backward compatibility.Checklist
Please check the following items before code is ready to be reviewed.
npm run formatcommand in the root directory