-
-
Notifications
You must be signed in to change notification settings - Fork 144
fix: flatten before detect #1361
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.
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
fix: flatten before detect #1361
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler (detect_schema)
participant Flatten Utils
Client->>Handler (detect_schema): Send JSON input
Handler->>Handler: Check nesting level
alt Too deeply nested
Handler-->>Client: Return BAD_REQUEST (cannot flatten)
else Not deeply nested
Handler->>Flatten Utils: generic_flattening
alt Flattening fails
Handler-->>Client: Return BAD_REQUEST (flattening failed)
else Flattening succeeds
Handler->>Flatten Utils: flatten::flatten
alt Detailed flattening fails
Handler-->>Client: Return BAD_REQUEST (flattening failed)
else Flattened JSON ready
Handler->>Handler: Convert to array & infer schema
Handler-->>Client: Return inferred schema
end
end
end
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (32)
✅ Files skipped from review due to trivial changes (24)
🧰 Additional context used🧠 Learnings (6)📓 Common learnings
src/handlers/http/users/dashboards.rs (4)
src/handlers/http/mod.rs (1)
src/utils/human_size.rs (1)
src/utils/mod.rs (1)
src/handlers/http/logstream.rs (6)
🧬 Code Graph Analysis (2)src/handlers/http/users/dashboards.rs (1)
src/handlers/http/cluster/utils.rs (1)
⏰ Context from checks skipped due to timeout of 90000ms (10)
🔇 Additional comments (15)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/handlers/http/logstream.rs (1)
137-143
: Consider providing more detailed error message.The error message for heavily nested JSON could be more helpful by indicating the actual nesting level or the configured limit.
- Err(StreamError::Custom { - msg: "heavily nested, cannot flatten this JSON".to_string(), - status: StatusCode::BAD_REQUEST, - }) + Err(StreamError::Custom { + msg: format!("JSON is too deeply nested (exceeds level {}), cannot flatten", + PARSEABLE.options.event_flatten_level), + status: StatusCode::BAD_REQUEST, + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/handlers/http/logstream.rs
(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1329
File: src/handlers/http/kinesis.rs:89-98
Timestamp: 2025-05-28T04:45:40.879Z
Learning: In Kinesis log processing, when handling deeply nested JSON that exceeds flattening limits, the original JSON structure must be preserved as-is. If the JSON is an array at the root level, attempting to add metadata fields like `requestId` and `timestamp` would fail because arrays don't support direct key-value insertion. The flattening process handles arrays by converting elements to individual objects, but when flattening is skipped, the original array structure must remain intact to prevent data loss.
Learnt from: de-sh
PR: parseablehq/parseable#1185
File: src/handlers/http/logstream.rs:255-261
Timestamp: 2025-02-14T09:49:25.818Z
Learning: In Parseable's logstream handlers, stream existence checks must be performed for both query and standalone modes. The pattern `!PARSEABLE.streams.contains(&stream_name) && (PARSEABLE.options.mode != Mode::Query || !PARSEABLE.create_stream_and_schema_from_storage(&stream_name).await?)` ensures proper error handling in both modes.
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1271
File: src/prism/home/mod.rs:207-224
Timestamp: 2025-03-26T06:13:48.898Z
Learning: In the Parseable codebase, if a stream is found, the stream_jsons array will always have at least one element. Additionally, for any valid stream_json object, the log_source array will always have at least one element. This is a design invariant that makes additional null checks unnecessary.
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1346
File: src/handlers/http/health_check.rs:81-90
Timestamp: 2025-06-16T02:04:58.990Z
Learning: In the shutdown function in src/handlers/http/health_check.rs, the design approach is to log errors from sync operations rather than propagate them. This is intentional because the shutdown function is called on SIGTERM/SIGINT signals, and the goal is to perform best-effort cleanup (syncing pending files to object storage) while allowing the shutdown to proceed regardless of sync failures. Logging provides debugging information without blocking the shutdown process.
src/handlers/http/logstream.rs (1)
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1329
File: src/handlers/http/kinesis.rs:89-98
Timestamp: 2025-05-28T04:45:40.879Z
Learning: In Kinesis log processing, when handling deeply nested JSON that exceeds flattening limits, the original JSON structure must be preserved as-is. If the JSON is an array at the root level, attempting to add metadata fields like `requestId` and `timestamp` would fail because arrays don't support direct key-value insertion. The flattening process handles arrays by converting elements to individual objects, but when flattening is skipped, the original array structure must remain intact to prevent data loss.
🧬 Code Graph Analysis (1)
src/handlers/http/logstream.rs (2)
src/utils/json/flatten.rs (4)
flatten
(58-93)convert_to_array
(351-357)generic_flattening
(269-328)has_more_than_max_allowed_levels
(335-348)src/event/format/mod.rs (2)
new
(126-131)override_data_type
(324-368)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: coverage
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
🔇 Additional comments (5)
src/handlers/http/logstream.rs (5)
33-35
: LGTM: Import additions are appropriate.The imported flatten utilities are necessary for the new flattening functionality in the
detect_schema
function.
120-125
: Approve the detailed flattening logic.The detailed flattening step with separator "_" and proper error handling is correctly implemented. The error message could be more descriptive, but the logic is sound.
126-130
: Good pattern matching for array conversion.The logic correctly handles both Array and Object cases from the flattened JSON, and the unreachable case is appropriate since the flatten function would have failed earlier for invalid types.
109-109
: No changes required for nesting level checkStarting at
current_level = 1
correctly treats the top-level JSON as level 1, aligns with the defaultevent_flatten_level
(10), and matches its use elsewhere in the codebase.
107-144
: Two-stage flattening is intentional and follows established patternI’ve confirmed that this same sequence—calling
generic_flattening
to expand arrays followed byflatten::flatten
to flatten nested objects—is used insrc/utils/json/mod.rs
. The two functions are complementary (one handles array branching, the other handles object nesting) and do not conflict. No changes needed.
Summary by CodeRabbit
Bug Fixes
New Features