chore: server side improvements #1396
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. increase dataset fields limit to 1000 2. add hostname to manifest file path 3. update hostname handling 4. list alerts to be sorted on state and then on severity
WalkthroughThis update increases the dataset field count limit from 250 to 1000, adds robust hostname retrieval and sanitization for filename and manifest generation, and introduces sorting logic for alert summaries by state and severity with pagination and tag filtering. It also enhances alert enums with ordering traits and improves error detail. No public API signatures were changed; only internal logic and constants were updated. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server (alerts.rs)
participant AlertStore
Client->>Server (alerts.rs): GET /alerts?tags=&offset=&limit=
Server (alerts.rs)->>Server (alerts.rs): Parse and validate query parameters
alt Invalid parameters
Server (alerts.rs)-->>Client: Return InvalidQueryParameter error with details
else Valid parameters
Server (alerts.rs)->>AlertStore: Fetch alert summaries filtered by tags
AlertStore-->>Server (alerts.rs): Return alert summaries
Server (alerts.rs)->>Server (alerts.rs): Sort alerts by state and severity
Server (alerts.rs)->>Server (alerts.rs): Paginate results by offset and limit
Server (alerts.rs)-->>Client: Return sorted and paginated alerts as JSON
end
sequenceDiagram
participant Stream
participant System
participant ULID
Stream->>System: Get hostname
alt Hostname retrieval fails or invalid UTF-8
Stream->>ULID: Generate new ULID string
end
Stream->>Stream: Sanitize hostname (alphanumeric, -, _)
Stream->>Stream: Use sanitized hostname in filename
sequenceDiagram
participant ObjectStorage
participant System
participant ULID
ObjectStorage->>System: Get hostname
alt Hostname retrieval fails or invalid UTF-8
ObjectStorage->>ULID: Generate new ULID string
end
ObjectStorage->>ObjectStorage: Sanitize hostname
ObjectStorage->>ObjectStorage: Build manifest filename with hostname
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
✨ Finishing Touches
🧪 Generate unit tests
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/storage/object_storage.rs (1)
1014-1022: Robust hostname handling with minor optimization opportunity.The hostname retrieval and sanitization logic is well-implemented with proper fallbacks. However, there's redundant character collection on lines 1021-1022.
Apply this diff to remove the redundant character collection:
- .chars() - .filter(|c| c.is_alphanumeric() || *c == '-' || *c == '_') - .collect::<String>() - .chars() - .collect::<String>(); + .chars() + .filter(|c| c.is_alphanumeric() || *c == '-' || *c == '_') + .collect::<String>();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/cli.rs(1 hunks)src/handlers/http/alerts.rs(1 hunks)src/parseable/streams.rs(2 hunks)src/storage/object_storage.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1276
File: src/prism/logstream/mod.rs:335-338
Timestamp: 2025-03-28T06:30:13.566Z
Learning: The datasets API in Parseable is exclusively used by the datasets page in the Parseable UI and only queries the specific fields "p_src_ip" and "p_user_agent", making additional field validation unnecessary.
📚 Learning: the datasets api in parseable is exclusively used by the datasets page in the parseable ui and only ...
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1276
File: src/prism/logstream/mod.rs:335-338
Timestamp: 2025-03-28T06:30:13.566Z
Learning: The datasets API in Parseable is exclusively used by the datasets page in the Parseable UI and only queries the specific fields "p_src_ip" and "p_user_agent", making additional field validation unnecessary.
Applied to files:
src/cli.rs
📚 Learning: in the parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlle...
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1388
File: src/alerts/mod.rs:88-104
Timestamp: 2025-07-24T11:09:21.781Z
Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.
Applied to files:
src/handlers/http/alerts.rs
📚 Learning: in parseable's logstream handlers, stream existence checks must be performed for both query and stan...
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.
Applied to files:
src/parseable/streams.rs
📚 Learning: in parseable's ingest or query mode, the node_id is always available because it's generated during s...
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1346
File: src/parseable/streams.rs:319-331
Timestamp: 2025-06-16T09:50:38.636Z
Learning: In Parseable's Ingest or Query mode, the node_id is always available because it's generated during server initialization itself, before the get_node_id_string() function in streams.rs would be called. This makes the .expect() calls on QUERIER_META.get() and INGESTOR_META.get() safe in this context.
Applied to files:
src/parseable/streams.rs
📚 Learning: in parseable server, filenames for arrow files are generated server-side during ingestion using a st...
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1346
File: src/storage/object_storage.rs:838-840
Timestamp: 2025-06-16T02:09:49.200Z
Learning: In Parseable server, filenames for arrow files are generated server-side during ingestion using a strict format pattern: `{stream_hash}.date={date}.hour={hour}.minute={minute}.{custom_partitions}{hostname}.data.{extension}`. This guarantees that when splitting by dots, the second element will always be `date={value}`, making direct indexing safe when extracting the date part by splitting on '=' and accessing index 1.
Applied to files:
src/parseable/streams.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: coverage
🔇 Additional comments (4)
src/cli.rs (1)
38-38: LGTM! Dataset field limit increase implemented correctly.The constant has been properly updated from 250 to 1000 fields, aligning with the PR objective to increase dataset capacity.
src/handlers/http/alerts.rs (1)
64-98: LGTM! Alert sorting logic correctly implemented.The sorting implementation properly prioritizes alerts by state (Triggered > Silenced > Resolved) and then by severity (Critical > High > Medium > Low). The helper closures provide clean mapping to numeric priorities, and unknown values are appropriately handled with lowest priority.
src/storage/object_storage.rs (1)
1024-1038: LGTM! Manifest file naming correctly incorporates hostname.The updated manifest file naming scheme properly includes the sanitized hostname in both ingest mode (
ingestor.{hostname}.{id}.{MANIFEST_FILE}) and other modes ({hostname}.{MANIFEST_FILE}), fulfilling the PR objective to add hostname to the manifest file path.src/parseable/streams.rs (1)
44-44: LGTM! Clean import addition.The ULID import is necessary for the new hostname fallback logic and is properly placed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements