Add InfluxDB timeseries storage for system metrics#24
Add InfluxDB timeseries storage for system metrics#24jbingham17 wants to merge 2 commits intomainfrom
Conversation
Adds a --debug CLI flag that enables timestamped debug logging for request handling, CPU/memory collection, and process fetching. Includes server:debug and start:debug npm scripts for convenience. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Write CPU, memory, load average, and process count data points to InfluxDB on each metrics collection. Add /api/history endpoint for querying historical data. Gracefully disabled when INFLUX_TOKEN is not set. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis change adds InfluxDB integration to the metrics server, enabling metrics persistence and historical querying. It introduces a new /api/history endpoint, debug mode with command-line flag support, and environment-driven configuration for InfluxDB connectivity. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Server
participant InfluxDB
rect rgba(0, 100, 200, 0.5)
Note over Server: Startup
Server->>InfluxDB: initInfluxDB() - create client
InfluxDB-->>Server: connected (if INFLUX_TOKEN set)
end
rect rgba(0, 150, 100, 0.5)
Note over Server: Metric Collection & Write
Client->>Server: GET /api/metrics
Server->>Server: collect CPU, memory, load metrics
Server->>InfluxDB: writeMetrics(aggregated data)
InfluxDB-->>Server: ack
Server-->>Client: metrics JSON
end
rect rgba(150, 100, 0, 0.5)
Note over Server: Historical Query
Client->>Server: GET /api/history?measurement=cpu&range=-1h
Server->>InfluxDB: queryMetrics(measurement, range, field)
InfluxDB-->>Server: time-value pairs
Server-->>Client: historical data JSON
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
server/influxdb.ts (3)
147-151: Consider resetting module state incloseInfluxDB.When closing the InfluxDB connection,
queryApiandenabledare not reset. If the module is reinitialized after close, stale state could cause issues. Additionally,closeInfluxDBshould handle errors gracefully.♻️ Proposed improvement
export async function closeInfluxDB(): Promise<void> { if (writeApi) { - await writeApi.close(); + try { + await writeApi.close(); + } finally { + writeApi = null; + queryApi = null; + enabled = false; + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/influxdb.ts` around lines 147 - 151, In closeInfluxDB, after awaiting writeApi.close(), also null out or reset module-level state variables queryApi and writeApi and set enabled to false so the module can be safely reinitialized; wrap the close logic in a try/catch to log or handle errors (without throwing) so cleanup doesn't crash the process and ensure the catch clears state as well (referencing closeInfluxDB, writeApi, queryApi, and enabled).
35-43: Consider extracting the metrics type tosrc/types.ts.The inline type definition for the
metricsparameter duplicates the structure already partially defined insrc/types.ts(which hasCpuUsageandSystemMetrics). Extracting a shared type would improve maintainability.Based on the relevant code snippet from
src/types.ts, theCpuUsageinterface already matches thecpuUsagearray element type here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/influxdb.ts` around lines 35 - 43, The writeMetrics function uses an inline metrics type that duplicates existing types; instead import CpuUsage and SystemMetrics (or a combined SystemMetrics-like interface) from src/types.ts and replace the inline parameter type in writeMetrics with the shared type (e.g., metrics: SystemMetrics or a new alias that combines CpuUsage array and the memory/load/process fields). Update the function signature in writeMetrics and add the appropriate import at the top so the function refers to the centralized types instead of the inline definition.
98-101: Non-blocking flush swallows errors silently.The
.catch()logs the error message but doesn't propagate the failure. While this prevents blocking the request, consider adding a counter or metric for write failures to aid debugging in production.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/influxdb.ts` around lines 98 - 101, The current non-blocking flush (writeApi.flush().catch(...)) only logs err.message and swallows failures; update the catch handler for writeApi.flush() to (1) log the full error (err) with context, and (2) increment or record a write-failure metric/counter (e.g., call metrics.influxWriteFailures.inc() or influxWriteFailures.increment()) so failures are observable in production; ensure the metric is imported/initialized (or created alongside existing metrics) and do not rethrow to keep flush non-blocking.server/index.ts (1)
22-23: Consider logging the initialization result.
initInfluxDB()returns a boolean indicating success, but the return value is ignored. While the function logs its own status, capturing and using the result could be useful for conditional startup behavior or diagnostics.♻️ Suggested improvement
// Initialize InfluxDB timeseries storage -initInfluxDB(debug); +const influxInitialized = initInfluxDB(debug); +if (DEBUG) { + debug("InfluxDB initialization:", influxInitialized ? "success" : "skipped/failed"); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/index.ts` around lines 22 - 23, The call to initInfluxDB(debug) ignores its boolean return; capture its result (e.g., const influxInitialized = initInfluxDB(debug)) and log or act on it so startup can respond to failure—use the returned value to emit a clear processLogger/info or error message and optionally adjust startup flow (e.g., exit or continue degraded) based on influxInitialized; reference the initInfluxDB symbol where the call is made and update nearby startup logic to handle the boolean result.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/influxdb.ts`:
- Around line 113-120: The Flux query is vulnerable because measurement, range,
and field are directly interpolated into the query string; validate and sanitize
these inputs before building the query (e.g., enforce a strict whitelist/regex
for measurement and field like /^[A-Za-z0-9_.-]+$/ or map them against known
allowed names, and validate range is a safe numeric/ISO value or convert it
server-side rather than accepting raw strings). Update the code that constructs
the query (the template using INFLUX_BUCKET, measurement, field, range) to
reject or sanitize any values failing the checks and only use validated values
when interpolating to eliminate possible Flux injection.
---
Nitpick comments:
In `@server/index.ts`:
- Around line 22-23: The call to initInfluxDB(debug) ignores its boolean return;
capture its result (e.g., const influxInitialized = initInfluxDB(debug)) and log
or act on it so startup can respond to failure—use the returned value to emit a
clear processLogger/info or error message and optionally adjust startup flow
(e.g., exit or continue degraded) based on influxInitialized; reference the
initInfluxDB symbol where the call is made and update nearby startup logic to
handle the boolean result.
In `@server/influxdb.ts`:
- Around line 147-151: In closeInfluxDB, after awaiting writeApi.close(), also
null out or reset module-level state variables queryApi and writeApi and set
enabled to false so the module can be safely reinitialized; wrap the close logic
in a try/catch to log or handle errors (without throwing) so cleanup doesn't
crash the process and ensure the catch clears state as well (referencing
closeInfluxDB, writeApi, queryApi, and enabled).
- Around line 35-43: The writeMetrics function uses an inline metrics type that
duplicates existing types; instead import CpuUsage and SystemMetrics (or a
combined SystemMetrics-like interface) from src/types.ts and replace the inline
parameter type in writeMetrics with the shared type (e.g., metrics:
SystemMetrics or a new alias that combines CpuUsage array and the
memory/load/process fields). Update the function signature in writeMetrics and
add the appropriate import at the top so the function refers to the centralized
types instead of the inline definition.
- Around line 98-101: The current non-blocking flush
(writeApi.flush().catch(...)) only logs err.message and swallows failures;
update the catch handler for writeApi.flush() to (1) log the full error (err)
with context, and (2) increment or record a write-failure metric/counter (e.g.,
call metrics.influxWriteFailures.inc() or influxWriteFailures.increment()) so
failures are observable in production; ensure the metric is imported/initialized
(or created alongside existing metrics) and do not rethrow to keep flush
non-blocking.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9b1e1e2f-f926-436b-a396-eb805cee1593
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
package.jsonserver/index.tsserver/influxdb.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-02-18T05:07:00.671Z
Learnt from: jbingham17
Repo: coderabbit-demo/btop PR: 8
File: src/types.ts:32-32
Timestamp: 2026-02-18T05:07:00.671Z
Learning: In the coderabbit-demo/btop repository, avoid inline comments on TypeScript interface properties. Ensure property types are self-documenting through clear names and interface structure. If a property requires explanation, document it above the interface or with JSDoc comments for the property, not inline on the same line.
Applied to files:
server/influxdb.tsserver/index.ts
🪛 Biome (2.4.4)
server/influxdb.ts
[error] 23-23: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
(lint/correctness/useHookAtTopLevel)
🔇 Additional comments (5)
package.json (2)
9-11: LGTM!The debug scripts are correctly configured and mirror the non-debug counterparts with the
--debugflag appended.
17-17: No action required—version 1.35.0 is valid and has no known direct vulnerabilities.Version
1.35.0of@influxdata/influxdb-clientis a legitimate stable release with no reported CVEs or published security advisories. The caret constraint (^1.35.0) will permit patch and minor version updates if newer stable releases become available.server/index.ts (3)
8-20: LGTM!The debug mode implementation is clean and well-structured. The timestamp formatting provides useful context for debugging, and the flag parsing is straightforward.
309-333: The/api/historyendpoint structure looks good.The endpoint correctly:
- Returns 503 when InfluxDB is not configured
- Provides sensible defaults for query parameters
- Handles errors with appropriate status codes and logging
Note: The input validation concern for
measurement,range, andfieldparameters is addressed in theserver/influxdb.tsreview comment regarding Flux injection.
297-299: LGTM!The
writeMetricscall is appropriately placed after collecting metrics and before returning the response. SincewriteMetricshandles the disabled state internally and uses non-blocking flush, this won't impact response latency.
| const query = ` | ||
| from(bucket: "${INFLUX_BUCKET}") | ||
| |> range(start: ${range}) | ||
| |> filter(fn: (r) => r._measurement == "${measurement}") | ||
| |> filter(fn: (r) => r._field == "${field}") | ||
| |> aggregateWindow(every: 10s, fn: mean, createEmpty: false) | ||
| |> yield(name: "mean") | ||
| `; |
There was a problem hiding this comment.
Critical: Flux query injection vulnerability.
The measurement, range, and field parameters are directly interpolated into the Flux query string without sanitization. A malicious user could inject arbitrary Flux code via the /api/history endpoint query parameters.
For example, a request like /api/history?measurement=cpu") |> drop() |> from(bucket: "other could manipulate the query logic.
🔒 Proposed fix: Validate and sanitize inputs
+// Allowed measurements and fields for safe querying
+const ALLOWED_MEASUREMENTS = ["cpu", "cpu_core", "memory", "load", "processes"];
+const ALLOWED_FIELDS = ["usage_percent", "user_percent", "system_percent", "idle_percent",
+ "total_bytes", "used_bytes", "free_bytes", "avg_1m", "avg_5m", "avg_15m", "count"];
+const RANGE_PATTERN = /^-\d+[smhdw]$/; // e.g., -1h, -30m, -7d
+
export async function queryMetrics(
measurement: string,
range: string = "-1h",
field: string = "usage_percent"
): Promise<{ time: string; value: number }[]> {
if (!enabled || !queryApi) {
return [];
}
+ // Validate inputs to prevent Flux injection
+ if (!ALLOWED_MEASUREMENTS.includes(measurement)) {
+ throw new Error(`Invalid measurement: ${measurement}`);
+ }
+ if (!ALLOWED_FIELDS.includes(field)) {
+ throw new Error(`Invalid field: ${field}`);
+ }
+ if (!RANGE_PATTERN.test(range)) {
+ throw new Error(`Invalid range format: ${range}`);
+ }
+
const query = `
from(bucket: "${INFLUX_BUCKET}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/influxdb.ts` around lines 113 - 120, The Flux query is vulnerable
because measurement, range, and field are directly interpolated into the query
string; validate and sanitize these inputs before building the query (e.g.,
enforce a strict whitelist/regex for measurement and field like
/^[A-Za-z0-9_.-]+$/ or map them against known allowed names, and validate range
is a safe numeric/ISO value or convert it server-side rather than accepting raw
strings). Update the code that constructs the query (the template using
INFLUX_BUCKET, measurement, field, range) to reject or sanitize any values
failing the checks and only use validated values when interpolating to eliminate
possible Flux injection.
1 participant
Summary
/api/historyendpoint for querying historical metrics with configurable measurement, time range, and field parametersINFLUX_TOKENenv var is not set — existing functionality is unaffectedTest plan
INFLUX_TOKEN)INFLUX_TOKEN,INFLUX_URL,INFLUX_ORG,INFLUX_BUCKETand verify data is written to InfluxDB/api/history?measurement=cpu&range=-1hand verify timeseries data is returned/api/historyreturns 503 when InfluxDB is not configured🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
/api/historyendpoint enables querying historical metrics with customizable measurement, time range, and field parameters; requires InfluxDB configurationChores