Add charts for sites/functions usage #2517
Conversation
|
🚅 Deployed to the console-pr-2517 environment in Imagine
|
ConsoleProject ID: Tip Appwrite has a Discord community with over 16 000 members. |
WalkthroughThis pull request updates multiple usage dashboard pages across the console to shift metrics focus from deployments to executions, introduce GB-hours calculations derived from build and execution MB-seconds data, and add bandwidth visualization with bar charts. Empty state handling displays when data is absent. The common Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/layout/usage.svelte (2)
43-47: TS type mismatch: parameter defaults to null but typed as Date.This won’t type-check under strict TS.
-export function accumulateFromEndingTotal( - metrics: Models.Metric[], - endingTotal: number, - startingDayToFillZero: Date = null -): Array<[string, number]> { +export function accumulateFromEndingTotal( + metrics: Models.Metric[], + endingTotal: number, + startingDayToFillZero: Date | null = null +): Array<[string, number]> {Also, strongly type the accumulator’s data array:
- { - total: endingTotal, - data: [] - } + { + total: endingTotal, + data: [] as Array<[string, number]> + }
5-32: Fix UTC/local mixing in periodToDates function—confirmed bugs at lines 11, 16, 19.The code mixes local timezone getters (
getHours(),getDate()) with UTC setters (setUTCHours(),setUTCDate()), causing incorrect date calculations:
- Line 11:
start.setUTCHours(start.getHours() - 24)— getHours() is local, not UTC- Line 16:
start.setUTCDate(start.getDate() - 30)— getDate() is local, not UTC- Line 19: Same issue for 90d case
Use
getUTC*()methods ornew Date(now.getTime() - offset)arithmetic instead.Regarding the 24h end boundary: the review suggests
end=nowinstead ofend=next_UTC_midnight. This change requires verification against the API contract forproject.getUsage()to ensure it accepts aligned vs. real-time boundaries for theOneHourperiod type.export function periodToDates(period: UsagePeriods): { start: string; end: string; period: ProjectUsageRange; } { - const start = new Date(); - switch (period) { - case '24h': - start.setUTCHours(start.getHours() - 24); - break; - case '30d': - start.setUTCHours(0, 0, 0, 0); - start.setUTCDate(start.getDate() - 30); - break; - case '90d': - start.setUTCHours(0, 0, 0, 0); - start.setUTCDate(start.getDate() - 90); - break; - } - const end = new Date(); - end.setUTCDate(end.getUTCDate() + 1); - end.setUTCHours(0, 0, 0, 0); - return { - start: start.toISOString(), - end: end.toISOString(), - period: period === '24h' ? ProjectUsageRange.OneHour : ProjectUsageRange.OneDay - }; + const now = new Date(); + if (period === '24h') { + const start = new Date(now.getTime() - 24 * 60 * 60 * 1000); + return { + start: start.toISOString(), + end: now.toISOString(), + period: ProjectUsageRange.OneHour + }; + } + // daily buckets + const end = new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate() + 1, 0, 0, 0, 0)); + const start = new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate(), 0, 0, 0, 0)); + start.setUTCDate(start.getUTCDate() - (period === '30d' ? 30 : 90)); + return { + start: start.toISOString(), + end: end.toISOString(), + period: ProjectUsageRange.OneDay + }; }
🧹 Nitpick comments (8)
src/lib/layout/usage.svelte (2)
34-41: Align helper signatures with nullish usage.Both helpers guard against undefined/null but are typed as non-null arrays.
-export function last(set: Models.Metric[]): Models.Metric | null { - if (!set) return null; - return set.slice(-1)[0] ?? null; -} - -export function total(set: Models.Metric[]): number { - return set?.reduce((prev, curr) => prev + curr.value, 0) ?? 0; -} +export function last(set: Models.Metric[] | null | undefined): Models.Metric | null { + if (!set?.length) return null; + return set[set.length - 1]; +} + +export function total(set: Models.Metric[] | null | undefined): number { + return set?.reduce((prev, curr) => prev + curr.value, 0) ?? 0; +}
138-143: Empty state UX is clear and consistent.Icon + concise copy looks good. Consider adding aria-hidden on the icon if not already handled by the component.
src/routes/(console)/project-[region]-[project]/functions/usage/[[period]]/+page.svelte (2)
22-22: Avoid rendering a bandwidth card with permanent “No data”.
bandwidth = []renders an empty-state card always. Either wire real data or conditionally hide the card.-$: bandwidth = []; +$: bandwidth = data.bandwidth ?? [];Optionally gate the entire Card with
{#if bandwidth.length}.
1-1: Prettier is failing CI.Run repository formatter to unblock CI:
- pnpm:
pnpm exec prettier --write .- npm:
npx prettier --write .[pipeline_failures]
src/routes/(console)/project-[region]-[project]/sites/usage/[[period]]/+page.svelte (1)
1-1: Prettier is failing CI.Run:
pnpm exec prettier --write .(ornpx prettier --write .).
[pipeline_failures]src/routes/(console)/project-[region]-[project]/sites/site-[site]/usage/[[period]]/+page.svelte (1)
1-1: Prettier is failing CI.Run:
pnpm exec prettier --write ..
[pipeline_failures]src/routes/(console)/project-[region]-[project]/functions/function-[function]/usage/[[period]]/+page.svelte (2)
21-21: Bandwidth card renders permanent empty state.Wire real data or hide the card when unavailable.
-$: bandwidth = []; +$: bandwidth = data.bandwidth ?? [];Optionally wrap the Card in
{#if bandwidth.length}to avoid UI noise.
1-1: Prettier is failing CI.Run:
pnpm exec prettier --write ..
[pipeline_failures]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/lib/layout/usage.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/usage/[[period]]/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/functions/usage/[[period]]/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/usage/[[period]]/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/sites/usage/[[period]]/+page.svelte(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Tests
src/routes/(console)/project-[region]-[project]/functions/usage/[[period]]/+page.svelte
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues in this file.
src/routes/(console)/project-[region]-[project]/functions/function-[function]/usage/[[period]]/+page.svelte
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues in this file.
src/routes/(console)/project-[region]-[project]/sites/usage/[[period]]/+page.svelte
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues in this file.
src/routes/(console)/project-[region]-[project]/sites/site-[site]/usage/[[period]]/+page.svelte
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues in this file.
🔇 Additional comments (4)
src/lib/layout/usage.svelte (1)
121-125: Nice horizontal alignment for headline/label.Readability improves with row layout and baseline alignment.
src/routes/(console)/project-[region]-[project]/sites/usage/[[period]]/+page.svelte (1)
15-22: Verify data structures and intended aggregation behavior before applying fix.The review raises a valid structural concern: if
buildsMbSecondsandexecutionsMbSecondscontain entries for the same dates, concatenating them would duplicate those dates. However, verification depends on the actual data:
- Overlapping dates: Do both arrays contain entries for the same dates? (If builds and executions are tracked per-date, they likely do.)
- Intended aggregation: Should the chart display merged totals per date, or separate builds/executions series? The
Usagecomponent receives a singlecountarray, suggesting merge intent.- MB-to-GB conversion: The constant
1000is correct per SI standard;1024(binary/IEC) is not standard for network/storage metrics.The same concatenation pattern appears in:
src/routes/(console)/project-[region]-[project]/sites/usage/[[period]]/+page.svelte(line 16)src/routes/(console)/project-[region]-[project]/functions/usage/[[period]]/+page.svelte(line 16)src/routes/(console)/project-[region]-[project]/functions/function-[function]/usage/[[period]]/+page.svelte(line 16)src/routes/(console)/project-[region]-[project]/sites/site-[site]/usage/[[period]]/+page.svelte(line 16)And for bandwidth:
- Line 23:
[...data.inbound, ...data.outbound]uses the same pattern.Confirm whether these should be merged per date or if separate series are intentional.
src/routes/(console)/project-[region]-[project]/functions/function-[function]/usage/[[period]]/+page.svelte (1)
15-21: Add missing.filter()call; verify date-merging requirement with API response structure.The code snippet in the review is incomplete. The target file is missing
.filter(({ value }) => value);that appears in analogous usage pages (/functions/usage,/sites/usage,/sites/site-[site]/usage). This filter should be added after the map on line 20.Regarding the suggested
mergeByDatelogic: this assumes overlapping dates exist in bothdata.buildsMbSecondsanddata.executionsMbSecondsarrays. Verify with the API response structure whether the two arrays can share the same dates before implementing aggregation. If they do, the merge-by-date approach is valid; otherwise, the current spread pattern may be sufficient.Constants
MB_PER_GB(1000) andSECONDS_PER_HOUR(3600) are correct.src/routes/(console)/project-[region]-[project]/sites/site-[site]/usage/[[period]]/+page.svelte (1)
15-22: Same GB-hours aggregation issue: merge per date + sort before scaling.-$: gbHoursTotal = (data.buildsMbSecondsTotal + data.executionsMbSecondsTotal) / 1000 / 3600; -$: gbHoursCount = [...data.buildsMbSeconds, ...data.executionsMbSeconds] - ?.map((metric) => ({ - ...metric, - value: metric.value / 1000 / 3600 - })) - .filter(({ value }) => value); +const mergeByDate = (...sets: { date: string; value: number }[][]) => { + const acc = new Map<string, number>(); + for (const s of sets) for (const m of s ?? []) acc.set(m.date, (acc.get(m.date) ?? 0) + m.value); + return [...acc.entries()] + .map(([date, value]) => ({ date, value })) + .sort((a, b) => Date.parse(a.date) - Date.parse(b.date)); +}; +const MB_PER_GB = 1000; // verify 1000 vs 1024 +const SECONDS_PER_HOUR = 3600; +$: gbHoursTotal = (data.buildsMbSecondsTotal + data.executionsMbSecondsTotal) / MB_PER_GB / SECONDS_PER_HOUR; +$: gbHoursCount = mergeByDate(data.buildsMbSeconds, data.executionsMbSeconds).map(({ date, value }) => ({ + date, + value: value / MB_PER_GB / SECONDS_PER_HOUR +}));
Task: https://linear.app/appwrite/issue/SER-476/add-charts-for-sitesfunctions-usage
Summary by CodeRabbit
Release Notes
New Features
Improvements