Skip to content

feat: update monitoring parameters for LiteLLM and Typesense, add notification prompts#14

Merged
FabrizioCafolla merged 5 commits intomainfrom
feat/change-params
Dec 15, 2025
Merged

feat: update monitoring parameters for LiteLLM and Typesense, add notification prompts#14
FabrizioCafolla merged 5 commits intomainfrom
feat/change-params

Conversation

@FabrizioCafolla
Copy link
Copy Markdown
Member

@FabrizioCafolla FabrizioCafolla commented Dec 15, 2025

PR Type

Enhancement


Description

  • Add notification_prompts parameter to pod restart alerts for LiteLLM and Typesense

  • Update default duration value from 0 to 120 seconds for pod restart monitoring

  • Remove alignment_period parameter from Typesense configuration

  • Clean up inline comments in variable definitions


Diagram Walkthrough

flowchart LR
  variables["variables.tf<br/>Update defaults"] --> litellm["lite_llm.tf<br/>Add notification_prompts"]
  variables --> typesense["typesense.tf<br/>Add notification_prompts"]
  variables --> changelog["CHANGELOG.md<br/>Document changes"]
  variables --> readme["README.md<br/>Update documentation"]
Loading

File Walkthrough

Relevant files
Configuration changes
variables.tf
Update pod restart alert parameters and defaults                 

variables.tf

  • Add notification_prompts parameter with default ["OPENED", "CLOSED"]
    for both LiteLLM and Typesense pod restart alerts
  • Change default duration from 0 to 120 seconds for pod restart
    monitoring
  • Remove alignment_period parameter from Typesense pod_restart
    configuration
  • Remove inline comments from Typesense variable definition
+5/-7     
Enhancement
lite_llm.tf
Add notification prompts to LiteLLM pod restart alerts     

lite_llm.tf

  • Add notification_prompts field to alert strategy block using value
    from pod_restart configuration
+1/-0     
typesense.tf
Add notification prompts to Typesense pod restart alerts 

typesense.tf

  • Add notification_prompts field to alert strategy block using value
    from pod_restart configuration
+1/-0     
Documentation
CHANGELOG.md
Document version 0.9.0 release changes                                     

CHANGELOG.md

  • Add version 0.9.0 release entry dated 2025-12-15
  • Document addition of notification_prompts parameter
  • Document modification of default duration and alignment_period values
+12/-0   
README.md
Update input variable documentation for LiteLLM and Typesense

README.md

  • Update LiteLLM input variable documentation to reflect new duration
    default (120) and notification_prompts parameter
  • Update Typesense input variable documentation to reflect new duration
    default (120), removed alignment_period, and added
    notification_prompts parameter
+2/-2     

@sparkfabrik-ai-bot
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Inconsistent Configuration

The alignment_period parameter was removed from Typesense's pod_restart configuration but remains in LiteLLM's configuration. This inconsistency may cause confusion and should be intentional. If both services use similar monitoring logic, they should have consistent parameter sets.

pod_restart = optional(object({
  threshold          = optional(number, 0)
  duration           = optional(number, 120)
  auto_close_seconds = optional(number, 3600)
  notification_prompts = optional(list(string), ["OPENED", "CLOSED"])
}), {})
Breaking Change

Changing the default duration from 0 to 120 seconds is a breaking change that will affect existing deployments. Users who relied on the previous default (immediate alerting) will now experience a 2-minute delay before alerts trigger. This should be clearly documented in migration notes.

duration           = optional(number, 120)

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates monitoring alert configurations for LiteLLM and Typesense services by adding notification_prompts parameters and adjusting the default duration values for pod restart alerts from 0 to 120 seconds. However, it also inadvertently removes the alignment_period field from Typesense while retaining it in LiteLLM, creating an inconsistency.

Key Changes:

  • Added notification_prompts parameter with default value ["OPENED", "CLOSED"] for both LiteLLM and Typesense pod restart alerts
  • Changed default duration from 0 to 120 seconds for pod restart monitoring in both services
  • Removed alignment_period from Typesense configuration (but not from LiteLLM)
  • Removed inline comments explaining configuration sections

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
variables.tf Updated typesense and litellm variable definitions to add notification_prompts parameter and adjust duration default; removed alignment_period from Typesense
typesense.tf Added notification_prompts field to alert strategy configuration
lite_llm.tf Added notification_prompts field to alert strategy configuration
README.md Updated documentation to reflect new parameters and default values
CHANGELOG.md Documented changes in version 0.9.0 release notes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@FabrizioCafolla
Copy link
Copy Markdown
Member Author

