Update: add last triggered timestamp to alerts #1419
Conversation
WalkthroughAdds an optional last_triggered_at timestamp to alert models and responses, initializes it to None on creation/migration, maps it across AlertConfig↔ThresholdAlert conversions, and updates it to the current UTC time when a ThresholdAlert transitions to Triggered. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant AlertRequest
participant AlertConfig
participant ThresholdAlert
participant Storage
rect rgb(240,248,255)
Note over AlertRequest,AlertConfig: Creation / Migration
Client->>AlertRequest: Create/Migrate alert
AlertRequest->>AlertConfig: into()
Note right of AlertConfig: last_triggered_at = None
AlertConfig-->>Storage: Persist config (includes last_triggered_at)
end
rect rgb(245,255,240)
Note over Client,ThresholdAlert: Runtime state update
Client->>ThresholdAlert: update_state(new_state)
alt new_state == Triggered
ThresholdAlert->>ThresholdAlert: set last_triggered_at = Utc::now()
else other states
ThresholdAlert->>ThresholdAlert: no change to last_triggered_at
end
ThresholdAlert-->>Storage: Serialize via serde (timestamp persisted)
end
rect rgb(255,250,240)
Note over AlertConfig,Client: Read path
Storage-->>AlertConfig: load()
AlertConfig->>Client: to_response() (includes last_triggered_at)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 0
🧹 Nitpick comments (5)
src/alerts/mod.rs (1)
620-680: Also include lastTriggeredAt in alert summaries (optional)Helps UIs show “last triggered” without another fetch.
Apply this diff:
@@ map.insert( "state".to_string(), serde_json::Value::String(self.state.to_string()), ); + if let Some(ts) = self.last_triggered_at { + map.insert( + "lastTriggeredAt".to_string(), + serde_json::Value::String(ts.to_string()), + ); + }src/alerts/alert_structs.rs (1)
432-439: Optional: surface lastTriggeredAt in AlertsInfo for summariesIf summaries should show “last triggered,” extend this DTO.
Proposed change:
pub struct AlertsInfo { pub title: String, pub id: Ulid, pub severity: Severity, + #[serde(skip_serializing_if = "Option::is_none")] + pub last_triggered_at: Option<DateTime<Utc>>, }Follow-up: populate this in
get_alerts_summary()when pushing entries.src/alerts/alert_types.rs (3)
194-203: Set last_triggered_at only on transition into TriggeredAvoids overwriting timestamp if updating to the same state.
Apply this diff:
- // update state in memory - self.state = new_state; - - // if new state is `Triggered`, change triggered at - if new_state.eq(&AlertState::Triggered) { - self.last_triggered_at = Some(Utc::now()); - } + // update state in memory + let prev_state = self.state; + self.state = new_state; + // update only on transition into Triggered + if new_state.eq(&AlertState::Triggered) && !prev_state.eq(&AlertState::Triggered) { + self.last_triggered_at = Some(Utc::now()); + }
226-236: De-duplicate logic and guard on real transitionMirror the transition guard in the normal path as well.
Apply this diff:
- // update state in memory - self.state = new_state; - - // if new state is `Triggered`, change triggered at - if new_state.eq(&AlertState::Triggered) { - self.last_triggered_at = Some(Utc::now()); - } + // update state in memory + let prev_state = self.state; + self.state = new_state; + // update only on transition into Triggered + if new_state.eq(&AlertState::Triggered) && !prev_state.eq(&AlertState::Triggered) { + self.last_triggered_at = Some(Utc::now()); + }
317-330: Use last_triggered_at in notification header (fallback to now)Aligns message timestamp with persisted trigger time.
Apply this diff:
- Ok(format!( + let triggered_at = self + .last_triggered_at + .map(|t| t.to_rfc3339()) + .unwrap_or_else(|| Utc::now().to_rfc3339()); + Ok(format!( "Alert Name: {}\nAlert Type: Threshold alert\nSeverity: {}\nTriggered at: {}\nThreshold: {}\nAlert ID: {}\nEvaluation Window: {}\nFrequency: {}\n\nValues crossing the threshold:", self.title, self.severity, - Utc::now().to_rfc3339(), + triggered_at, format_args!( "{} {}", self.threshold_config.operator, self.threshold_config.value ), self.id, self.get_eval_window(), self.get_eval_frequency() ))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/alerts/alert_structs.rs(4 hunks)src/alerts/alert_types.rs(5 hunks)src/alerts/mod.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/alerts/alert_types.rs (1)
src/alerts/mod.rs (5)
value(415-415)value(423-423)value(431-431)value(444-444)value(455-455)
⏰ 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: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Kafka 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 x86_64-pc-windows-msvc
- GitHub Check: Build Default aarch64-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
🔇 Additional comments (7)
src/alerts/mod.rs (1)
121-138: Initialize last_triggered_at during v1→v2 migration — LGTMSetting
last_triggered_at: Noneon migrated alerts is correct and keeps semantics explicit.src/alerts/alert_structs.rs (4)
319-320: Default last_triggered_at to None on create — LGTM
346-347: Add last_triggered_at to AlertConfig — LGTMSerde camelCase will expose this as
lastTriggeredAtin JSON.
372-373: Expose last_triggered_at on API response — LGTM
411-412: Preserve last_triggered_at in to_response — LGTMsrc/alerts/alert_types.rs (2)
65-66: Add last_triggered_at field to ThresholdAlert — LGTM
381-405: Mapping between ThresholdAlert and AlertConfig — LGTMThe new field is propagated both ways.
2 participants
Fixes #XXXX.
Description
This PR has:
Summary by CodeRabbit
New Features
Chores