Skip to content

ci(oss-health): re-enable monthly telemetry cron (after telemetry-server#8 deploy) #585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Timur Tukaev (tym83) wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

ci(oss-health): re-enable monthly telemetry cron (after telemetry-server#8 deploy) #585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions .github/workflows/fetch-telemetry.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
name: Fetch Telemetry Data

on:
# Keep this as a manual telemetry-only backfill path. The monthly OSS Health
# refresh intentionally does not update telemetry while /api/overview differs
# from the Grafana-backed source of truth.
# Monthly telemetry refresh. /api/overview now counts entities over the period
# window instead of at an instant (cozystack/cozystack-telemetry-server#8), so
# it matches the Grafana-backed source of truth and is safe to consume on a
# schedule again. Aligned with the monthly OSS Health cron; workflow_dispatch
# remains for manual backfills.
schedule:
- cron: '0 4 1 * *'
workflow_dispatch:

permissions:
Expand Down
9 changes: 6 additions & 3 deletions hack/fetch_telemetry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
3. Merge case-insensitive / Pax* / legacy-name aliases into one canonical entry
per application, keeping the maximum instance count (zero-count entries
left after the merge are dropped from the table).
4. Pull `Tenant` out of the apps map and surface it as the top-level Tenants
summary card (the raw `total_tenants` field from the API is always zero).
4. Surface tenant count as the top-level Tenants summary card, preferring the
period's `total_tenants` field (correct since telemetry-server #8) and
falling back to `apps.Tenant` for older payloads where it was zero.
5. Emit the payload in the shape consumed by `oss-health-app.html` +
`renderTelemetry`, including `summary_cards`, `apps`, `range`.

Expand Down Expand Up @@ -100,7 +101,9 @@ def transform_period(raw_period: dict, label_fallback: str) -> dict | None:
if not raw_period:
return None
apps_raw = raw_period.get("apps", {}) or {}
tenants = int(apps_raw.get("Tenant", 0))
# Prefer the period's own total_tenants (correct since telemetry-server #8);
# fall back to apps.Tenant for older payloads where total_tenants was 0.
tenants = int(raw_period.get("total_tenants") or apps_raw.get("Tenant", 0))

@gemini-code-assist gemini-code-assist Bot Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If the API response contains an explicit null value for Tenant (e.g., {"apps": {"Tenant": null}}), apps_raw.get("Tenant", 0) will return None instead of the default 0 because the key exists. This would cause int(None) to raise a TypeError and crash the script. Using apps_raw.get("Tenant") or 0 ensures that if the value is None or the key is missing, it safely falls back to 0.

Suggested change
tenants = int(raw_period.get("total_tenants") or apps_raw.get("Tenant", 0))
tenants = int(raw_period.get("total_tenants") or apps_raw.get("Tenant") or 0)

clusters = int(raw_period.get("clusters", 0))
total_nodes = int(raw_period.get("total_nodes", 0))
avg_nodes = raw_period.get("avg_nodes_per_cluster")
Expand Down