add dataset name and type to home api response #1271
Conversation
WalkthroughThe pull request introduces a new enumeration ( Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant HomeResponseGenerator
participant StreamDataProcessor
participant StatsService
Caller->>HomeResponseGenerator: Request HomeResponse
HomeResponseGenerator->>StreamDataProcessor: Process stream_jsons
StreamDataProcessor-->>HomeResponseGenerator: Return DataSet (title, type)
HomeResponseGenerator->>StatsService: Call stats_for_date with stream_wise_stream_json
StatsService-->>HomeResponseGenerator: Return stats
HomeResponseGenerator-->>Caller: Return HomeResponse with datasets & stats
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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:
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/prism/home/mod.rs(6 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/prism/home/mod.rs (5)
src/alerts/mod.rs (3)
new(99-105)new(140-152)new(163-169)src/handlers/http/cluster/utils.rs (4)
new(35-47)new(61-77)new(92-110)new(122-129)src/storage/mod.rs (3)
new(182-184)new(195-201)default(205-225)src/parseable/mod.rs (1)
new(140-162)src/metadata.rs (1)
new(95-130)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- 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 (7)
src/prism/home/mod.rs (7)
65-70: New Enum: DataSetTypeThe enum properly categorizes dataset types as Logs, Metrics, or Traces. Good use of the
DebugandSerializederive macros for debugging and JSON serialization.
72-76: New Struct: DataSetClean implementation of the DataSet struct with title and type fields. The structure aligns well with the PR objective of adding dataset name and type to the API response.
86-86: HomeResponse UpdateThe HomeResponse struct is properly updated to include the new datasets field, which will allow the API to return dataset information to clients.
178-178: Initialize Datasets CollectionAppropriate initialization of the datasets vector before populating it in the loop.
182-182: Variable Naming ImprovementGood renaming of variables to be more concise while maintaining clarity:
stream_wise_stream_json(from likelystream_wise_ingestor_stream_json)stream_jsons(from likelyingestor_stream_jsons)The simplified naming makes the code more readable while preserving functionality.
Also applies to: 194-194, 203-203, 205-205
227-227: Updated Function CallThe function call is correctly updated to use the renamed variable.
238-240: Including Datasets in ResponseThe datasets vector is properly included in the HomeResponse, fulfilling the PR objective of including dataset information in the API response.
| let log_source = &stream_jsons[0].clone().log_source; | ||
|
|
||
| // if log_source_format is otel-metrics, set DataSetType to metrics | ||
| //if log_source_format is otel-traces, set DataSetType to traces | ||
| //else set DataSetType to logs | ||
|
|
||
| let dataset_type = match log_source[0].log_source_format { | ||
| LogSource::OtelMetrics => DataSetType::Metrics, | ||
| LogSource::OtelTraces => DataSetType::Traces, | ||
| _ => DataSetType::Logs, | ||
| }; | ||
|
|
||
| let dataset = DataSet { | ||
| title: stream.clone(), | ||
| dataset_type, | ||
| }; | ||
| datasets.push(dataset); | ||
| } |
There was a problem hiding this comment.
Add Missing Error Handling for Empty Collections
The code assumes stream_jsons and log_source always have at least one element, which could cause panic if empty.
Add error handling to prevent panic:
-let log_source = &stream_jsons[0].clone().log_source;
+if stream_jsons.is_empty() {
+ continue;
+}
+let log_source = &stream_jsons[0].clone().log_source;
+
+if log_source.is_empty() {
+ continue;
+}
let dataset_type = match log_source[0].log_source_format {
LogSource::OtelMetrics => DataSetType::Metrics,
LogSource::OtelTraces => DataSetType::Traces,
_ => DataSetType::Logs,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let log_source = &stream_jsons[0].clone().log_source; | |
| // if log_source_format is otel-metrics, set DataSetType to metrics | |
| //if log_source_format is otel-traces, set DataSetType to traces | |
| //else set DataSetType to logs | |
| let dataset_type = match log_source[0].log_source_format { | |
| LogSource::OtelMetrics => DataSetType::Metrics, | |
| LogSource::OtelTraces => DataSetType::Traces, | |
| _ => DataSetType::Logs, | |
| }; | |
| let dataset = DataSet { | |
| title: stream.clone(), | |
| dataset_type, | |
| }; | |
| datasets.push(dataset); | |
| } | |
| if stream_jsons.is_empty() { | |
| continue; | |
| } | |
| let log_source = &stream_jsons[0].clone().log_source; | |
| if log_source.is_empty() { | |
| continue; | |
| } | |
| let dataset_type = match log_source[0].log_source_format { | |
| LogSource::OtelMetrics => DataSetType::Metrics, | |
| LogSource::OtelTraces => DataSetType::Traces, | |
| _ => DataSetType::Logs, | |
| }; | |
| let dataset = DataSet { | |
| title: stream.clone(), | |
| dataset_type, | |
| }; | |
| datasets.push(dataset); | |
| } |
There was a problem hiding this comment.
if the stream is found, stream_json always exists, and for a stream_json, log_source always exists, do we still need a check?
There was a problem hiding this comment.
@nikhilsinhaparseable Thank you for this clarification. I understand now that these are system invariants - if a stream is found, stream_jsons will always exist with at least one element, and for any valid stream_json, the log_source array will always have at least one element as well. This explains why the additional null checks aren't necessary in this context.
✏️ Learnings added
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1271
File: src/prism/home/mod.rs:207-224
Timestamp: 2025-03-26T06:13:48.529Z
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.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
added field
Summary by CodeRabbit
New Features
Refactor