revamp home api #1307
revamp home api #1307
Conversation
update the implementation of `/api/prism/v1/home`
remove titles of alerts, dashboards, correlations, filters from home api
updated response -
```
{
"alerts_info": {
"total": 0,
"silenced": 0,
"resolved": 0,
"triggered": 0,
"low": 0,
"medium": 0,
"high": 0,
"critical": 0
},
"stats_details": [
{
"date": "2025-04-30",
"events": 0,
"ingestion_size": 0,
"storage_size": 0
},
{
"date": "2025-05-01",
"events": 0,
"ingestion_size": 0,
"storage_size": 0
},
{
"date": "2025-05-02",
"events": 3866860,
"ingestion_size": 5260602290,
"storage_size": 736510108
},
{
"date": "2025-05-03",
"events": 0,
"ingestion_size": 0,
"storage_size": 0
},
{
"date": "2025-05-04",
"events": 0,
"ingestion_size": 0,
"storage_size": 0
},
{
"date": "2025-05-05",
"events": 0,
"ingestion_size": 0,
"storage_size": 0
},
{
"date": "2025-05-06",
"events": 2027400,
"ingestion_size": 2757834546,
"storage_size": 384782870
}
],
"datasets": [
{
"title": "test15",
"dataset_type": "Logs"
}
]
}
```
add another api to be called from prism
`/api/prism/v1/home/search`
server sends title and id of alerts, dashboards, correlations, filters
```
{
"alerts": [],
"correlations": [],
"dashboards": [],
"filters": [
{
"title": "body not null",
"id": "e71d1affa4ad72136e03092a717a4b0e0c3fd6d643a09572ad65e1748a5c2df8"
},
{
"title": "select *",
"id": "6fe16f99b05566d7d4a598a7f0fa2604379d75164cdda5657c7fdbf61e54a555"
}
]
}
```
update datasets api -
update readers priviledge - add `GetRetention` action
remove distinct values from datasets api
prism to call the query apis directly for p_src_ip and p_user_agent
|
""" WalkthroughThis change splits the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant PrismHomeHandlers
participant HomeLogic
Client->>Server: GET /home
Server->>PrismHomeHandlers: home_api(req)
PrismHomeHandlers->>HomeLogic: generate_home_response(key)
HomeLogic-->>PrismHomeHandlers: HomeResponse
PrismHomeHandlers-->>Server: JSON(HomeResponse)
Server-->>Client: Response
Client->>Server: GET /home/search
Server->>PrismHomeHandlers: home_search(req)
PrismHomeHandlers->>HomeLogic: generate_home_search_response(key, query)
HomeLogic-->>PrismHomeHandlers: HomeSearchResponse
PrismHomeHandlers-->>Server: JSON(HomeSearchResponse)
Server-->>Client: Response
Possibly related PRs
Suggested labels
Poem
📜 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 (10)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 2
🧹 Nitpick comments (3)
src/prism/home/mod.rs (3)
47-52: StreamStats appears unused & carries commented-out fields
StreamStatsis declared but nothing in this module (nor the response type) uses it except for the temporarysummaryvariable created later.
Keeping dead code and commented-out field stubs (stream_count,log_source_count) adds noise and invites drift.Consider deleting the struct (and the accompanying aggregation code) or, if you intend to expose a summary in the JSON payload soon, actually add it to
HomeResponse.
148-156:summaryis computed but never returnedThe loop aggregates
events,ingestion, andstorageintosummary.stats_summary.*, yetsummaryis never used after this block.
This is both a wasted O(n) pass and will raise an “unused variable” compiler warning.Two quick fixes:
- let mut summary = StreamStats::default(); + // let mut summary = StreamStats::default(); // remove until we need itand inside the loop:
- summary.stats_summary.events += dated_stats.events; - summary.stats_summary.ingestion += dated_stats.ingestion_size; - summary.stats_summary.storage += dated_stats.storage_size;Or, if a total summary is valuable, add a
summary: StreamStatsfield toHomeResponseso callers can see the roll-up.
293-309: Minor: unnecessaryclone()ofSessionKeyin authorization loop
Users.authorizeonly needs an&SessionKey, but each iteration clones the key, allocating each time:Users.authorize(key.clone(), Action::ListStream, Some(logstream), None)You can pass a reference instead:
Users.authorize(key, Action::ListStream, Some(logstream), None)(assuming the signature supports it). This avoids repeated heap allocations in tight loops.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/handlers/http/modal/server.rs(1 hunks)src/handlers/http/prism_home.rs(2 hunks)src/prism/home/mod.rs(4 hunks)src/prism/logstream/mod.rs(1 hunks)src/rbac/role.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: coverage
- GitHub Check: Build Default x86_64-unknown-linux-gnu
🔇 Additional comments (9)
src/rbac/role.rs (1)
327-327: Appropriate permission addition for Reader role.Adding the
GetRetentionaction to the reader role permissions aligns with the PR objective of updating reader privileges. This allows readers to access retention information for streams they have permission to read.src/handlers/http/prism_home.rs (2)
22-22: Correctly updated import for new search functionality.The import statement now includes the new
generate_home_search_responsefunction along with the existing imports.
41-48: Well-implemented new handler for the home search endpoint.The
home_searchhandler follows the same pattern as the existinghome_apihandler, with proper session key extraction and error handling. This implementation is consistent with the existing code style and error handling approach.src/handlers/http/modal/server.rs (1)
151-155: Good refactoring of route handling for home API.The method has been changed from returning a single
Resourceto returning aScopethat contains two routes:
- The root path (
"") which maps to the existinghome_apihandler- A new
/searchpath which maps to the newhome_searchhandlerThis approach maintains backward compatibility while adding the new search endpoint, which aligns perfectly with the PR objectives.
src/prism/logstream/mod.rs (5)
35-36: Removed unnecessary imports related to distinct values functionality.The import statement has been simplified by removing imports that were specifically used for the now-removed distinct values feature. This is part of the overall cleanup related to removing the distinct values feature from the datasets API.
39-40: Simplified imports after removing distinct values feature.The import statement now focuses only on necessary components after the removal of the distinct values feature.
43-44: Removed TimeParseError import that was used for distinct values.This change completes the import cleanup related to the removed distinct values feature.
199-214: Dataset response structure appropriately simplified.The
PrismDatasetResponsestruct no longer includes thedistinct_sourcesfield that was previously used to store distinct values. This aligns with the PR objective to remove the distinct values feature from the datasets API. The structure now contains only the essential information needed for the response.
308-328: Streamlined dataset response construction.The
build_dataset_responsemethod has been simplified to no longer fetch and include distinct values. This directly implements the PR objective of removing the distinct values feature, potentially improving performance by eliminating unnecessary data fetching operations.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/prism/home/mod.rs (1)
346-363: Consider simplifying redundant error handlingSimilar to the dashboard titles function, there's redundant error handling here. As previously confirmed, filter IDs are guaranteed to never be None.
You could simplify this to:
- id: filter - .filter_id - .as_ref() - .ok_or_else(|| anyhow::Error::msg("Filter ID is null")) - .unwrap() - .clone(), + id: filter.filter_id.as_ref().unwrap().clone(),Again, the current implementation provides a better error message, so this is purely a stylistic suggestion.
🧹 Nitpick comments (2)
src/prism/home/mod.rs (2)
327-344: Consider simplifying redundant error handlingWhile this pattern works correctly (as confirmed in previous discussions), the
ok_or_else(...).unwrap()sequence is technically redundant since dashboard IDs are guaranteed to never be None.You could simplify this to:
- id: dashboard - .dashboard_id - .as_ref() - .ok_or_else(|| anyhow::Error::msg("Dashboard ID is null")) - .unwrap() - .clone(), + id: dashboard.dashboard_id.as_ref().unwrap().clone(),The current implementation provides a better error message, so this is purely a stylistic suggestion.
100-101: Consider adding a descriptive error messageFor consistency with other error handling patterns in this file, consider adding a descriptive error message to this unwrap.
- .unwrap() + .unwrap_or_else(|| panic!("Date calculation failed, this should never happen"))Or use a similar pattern to other parts of the code:
- .unwrap() + .expect("Date calculation failed")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/prism/home/mod.rs(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/prism/home/mod.rs (1)
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1307
File: src/prism/home/mod.rs:358-375
Timestamp: 2025-05-06T09:28:39.559Z
Learning: In Parseable, filter_id, dashboard_id, and correlation IDs are generated by the server at creation time and are guaranteed to never be None when listing/retrieving these resources.
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: coverage
🔇 Additional comments (3)
src/prism/home/mod.rs (3)
77-85: Well-structured separation of concerns!You've effectively split the response types to separate aggregate data (
HomeResponse) from search-related metadata (HomeSearchResponse). This aligns with the API changes described in the PR objectives, making each endpoint more focused and maintainable.
89-91: Clean concurrency model for simplified home responseThe
tokio::join!macro is now correctly updated to only fetch the data needed for the simplifiedHomeResponse, removing the fetching of title information which has been moved to the search endpoint.
257-278: Well-implemented search endpoint handlerThe new
generate_home_search_responsefunction effectively uses concurrency throughtokio::join!to fetch all title and ID data in parallel, following the same patterns as the main home response. The error handling is consistent and appropriate.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/prism/home/mod.rs (2)
333-334: Consider defensive unwrapping for dashboard IDsWhile I understand from prior discussions that dashboard IDs are generated by the server and can never be None, consider using a more descriptive unwrap pattern:
-id: dashboard.dashboard_id.as_ref().unwrap().clone(), +id: dashboard + .dashboard_id + .clone() + .unwrap_or_else(|| panic!("Dashboard ID unexpectedly null despite server guarantee")),This would provide more context in the extremely unlikely case of a panic.
347-348: Similar unwrap pattern for filter IDsThe same suggestion applies to filter IDs - consider a more descriptive unwrap for better debugging context if needed.
-id: filter.filter_id.as_ref().unwrap().clone(), +id: filter + .filter_id + .clone() + .unwrap_or_else(|| panic!("Filter ID unexpectedly null despite server guarantee")),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/prism/home/mod.rs(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/prism/home/mod.rs (1)
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1307
File: src/prism/home/mod.rs:358-375
Timestamp: 2025-05-06T09:28:39.559Z
Learning: In Parseable, filter_id, dashboard_id, and correlation IDs are generated by the server at creation time and are guaranteed to never be None when listing/retrieving these resources.
🧬 Code Graph Analysis (1)
src/prism/home/mod.rs (4)
src/alerts/mod.rs (4)
get_alerts_info(982-1019)new(114-120)new(155-167)new(178-184)src/parseable/mod.rs (1)
new(136-148)src/storage/mod.rs (2)
new(182-184)new(195-201)src/metadata.rs (1)
new(95-130)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: coverage
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
🔇 Additional comments (5)
src/prism/home/mod.rs (5)
79-85: Well-structured newHomeSearchResponsetypeThe new structure clearly separates search metadata from the main home response, maintaining a clean one-concern-per-endpoint design.
87-91: Simplified concurrency pattern ingenerate_home_responseYou've successfully refactored this function to focus on essential data, removing the title fetching operations that have been moved to the new search endpoint.
153-158: Streamlined response constructionThe
HomeResponsestructure has been properly updated to match the simplified data fetching pattern. This eliminates unnecessary metadata from the main home endpoint.
256-277: Well-implemented search endpoint handlerThe new
generate_home_search_responsefunction makes good use of concurrent execution withtokio::join!to efficiently fetch all the title data in parallel. Error handling is properly implemented with the?operator.
354-366: Error type unchanged and still comprehensiveThe
PrismHomeErrorenum correctly handles all the possible error cases for both the original home endpoint and the new search endpoint.
request - `http://localhost:8000/api/prism/v1/home/search?key=frontend`
response -
```
{
"resources": [
{
"id": "e890c2f70362528b57d528b65a39de62bbd3c65f90f6923dce25e27a8a66e8a4",
"name": "select flags not 0",
"resource_type": "Filter"
},
{
"id": "e71d1affa4ad72136e03092a717a4b0e0c3fd6d643a09572ad65e1748a5c2df8",
"name": "body not null",
"resource_type": "Filter"
},
{
"id": "6fe16f99b05566d7d4a598a7f0fa2604379d75164cdda5657c7fdbf61e54a555",
"name": "select *",
"resource_type": "Filter"
},
{
"id": "fcda9d6a2b98c3368cbf1d903bb408e59f084df2dcb7c7d5d7b044e800ec1437",
"name": "frontend limit",
"resource_type": "Filter"
}
]
}
```
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/prism/home/mod.rs (2)
378-378: IDs guaranteed to be non-null by the server.Based on previous discussions in PR comments, dashboard_id is guaranteed to be present by the server implementation, so this unwrap is safe. However, for better code readability, you might consider adding a comment explaining this guarantee.
405-405: IDs guaranteed to be non-null by the server.Similar to dashboard_id, filter_id is guaranteed to be present by the server implementation, making this unwrap safe. Consider adding a comment for clarity.
🧹 Nitpick comments (2)
src/prism/home/mod.rs (2)
107-107: Consider handling potential panic in date calculation.Using
expecthere could cause a runtime panic if the date conversion fails. While this is unlikely to happen in practice, consider using a more robust error handling approach.- .expect("Date conversion failed") + .ok_or_else(|| PrismHomeError::Anyhow(anyhow::Error::msg("Date conversion failed")))?
286-294: Consider aligning stream filtering approach with other resources.While other resources are filtered within their respective helper functions, stream filtering is done in the main function after fetching. Consider moving this filtering logic into the
get_stream_titlesfunction for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/handlers/http/prism_home.rs(2 hunks)src/prism/home/mod.rs(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: coverage
🔇 Additional comments (6)
src/prism/home/mod.rs (4)
67-71: Simplified HomeResponse structure aligns with PR objectives.The HomeResponse struct is now streamlined to only include essential data (alerts_info, stats_details, and datasets), removing all of the resource titles. This separation of concerns is good design - having basic information in the home response and detailed search functionality in a separate endpoint.
73-92: Well-structured resource representation.The new types (
ResourceType,Resource, andHomeSearchResponse) provide a clean and unified way to represent different resource types in the system. This abstraction allows for consistent handling of various resources in the search response.
263-296: Clean implementation of search functionality.The new
generate_home_search_responsefunction effectively fetches and filters resources based on the query string. The concurrent fetching of different resource types is a good approach for performance.
434-435: Good error handling for the search functionality.Adding a specific error variant for invalid query parameters with appropriate HTTP status code mapping is a good practice.
src/handlers/http/prism_home.rs (2)
21-24: Import updates for new search functionality.The imports are appropriately updated to include the new
generate_home_search_responsefunction.
26-26: Good use of constants for query parameters.Using a constant for the query parameter name enhances maintainability and reduces the risk of inconsistency.
update the implementation of
/api/prism/v1/homeremove titles of alerts, dashboards, correlations, filters from home api
updated response -
add another api to be called from prism
/api/prism/v1/home/searchserver sends title and id of alerts, dashboards, correlations, filters
update datasets api -
update readers priviledge - add
GetRetentionaction remove distinct values from datasets apiprism to call the query apis directly for p_src_ip and p_user_agent
Summary by CodeRabbit
New Features
/home/searchendpoint, providing search-related metadata for alerts, correlations, dashboards, filters, and datasets.Improvements
/homeendpoint to support multiple routes, allowing for more flexible data retrieval under the/homeprefix.Refactor
Bug Fixes