Skip to content

fix(noc): surface OpenRouter quota in model health check#220

Open
Svaag wants to merge 1 commit into
mainfrom
fix/noc-openrouter-quota-health
Open

fix(noc): surface OpenRouter quota in model health check#220
Svaag wants to merge 1 commit into
mainfrom
fix/noc-openrouter-quota-health

Conversation

@Svaag

@Svaag Svaag commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • include OpenRouter key/account credit fields in noc-agent model health check output
  • prefer management API account remaining credit for quota-left, falling back to per-key limit_remaining
  • add OpenRouter credit/usage perfdata for Icinga trending

Validation

  • sh -n configs/mon/icinga2/scripts/check_noc_agent_model_health.sh
  • local mock /health/model 503 response confirms account quota-left and perfdata are emitted

@Svaag Svaag requested a review from a team as a code owner June 14, 2026 16:20
@github-actions

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

Deploy risk

This is a monitoring script that reads from a noc-agent health endpoint and outputs Icinga perfdata. The risk is low because the script itself does not execute any deploy logic, checkout branches, or touch secrets. However, note that field() is used without definition in the visible diff — if field() is a custom shell function defined elsewhere in this file (not shown), any logic error in it would silently produce '?' values and degrade alerting accuracy. Verify that field() is defined in the full file and handles .provider_monitoring paths correctly.

or_status=$(field '.provider_monitoring.providers.openrouter.status // .quota.providers.openrouter.status')
or_key_status=$(field '.provider_monitoring.providers.openrouter.key.status // .quota.providers.openrouter.key.status')
or_key_msg=$(field '.provider_monitoring.providers.openrouter.key.message // .quota.providers.openrouter.key.message')
or_key_remaining=$(field '.provider_monitoring.providers.openrouter.key.limit_remaining // .quota.providers.openrouter.key.limit_remaining')
or_key_limit=$(field '.provider_monitoring.providers.openrouter.key.limit // .quota.providers.openrouter.key.limit')
or_key_usage=$(field '.provider_monitoring.providers.openrouter.key.usage // .quota.providers.openrouter.key.usage')
or_key_daily=$(field '.provider_monitoring.providers.openrouter.key.usage_daily // .quota.providers.openrouter.key.usage_daily')
or_key_monthly=$(field '.provider_monitoring.providers.openrouter.key.usage_monthly // .quota.providers.openrouter.key.usage_monthly')
or_account_status=$(field '.provider_monitoring.providers.openrouter.account.status // .quota.providers.openrouter.account.status')
or_account_msg=$(field '.provider_monitoring.providers.openrouter.account.message // .quota.providers.openrouter.account.message')
or_account_total=$(field '.provider_monitoring.providers.openrouter.account.total_credits // .quota.providers.openrouter.account.total_credits')
or_account_usage=$(field '.provider_monitoring.providers.openrouter.account.total_usage // .quota.providers.openrouter.account.total_usage')
or_account_remaining=$(field '.provider_monitoring.providers.openrouter.account.remaining // .quota.providers.openrouter.account.remaining')

@github-actions

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Improve numeric validation robustness

The is_number function uses grep -E which may fail if the input contains special
characters or newlines. Use a more robust pattern like [[ $1 =~
^-?[0-9]+(.[0-9]+)?$ ]] to avoid potential shell injection or parsing issues.

configs/mon/icinga2/scripts/check_noc_agent_model_health.sh [113-115]

 is_number() {
-    printf '%s' "$1" | grep -Eq '^-?[0-9]+([.][0-9]+)?$'
+    [[ $1 =~ ^-?[0-9]+(\.[0-9]+)?$ ]]
 }
Suggestion importance[1-10]: 6

__

Why: The suggestion replaces grep -E with a bash regex, which is more efficient and avoids potential issues with special characters. While the existing code is functional, the improvement is valid and enhances robustness. Score reflects moderate impact.

Low
Guard against empty detail string

The perfdata variable is built with spaces between metrics, but the final
concatenation with | may produce malformed output if detail is empty. Ensure detail
is non-empty before appending the pipe separator, or use a consistent format like
$detail | $perfdata only when both are non-empty.

configs/mon/icinga2/scripts/check_noc_agent_model_health.sh [131]

-[ -n "$perfdata" ] && detail="$detail | $perfdata"
+[ -n "$perfdata" ] && [ -n "$detail" ] && detail="$detail | $perfdata"
Suggestion importance[1-10]: 3

__

Why: The suggestion adds a guard for an empty detail string, but in the current PR code detail is always initialized (line 90), making this check unnecessary. The improvement is technically correct but has low practical impact.

Low
Possible issue
Ensure field command is defined

The field command is not defined in the script. It appears to be a custom function
or external tool. If it's not available, these assignments will fail silently.
Ensure field is defined or replace with jq -r to avoid runtime errors.

configs/mon/icinga2/scripts/check_noc_agent_model_health.sh [66]

-or_status=$(field '.provider_monitoring.providers.openrouter.status // .quota.providers.openrouter.status')
+or_status=$(jq -r '.provider_monitoring.providers.openrouter.status // .quota.providers.openrouter.status' "$body" 2>/dev/null || echo "?")
Suggestion importance[1-10]: 4

__

Why: The suggestion points out that field may not be defined, but the PR likely relies on a custom function defined elsewhere in the script. The proposed replacement with jq changes behavior and may not be appropriate. The suggestion is speculative and has moderate relevance.

Low

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.

1 participant