Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions api/generated-schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -1401,6 +1401,17 @@ type CpuUtilization implements Node {
cpus: [CpuLoad!]!
}

type CpuPackages implements Node {
"""Total CPU package power draw (W)"""
totalPower: Float!

"""Power draw per package (W)"""
power: [Float!]

"""description: 'Temperature per package (°C)"""
temp: [Float!]
}

type InfoCpu implements Node {
id: PrefixedID!

Expand Down Expand Up @@ -1446,6 +1457,9 @@ type InfoCpu implements Node {
"""Number of physical processors"""
processors: Int

"""CPU packages information"""
packages: CpuPackages

"""CPU socket type"""
socket: String

Expand Down Expand Up @@ -2642,6 +2656,7 @@ type Subscription {
arraySubscription: UnraidArray!
logFile(path: String!): LogFileContent!
systemMetricsCpu: CpuUtilization!
systemMetricsCpuTelemetry: CpuPackages!
systemMetricsMemory: MemoryUtilization!
upsUpdates: UPSDevice!
}
211 changes: 211 additions & 0 deletions api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,211 @@
import { Injectable, Logger } from '@nestjs/common';
import { constants as fsConstants } from 'node:fs';
import { access, readdir, readFile } from 'node:fs/promises';
import { join } from 'path';

@Injectable()
export class CpuTopologyService {
private readonly logger = new Logger(CpuTopologyService.name);

private topologyCache: { id: number; cores: number[][] }[] | null = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Remove unused field.

The topologyCache field is declared but never used anywhere in this service.

-    private topologyCache: { id: number; cores: number[][] }[] | null = null;
-
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private topologyCache: { id: number; cores: number[][] }[] | null = null;
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts around
line 12, the private field `topologyCache` is declared but never referenced;
remove this unused field declaration to clean up the class (and run the
linter/TS compiler to ensure no other references exist), or if it was intended
to be used, implement its usage instead of leaving it unused.


// -----------------------------------------------------------------
// Read static CPU topology, per-package core thread pairs
// -----------------------------------------------------------------
async generateTopology(): Promise<number[][][]> {
const packages: Record<number, number[][]> = {};
const cpuDirs = await readdir('/sys/devices/system/cpu');

for (const dir of cpuDirs) {
if (!/^cpu\d+$/.test(dir)) continue;

const basePath = join('/sys/devices/system/cpu', dir, 'topology');
const pkgFile = join(basePath, 'physical_package_id');
const siblingsFile = join(basePath, 'thread_siblings_list');

try {
const [pkgIdStr, siblingsStrRaw] = await Promise.all([
readFile(pkgFile, 'utf8'),
readFile(siblingsFile, 'utf8'),
]);

const pkgId = parseInt(pkgIdStr.trim(), 10);

// expand ranges
const siblings = siblingsStrRaw
.trim()
.replace(/(\d+)-(\d+)/g, (_, start, end) =>
Array.from(
{ length: parseInt(end) - parseInt(start) + 1 },
(_, i) => parseInt(start) + i
).join(',')
)
.split(',')
.map((n) => parseInt(n, 10));

if (!packages[pkgId]) packages[pkgId] = [];
if (!packages[pkgId].some((arr) => arr.join(',') === siblings.join(','))) {
packages[pkgId].push(siblings);
}
} catch (err) {
console.warn('Topology read error for', dir, err);
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Use consistent logging: prefer this.logger.warn.

The rest of the file uses this.logger.warn for consistency. Replace console.warn with the injected logger.

-                console.warn('Topology read error for', dir, err);
+                this.logger.warn('Topology read error for', dir, err);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.warn('Topology read error for', dir, err);
this.logger.warn('Topology read error for', dir, err);
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts around
line 53, replace the console.warn call with the injected logger to maintain
consistency: change console.warn('Topology read error for', dir, err) to use
this.logger.warn, passing a clear message and the dir and error (e.g.
this.logger.warn('Topology read error for %s', dir, err) or equivalent
structured logging) so the same logging instance and format are used throughout
the file.

}
}
// Sort cores within each package, and packages by their lowest core index
const result = Object.entries(packages)
.sort((a, b) => a[1][0][0] - b[1][0][0]) // sort packages by first CPU ID
.map(
([pkgId, cores]) => cores.sort((a, b) => a[0] - b[0]) // sort cores within package
);

return result;
}

// -----------------------------------------------------------------
// Dynamic telemetry (power + temperature)
// -----------------------------------------------------------------
private async getPackageTemps(): Promise<number[]> {
const temps: number[] = [];
try {
const hwmons = await readdir('/sys/class/hwmon');
for (const hwmon of hwmons) {
const path = join('/sys/class/hwmon', hwmon);
try {
const label = (await readFile(join(path, 'name'), 'utf8')).trim();
if (/coretemp|k10temp|zenpower/i.test(label)) {
const files = await readdir(path);
for (const f of files) {
if (f.startsWith('temp') && f.endsWith('_label')) {
const lbl = (await readFile(join(path, f), 'utf8')).trim().toLowerCase();
if (
lbl.includes('package id') ||
lbl.includes('tctl') ||
lbl.includes('tdie')
) {
const inputFile = join(path, f.replace('_label', '_input'));
try {
const raw = await readFile(inputFile, 'utf8');
temps.push(parseInt(raw.trim(), 10) / 1000);
} catch (err) {
this.logger.warn('Failed to read file', err);
}
}
}
}
}
} catch (err) {
this.logger.warn('Failed to read file', err);
}
}
} catch (err) {
this.logger.warn('Failed to read file', err);
}
return temps;
}

private async getPackagePower(): Promise<Record<number, Record<string, number>>> {
const basePath = '/sys/class/powercap';
const prefixes = ['intel-rapl', 'intel-rapl-mmio', 'amd-rapl'];
const raplPaths: string[] = [];

try {
const entries = await readdir(basePath, { withFileTypes: true });
for (const entry of entries) {
if (entry.isSymbolicLink() && prefixes.some((p) => entry.name.startsWith(p))) {
if (/:\d+:\d+/.test(entry.name)) continue;
raplPaths.push(join(basePath, entry.name));
}
}
} catch {
return {};
}

if (!raplPaths.length) return {};

const readEnergy = async (p: string): Promise<number | null> => {
try {
await access(join(p, 'energy_uj'), fsConstants.R_OK);
const raw = await readFile(join(p, 'energy_uj'), 'utf8');
return parseInt(raw.trim(), 10);
} catch {
return null;
}
};

const prevE = new Map<string, number>();
const prevT = new Map<string, bigint>();

for (const p of raplPaths) {
const val = await readEnergy(p);
if (val !== null) {
prevE.set(p, val);
prevT.set(p, process.hrtime.bigint());
}
}

await new Promise((res) => setTimeout(res, 100));

const results: Record<number, Record<string, number>> = {};

for (const p of raplPaths) {
const now = await readEnergy(p);
if (now === null) continue;

const prevVal = prevE.get(p);
const prevTime = prevT.get(p);
if (prevVal === undefined || prevTime === undefined) continue;

const diffE = now - prevVal;
const diffT = Number(process.hrtime.bigint() - prevTime);
if (diffT <= 0 || diffE < 0) continue;

const watts = (diffE * 1e-6) / (diffT * 1e-9);
const powerW = Math.round(watts * 100) / 100;

const nameFile = join(p, 'name');
let label = 'package';
try {
label = (await readFile(nameFile, 'utf8')).trim();
} catch (err) {
this.logger.warn('Failed to read file', err);
}

const pkgMatch = label.match(/package-(\d+)/i);
const pkgId = pkgMatch ? Number(pkgMatch[1]) : 0;

if (!results[pkgId]) results[pkgId] = {};
results[pkgId][label] = powerW;
}

for (const domains of Object.values(results)) {
const total = Object.values(domains).reduce((a, b) => a + b, 0);
(domains as any)['total'] = Math.round(total * 100) / 100;
}
Comment on lines +180 to +183
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid any type: add proper typing for domains.

Line 184 uses (domains as any)['total'] which violates the coding guideline: "Never use the any type; prefer precise typing."

The domains object should be properly typed to allow the total property:

-        for (const domains of Object.values(results)) {
+        for (const domains of Object.values(results) as Record<string, number>[]) {
             const total = Object.values(domains).reduce((a, b) => a + b, 0);
-            (domains as any)['total'] = Math.round(total * 100) / 100;
+            domains['total'] = Math.round(total * 100) / 100;
         }

Based on coding guidelines.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for (const domains of Object.values(results)) {
const total = Object.values(domains).reduce((a, b) => a + b, 0);
(domains as any)['total'] = Math.round(total * 100) / 100;
}
for (const domains of Object.values(results) as Record<string, number>[]) {
const total = Object.values(domains).reduce((a, b) => a + b, 0);
domains['total'] = Math.round(total * 100) / 100;
}
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.ts around
lines 182 to 185, the code uses (domains as any)['total'] which violates the
no-any rule; instead give domains a precise type (for example declare a
DomainMap type = Record<string, number> & { total?: number } or an appropriate
interface) and ensure results is typed so Object.values(results) yields
DomainMap[], then set domains.total = Math.round(total * 100) / 100 without any
casting; update function signatures/variable declarations so TypeScript infers
the correct DomainMap type everywhere.


return results;
}

async generateTelemetry(): Promise<{ id: number; power: number; temp: number }[]> {
const temps = await this.getPackageTemps();
const powerData = await this.getPackagePower();

const maxPkg = Math.max(temps.length - 1, ...Object.keys(powerData).map(Number), 0);

const result: {
id: number;
power: number;
temp: number;
}[] = [];

for (let pkgId = 0; pkgId <= maxPkg; pkgId++) {
const entry = powerData[pkgId] ?? {};
result.push({
id: pkgId,
power: entry.total ?? -1,
temp: temps[pkgId] ?? -1,
});
}

return result;
}
}
20 changes: 20 additions & 0 deletions api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ export class CpuLoad {
percentSteal!: number;
}

@ObjectType()
export class CpuPackages {
@Field(() => Float, { description: 'Total CPU package power draw (W)' })
totalPower?: number;

@Field(() => [Float], { description: 'Power draw per package (W)' })
power?: number[];

@Field(() => [Float], { description: 'Temperature per package (°C)' })
temp?: number[];
}
Comment on lines +42 to +52
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Make CpuPackages a Node type and align nullability/casing.

Implement Node and make fields definite to match GraphQL expectations; we already return these arrays non-null.

-@ObjectType()
-export class CpuPackages {
-    @Field(() => Float, { description: 'Total CPU package power draw (W)' })
-    totalPower?: number;
-    @Field(() => [Float], { description: 'Power draw per package (W)' })
-    power?: number[];
-    @Field(() => [Float], { description: 'Temperature per package (°C)' })
-    temp?: number[];
-}
+@ObjectType({ implements: () => Node })
+export class CpuPackages extends Node {
+    @Field(() => Float, { description: 'Total CPU package power draw (W)' })
+    totalPower!: number;
+    @Field(() => [Float], { description: 'Power draw per package (W)' })
+    power!: number[];
+    @Field(() => [Float], { description: 'Temperature per package (°C)' })
+    temp!: number[];
+}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/info/cpu/cpu.model.ts around lines 42 to
52, change CpuPackages to implement the Node GraphQL type and make all fields
non-optional/non-nullable to match what we actually return: add the Node id
field, mark the class to implement Node (e.g., @ObjectType({ implements: Node
})) and remove the TypeScript optional markers (?) from totalPower, power and
temp; update their @Field decorators to be explicitly non-nullable (set
nullable: false and ensure list fields are non-null lists/values as your GraphQL
decorator supports).


@ObjectType({ implements: () => Node })
export class CpuUtilization extends Node {
@Field(() => Float, { description: 'Total CPU load in percent' })
Expand Down Expand Up @@ -100,4 +112,12 @@ export class InfoCpu extends Node {

@Field(() => [String], { nullable: true, description: 'CPU feature flags' })
flags?: string[];

@Field(() => [[[Int]]], {
description: 'Per-package array of core/thread pairs, e.g. [[[0,1],[2,3]], [[4,5],[6,7]]]',
})
topology!: number[][][];

@Field(() => CpuPackages)
packages!: CpuPackages;
}
10 changes: 10 additions & 0 deletions api/src/unraid-api/graph/resolvers/info/cpu/cpu.module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { Module } from '@nestjs/common';

import { CpuTopologyService } from '@app/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.js';
import { CpuService } from '@app/unraid-api/graph/resolvers/info/cpu/cpu.service.js';

@Module({
providers: [CpuService, CpuTopologyService],
exports: [CpuService, CpuTopologyService],
})
export class CpuModule {}
36 changes: 35 additions & 1 deletion api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';

import { CpuTopologyService } from '@app/unraid-api/graph/resolvers/info/cpu/cpu-topology.service.js';
import { CpuService } from '@app/unraid-api/graph/resolvers/info/cpu/cpu.service.js';

vi.mock('systeminformation', () => ({
Expand Down Expand Up @@ -88,9 +89,27 @@ vi.mock('systeminformation', () => ({

describe('CpuService', () => {
let service: CpuService;
let cpuTopologyService: CpuTopologyService;

beforeEach(() => {
service = new CpuService();
cpuTopologyService = {
generateTopology: vi.fn().mockResolvedValue([
[
[0, 1],
[2, 3],
],
[
[4, 5],
[6, 7],
],
]),
generateTelemetry: vi.fn().mockResolvedValue([
{ power: 32.5, temp: 45.0 },
{ power: 33.0, temp: 46.0 },
]),
} as any;
Comment on lines +95 to +110
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Replace as any with proper typing.

The coding guideline states to never use the any type. Replace the type assertion with a more precise cast.

As per coding guidelines.

Apply this diff:

         cpuTopologyService = {
             generateTopology: vi.fn().mockResolvedValue([
                 [
                     [0, 1],
                     [2, 3],
                 ],
                 [
                     [4, 5],
                     [6, 7],
                 ],
             ]),
             generateTelemetry: vi.fn().mockResolvedValue([
                 { power: 32.5, temp: 45.0 },
                 { power: 33.0, temp: 46.0 },
             ]),
-        } as any;
+        } as unknown as CpuTopologyService;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cpuTopologyService = {
generateTopology: vi.fn().mockResolvedValue([
[
[0, 1],
[2, 3],
],
[
[4, 5],
[6, 7],
],
]),
generateTelemetry: vi.fn().mockResolvedValue([
{ power: 32.5, temp: 45.0 },
{ power: 33.0, temp: 46.0 },
]),
} as any;
cpuTopologyService = {
generateTopology: vi.fn().mockResolvedValue([
[
[0, 1],
[2, 3],
],
[
[4, 5],
[6, 7],
],
]),
generateTelemetry: vi.fn().mockResolvedValue([
{ power: 32.5, temp: 45.0 },
{ power: 33.0, temp: 46.0 },
]),
} as unknown as CpuTopologyService;
🤖 Prompt for AI Agents
In api/src/unraid-api/graph/resolvers/info/cpu/cpu.service.spec.ts around lines
95 to 110, the test stub uses an unsafe "as any" cast for cpuTopologyService;
replace it with a proper typed mock such as vi.Mocked<CpuTopologyService> (or
Partial<CpuTopologyService> if the full mocked type isn't available): import the
CpuTopologyService type from its module, change the assertion to "as
vi.Mocked<CpuTopologyService>", and ensure vitest's vi.Mocked is used so
generateTopology and generateTelemetry remain correctly typed as mocked
functions.


service = new CpuService(cpuTopologyService);
});

describe('generateCpu', () => {
Expand Down Expand Up @@ -121,6 +140,21 @@ describe('CpuService', () => {
l3: 12582912,
},
flags: ['fpu', 'vme', 'de', 'pse', 'tsc', 'msr', 'pae', 'mce', 'cx8'],
packages: {
totalPower: 65.5,
power: [32.5, 33.0],
temp: [45.0, 46.0],
},
topology: [
[
[0, 1],
[2, 3],
],
[
[4, 5],
[6, 7],
],
],
});
});

Expand Down
Loading
Loading