/improve

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@FabrizioCafolla
Copy link
Copy Markdown
Member Author

/improve

@FabrizioCafolla
Copy link
Copy Markdown
Member Author

/improve

@sparkfabrik-ai-bot
Copy link
Copy Markdown

sparkfabrik-ai-bot bot commented Dec 15, 2025

PR Code Suggestions ✨

Latest suggestions up to 07553ad

CategorySuggestion                                                                                                                                    Impact
General
Align default notification_prompts value

The default value for notification_prompts is inconsistent between the variable
definition (null) and the README documentation (["OPENED", "CLOSED"]). This mismatch
can cause confusion and unexpected behavior. Align the default value with the
documented behavior.

variables.tf [127]

-notification_prompts = optional(list(string), null)
+notification_prompts = optional(list(string), ["OPENED", "CLOSED"])
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies an inconsistency between the variable definition (default null) and the README documentation (default ["OPENED", "CLOSED"]). This discrepancy could lead to confusion and unexpected behavior. However, the suggestion assumes the README is correct without verifying the intended behavior, and changing defaults could affect existing users.

Medium

Previous suggestions

Suggestions up to commit 64ea4f8
CategorySuggestion                                                                                                                                    Impact
General
Inconsistent alignment_period parameter handling

The alignment_period parameter was removed from the Typesense pod_restart
configuration but remains in the LiteLLM configuration. This inconsistency could
lead to unexpected behavior or configuration drift between similar monitoring
setups. Consider either removing it from LiteLLM or adding it back to Typesense with
a consistent default value.

variables.tf [122-127]

 pod_restart = optional(object({
   threshold          = optional(number, 0)
+  alignment_period   = optional(number, 60)
   duration           = optional(number, 120)
   auto_close_seconds = optional(number, 3600)
   notification_prompts = optional(list(string), ["OPENED", "CLOSED"])
 }), {})
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies an inconsistency where alignment_period was removed from Typesense but kept in LiteLLM's pod_restart configuration. This could lead to configuration drift and unexpected behavior between similar monitoring setups, making it a valid concern for consistency.

Medium

Copy link
Copy Markdown
Contributor

@filippolmt filippolmt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@FabrizioCafolla FabrizioCafolla merged commit dce5a25 into main Dec 15, 2025
1 check passed
@FabrizioCafolla FabrizioCafolla deleted the feat/change-params branch December 15, 2025 16:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

threshold = optional(number, 0)
alignment_period = optional(number, 60)
duration = optional(number, 0)
duration = optional(number, 180)
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR title and description mention changing the default duration from 0 to 120 seconds, but the actual code changes it to 180 seconds. This inconsistency should be corrected - either update the title/description to reflect 180 seconds, or change the code to use 120 seconds as stated.

Suggested change
duration = optional(number, 180)
duration = optional(number, 120)

Copilot uses AI. Check for mistakes.
threshold = optional(number, 0)
alignment_period = optional(number, 60)
duration = optional(number, 0)
duration = optional(number, 180)
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR title and description mention changing the default duration from 0 to 120 seconds, but the actual code changes it to 180 seconds. This inconsistency should be corrected - either update the title/description to reflect 180 seconds, or change the code to use 120 seconds as stated.

Suggested change
duration = optional(number, 180)
duration = optional(number, 120)

Copilot uses AI. Check for mistakes.
duration = optional(number, 0)
duration = optional(number, 180)
auto_close_seconds = optional(number, 3600)
notification_prompts = optional(list(string), null)
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default value for notification_prompts in the README is documented as ["OPENED", "CLOSED"], but in the variables.tf file it's defined as null. These should be consistent. If the default should be ["OPENED", "CLOSED"], update the variable definition. If it should be null, update the README documentation.

Suggested change
notification_prompts = optional(list(string), null)
notification_prompts = optional(list(string), ["OPENED", "CLOSED"])

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants