-
Notifications
You must be signed in to change notification settings - Fork 953
feat: add process info diagnostic for cpu, memory, and system metrics #10204
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,5 @@ | ||||||||||
| import { getBitVersion } from '@teambit/bit.get-bit-version'; | ||||||||||
| import os from 'os'; | ||||||||||
|
|
||||||||||
| import type { SlotRegistry } from '@teambit/harmony'; | ||||||||||
| import { Slot } from '@teambit/harmony'; | ||||||||||
|
|
@@ -45,13 +46,46 @@ export class DiagnosticMain { | |||||||||
| return { version }; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| static getProcessInfo() { | ||||||||||
| const memUsage = process.memoryUsage(); | ||||||||||
| const cpuUsage = process.cpuUsage(); | ||||||||||
| return { | ||||||||||
| uptime: process.uptime(), | ||||||||||
| pid: process.pid, | ||||||||||
| nodeVersion: process.version, | ||||||||||
| platform: process.platform, | ||||||||||
| arch: process.arch, | ||||||||||
| memory: { | ||||||||||
| rss: memUsage.rss, | ||||||||||
| heapTotal: memUsage.heapTotal, | ||||||||||
| heapUsed: memUsage.heapUsed, | ||||||||||
| external: memUsage.external, | ||||||||||
| arrayBuffers: memUsage.arrayBuffers, | ||||||||||
|
||||||||||
| arrayBuffers: memUsage.arrayBuffers, | |
| ...(typeof (memUsage as any).arrayBuffers === 'number' | |
| ? { arrayBuffers: (memUsage as any).arrayBuffers } | |
| : {}), |
Copilot
AI
Mar 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getProcessInfo() exposes CPU usage as the raw output of process.cpuUsage() (user/system in microseconds). Without units in the field names, this is easy to misinterpret as percentages or milliseconds. Consider either converting to a clearer unit (e.g. milliseconds) or renaming/annotating the fields to include the unit so downstream consumers don’t misread the numbers.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||
| import type { Route, Request, Response } from '@teambit/express'; | ||||||
| import type { Route, Request, Response, Middleware } from '@teambit/express'; | ||||||
| import { Verb } from '@teambit/express'; | ||||||
| import type { DiagnosticMain } from './diagnostic.main.runtime'; | ||||||
|
|
||||||
|
|
@@ -7,12 +7,12 @@ export class DiagnosticRoute implements Route { | |||||
|
|
||||||
| method = 'GET'; | ||||||
| route = '/_diagnostic'; | ||||||
| verb = Verb.READ; | ||||||
| verb = Verb.WRITE; | ||||||
|
||||||
| verb = Verb.WRITE; | |
| verb = Verb.READ; |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,76 @@ | ||||||||||||||
| import os from 'os'; | ||||||||||||||
| import { expect } from 'chai'; | ||||||||||||||
| import { DiagnosticMain } from './diagnostic.main.runtime'; | ||||||||||||||
|
|
||||||||||||||
| describe('DiagnosticMain', () => { | ||||||||||||||
| describe('getBitVersion', () => { | ||||||||||||||
| it('should return an object with a version string', () => { | ||||||||||||||
| const result = DiagnosticMain.getBitVersion(); | ||||||||||||||
| expect(result).to.have.property('version'); | ||||||||||||||
| expect(result.version).to.be.a('string'); | ||||||||||||||
| }); | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| describe('getProcessInfo', () => { | ||||||||||||||
| it('should return process uptime as a positive number', () => { | ||||||||||||||
| const result = DiagnosticMain.getProcessInfo(); | ||||||||||||||
| expect(result.uptime).to.be.a('number'); | ||||||||||||||
| expect(result.uptime).to.be.greaterThan(0); | ||||||||||||||
| }); | ||||||||||||||
|
Comment on lines
+15
to
+19
|
||||||||||||||
|
|
||||||||||||||
| it('should return the current process pid', () => { | ||||||||||||||
| const result = DiagnosticMain.getProcessInfo(); | ||||||||||||||
| expect(result.pid).to.equal(process.pid); | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| it('should return node version, platform, and arch', () => { | ||||||||||||||
| const result = DiagnosticMain.getProcessInfo(); | ||||||||||||||
| expect(result.nodeVersion).to.equal(process.version); | ||||||||||||||
| expect(result.platform).to.equal(process.platform); | ||||||||||||||
| expect(result.arch).to.equal(process.arch); | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| it('should return memory usage with all expected fields', () => { | ||||||||||||||
| const result = DiagnosticMain.getProcessInfo(); | ||||||||||||||
| expect(result.memory).to.have.property('rss').that.is.a('number'); | ||||||||||||||
| expect(result.memory).to.have.property('heapTotal').that.is.a('number'); | ||||||||||||||
| expect(result.memory).to.have.property('heapUsed').that.is.a('number'); | ||||||||||||||
| expect(result.memory).to.have.property('external').that.is.a('number'); | ||||||||||||||
| expect(result.memory).to.have.property('arrayBuffers').that.is.a('number'); | ||||||||||||||
|
||||||||||||||
| expect(result.memory).to.have.property('arrayBuffers').that.is.a('number'); | |
| if ('arrayBuffers' in result.memory && result.memory.arrayBuffers !== undefined) { | |
| expect(result.memory.arrayBuffers).to.be.a('number'); | |
| } |
Copilot
AI
Mar 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests assume several memory fields are strictly > 0 (rss, heapTotal, heapUsed). Depending on Node/V8 and the runtime environment, some of these can be 0 (especially in minimal/embedded environments), which can cause unnecessary flakes. Prefer >= 0 (or only assert presence/type) unless strict positivity is required by the contract.
| expect(result.memory.rss).to.be.greaterThan(0); | |
| expect(result.memory.heapTotal).to.be.greaterThan(0); | |
| expect(result.memory.heapUsed).to.be.greaterThan(0); | |
| expect(result.memory.rss).to.be.at.least(0); | |
| expect(result.memory.heapTotal).to.be.at.least(0); | |
| expect(result.memory.heapUsed).to.be.at.least(0); |
Copilot
AI
Mar 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test asserts freeMemory is > 0. In some constrained/containerized environments it may be possible to observe 0 (or near-0) free memory, which would make the test flaky even though the implementation is fine. Consider asserting >= 0 and <= totalMemory only.
| expect(result.system.freeMemory).to.be.greaterThan(0); | |
| expect(result.system.freeMemory).to.be.at.least(0); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,6 @@ | ||
| export type { RouteSlot } from './express.main.runtime'; | ||
| export type { Route } from './types'; | ||
| export { Verb } from './types'; | ||
| export type { Request, Response, NextFunction, Middleware } from './types'; | ||
| export type { Route, Request, Response, NextFunction, Middleware } from './types'; | ||
| export type { MiddlewareManifest } from './middleware-manifest'; | ||
| export type { ExpressMain } from './express.main.runtime'; | ||
| export { ExpressAspect } from './express.aspect'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| export { Request } from './request'; | ||
| export { Response } from './response'; | ||
| export { NextFunction } from './next'; | ||
| export { Route, Middleware, Verb } from './route'; | ||
| export type { Request } from './request'; | ||
| export type { Response } from './response'; | ||
| export type { NextFunction } from './next'; | ||
| export type { Route, Middleware } from './route'; | ||
| export { Verb } from './route'; |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,9 @@ | ||||||||
| import type { DocumentNode } from 'graphql'; | ||||||||
| import type { SchemaDirectives } from '@graphql-modules/core'; | ||||||||
|
|
||||||||
| export type { DocumentNode } from 'graphql'; | ||||||||
| export type { SchemaDirectives } from '@graphql-modules/core'; | ||||||||
|
Comment on lines
+4
to
+5
|
||||||||
| export type { DocumentNode } from 'graphql'; | |
| export type { SchemaDirectives } from '@graphql-modules/core'; | |
| export type { DocumentNode, SchemaDirectives }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GraphQL resolver for
_diagnostichas been commented out and now returns an empty object. This breaks the GraphQL API functionality. If the intention is to disable this temporarily, it should either be removed completely or have a clear explanation/TODO comment. If this is intentional and permanent, the schema and resolver should be removed entirely.This change is not mentioned in the PR description and appears to be unrelated to adding the
getProcessInfodiagnostic function.