From 2a1feaf71e6801888a32160634d9c427eced4c4d Mon Sep 17 00:00:00 2001 From: songzhendong <289505773@qq.com> Date: Tue, 7 Jul 2026 11:02:30 +0800 Subject: [PATCH 1/4] feat(runtime): add six instance_nodejs_* meters (6 -> 12) - Extend RuntimeSampler/RuntimeMetricsCollector to 12 meters - Per-snapshot collectedAt on MeterSender.setTimestamp - SW_AGENT_RUNTIME_METRICS_MAX_SNAPSHOTS_PER_REPORT (default 1) - HEAP_SPACE_DETAIL gate + unit tests; TestUnitRemote Node 20/22/24 Signed-off-by: songzhendong <289505773@qq.com> --- .github/workflows/test.yaml | 28 +++ README.md | 8 +- src/agent/core/meter/MeterSender.ts | 16 +- .../core/meter/RuntimeMetricsCollector.ts | 6 + src/agent/core/meter/RuntimeSampler.ts | 31 +++- src/config/AgentConfig.ts | 9 + tests/plugins/express/expected.data.yaml | 52 +++++- tests/remote/MeterSender.test.ts | 167 ++++++++++++++++++ tests/runtime/RuntimeMetricsCollector.test.ts | 76 ++++++-- tests/runtime/RuntimeSampler.test.ts | 143 +++++++++++++++ 10 files changed, 509 insertions(+), 27 deletions(-) create mode 100644 tests/remote/MeterSender.test.ts create mode 100644 tests/runtime/RuntimeSampler.test.ts diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index e8f2c794..dc60c1c7 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -47,6 +47,34 @@ jobs: npm i npm run lint + TestUnitRemote: + runs-on: ubuntu-latest + timeout-minutes: 10 + strategy: + matrix: + node-version: [ 20, 22, 24 ] + steps: + - uses: actions/checkout@v4 + with: + submodules: true + + - uses: actions/cache@v4 + with: + path: ~/.npm + key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }} + restore-keys: | + ${{ runner.os }}-node- + + - name: Set Up NodeJS ${{ matrix.node-version }} + uses: actions/setup-node@v4 + with: + node-version: ${{ matrix.node-version }} + + - name: Unit tests on Node@${{ matrix.node-version }} + run: | + npm i + npx jest tests/remote/ tests/config/ tests/runtime/ --runInBand + build-matrix: runs-on: ubuntu-latest timeout-minutes: 5 diff --git a/README.md b/README.md index 50586c5d..bfda9d20 100644 --- a/README.md +++ b/README.md @@ -84,7 +84,7 @@ Note that the various ignore options like `SW_IGNORE_SUFFIX`, `SW_TRACE_IGNORE_P ## Node.js Runtime Metrics -The agent reports six process-level meters (`instance_nodejs_*`) via `MeterReportService` by default (collect 1s, report 1s). Set `SW_AGENT_NODEJS_RUNTIME_METRICS_REPORTER_ACTIVE=false` to disable. Process CPU combines `process.cpuUsage()` user + system, normalized by logical CPU count (0–100%). +The agent reports twelve process-level meters (`instance_nodejs_*`) via `MeterReportService` by default (collect 1s, report 1s). Set `SW_AGENT_NODEJS_RUNTIME_METRICS_REPORTER_ACTIVE=false` to disable. Process CPU combines `process.cpuUsage()` user + system, normalized by logical CPU count (0–100%). | Node.js source | Meter name | Notes | | :--- | :--- | :--- | @@ -94,6 +94,12 @@ The agent reports six process-level meters (`instance_nodejs_*`) via `MeterRepor | `v8.getHeapStatistics().heap_size_limit` | `instance_nodejs_heap_limit` | bytes | | `process.memoryUsage().rss` | `instance_nodejs_rss` | bytes | | `process.memoryUsage().external` | `instance_nodejs_external_memory` | bytes | +| `process.memoryUsage().arrayBuffers` | `instance_nodejs_array_buffers` | bytes | +| `process.uptime()` | `instance_nodejs_uptime` | seconds | +| `v8.getHeapStatistics().peak_malloced_memory` | `instance_nodejs_peak_malloced_memory` | bytes | +| `v8.getHeapStatistics().malloced_memory` | `instance_nodejs_malloced_memory` | bytes | +| `v8.getHeapSpaceStatistics()` old_space | `instance_nodejs_old_space_used` | bytes | +| `v8.getHeapSpaceStatistics()` new_space | `instance_nodejs_new_space_used` | bytes | Custom business metrics are not available through a public API; use [OpenTelemetry metrics](https://skywalking.apache.org/docs/main/latest/en/setup/backend/opentelemetry-receiver/) if you need those. diff --git a/src/agent/core/meter/MeterSender.ts b/src/agent/core/meter/MeterSender.ts index b66d370a..5db370ac 100644 --- a/src/agent/core/meter/MeterSender.ts +++ b/src/agent/core/meter/MeterSender.ts @@ -142,7 +142,8 @@ export default class MeterSender implements BootService, GRPCChannelListener { return; } - const snapshots = this.buffer.splice(0, this.buffer.length); + const maxSnapshots = config.runtimeMetricsMaxSnapshotsPerReport ?? 1; + const snapshots = this.buffer.splice(0, Math.min(this.buffer.length, maxSnapshots)); const stream = this.reporterClient.collect( new grpc.Metadata(), { deadline: Date.now() + (config.traceTimeout || 10000) }, @@ -156,17 +157,12 @@ export default class MeterSender implements BootService, GRPCChannelListener { ); try { - let metadataWritten = false; - const timestamp = Date.now(); for (const snapshot of snapshots) { for (const meterData of this.collector.toMeterData(snapshot)) { - if (!metadataWritten) { - meterData - .setService(config.serviceName) - .setServiceinstance(config.serviceInstance) - .setTimestamp(timestamp); - metadataWritten = true; - } + meterData + .setService(config.serviceName) + .setServiceinstance(config.serviceInstance) + .setTimestamp(snapshot.collectedAt); stream.write(meterData); } } diff --git a/src/agent/core/meter/RuntimeMetricsCollector.ts b/src/agent/core/meter/RuntimeMetricsCollector.ts index 0d59a296..af288d1e 100644 --- a/src/agent/core/meter/RuntimeMetricsCollector.ts +++ b/src/agent/core/meter/RuntimeMetricsCollector.ts @@ -36,6 +36,12 @@ export default class RuntimeMetricsCollector { ['instance_nodejs_heap_limit', snapshot.heapSizeLimit], ['instance_nodejs_rss', snapshot.rss], ['instance_nodejs_external_memory', snapshot.external], + ['instance_nodejs_array_buffers', snapshot.arrayBuffers], + ['instance_nodejs_uptime', snapshot.uptime], + ['instance_nodejs_peak_malloced_memory', snapshot.peakMallocedMemory], + ['instance_nodejs_malloced_memory', snapshot.mallocedMemory], + ['instance_nodejs_old_space_used', snapshot.oldSpaceUsed], + ['instance_nodejs_new_space_used', snapshot.newSpaceUsed], ]; return gauges.map(([name, value]) => diff --git a/src/agent/core/meter/RuntimeSampler.ts b/src/agent/core/meter/RuntimeSampler.ts index c7ac80eb..4654e3b0 100644 --- a/src/agent/core/meter/RuntimeSampler.ts +++ b/src/agent/core/meter/RuntimeSampler.ts @@ -19,8 +19,10 @@ import os from 'os'; import v8 from 'v8'; +import config from '../../../config/AgentConfig'; export type RuntimeSnapshot = { + collectedAt: number; heapUsed: number; heapTotal: number; heapSizeLimit: number; @@ -28,6 +30,12 @@ export type RuntimeSnapshot = { external: number; cpuUserPercent: number; cpuSystemPercent: number; + arrayBuffers: number; + uptime: number; + peakMallocedMemory: number; + mallocedMemory: number; + oldSpaceUsed: number; + newSpaceUsed: number; }; export default class RuntimeSampler { @@ -38,17 +46,30 @@ export default class RuntimeSampler { sample(): RuntimeSnapshot { const memory = process.memoryUsage(); const heapStats = v8.getHeapStatistics(); - const cpuUsage = process.cpuUsage(this.lastCpuUsage); + const cpuNow = process.cpuUsage(); + const cpuUsage = { + user: cpuNow.user - this.lastCpuUsage.user, + system: cpuNow.system - this.lastCpuUsage.system, + }; const now = process.hrtime.bigint(); const elapsedMicros = Number(now - this.lastCpuTimestamp) / 1000; - this.lastCpuUsage = process.cpuUsage(); + this.lastCpuUsage = cpuNow; this.lastCpuTimestamp = now; const cpuScale = elapsedMicros > 0 ? 100 / elapsedMicros / this.logicalCpuCount : 0; const cpuUserPercent = cpuUsage.user * cpuScale; const cpuSystemPercent = cpuUsage.system * cpuScale; + const heapSpaceDetail = config.runtimeMetricsHeapSpaceDetail !== false; + let oldSpaceUsed = 0; + let newSpaceUsed = 0; + if (heapSpaceDetail) { + const heapSpaces = v8.getHeapSpaceStatistics(); + oldSpaceUsed = heapSpaces.find((entry) => entry.space_name === 'old_space')?.space_used_size ?? 0; + newSpaceUsed = heapSpaces.find((entry) => entry.space_name === 'new_space')?.space_used_size ?? 0; + } return { + collectedAt: Date.now(), heapUsed: memory.heapUsed, heapTotal: memory.heapTotal, heapSizeLimit: heapStats.heap_size_limit, @@ -56,6 +77,12 @@ export default class RuntimeSampler { external: memory.external, cpuUserPercent, cpuSystemPercent, + arrayBuffers: memory.arrayBuffers ?? 0, + uptime: process.uptime(), + peakMallocedMemory: heapStats.peak_malloced_memory, + mallocedMemory: heapStats.malloced_memory, + oldSpaceUsed, + newSpaceUsed, }; } diff --git a/src/config/AgentConfig.ts b/src/config/AgentConfig.ts index 54d8a34c..67424d9d 100644 --- a/src/config/AgentConfig.ts +++ b/src/config/AgentConfig.ts @@ -47,6 +47,8 @@ export type AgentConfig = { runtimeMetricsCollectPeriod?: number; runtimeMetricsReportPeriod?: number; runtimeMetricsBufferSize?: number; + runtimeMetricsMaxSnapshotsPerReport?: number; + runtimeMetricsHeapSpaceDetail?: boolean; /** @deprecated use runtimeMetricsReporterActive */ nvmMetricsReporterActive?: boolean; /** @deprecated use runtimeMetricsCollectPeriod */ @@ -279,6 +281,13 @@ const _config = { 10, ), ), + runtimeMetricsHeapSpaceDetail: ((): boolean => { + const configured = process.env.SW_AGENT_RUNTIME_METRICS_HEAP_SPACE_DETAIL; + return configured?.toLowerCase() !== 'false'; + })(), + runtimeMetricsMaxSnapshotsPerReport: ((n) => (Number.isSafeInteger(n) && n > 0 ? n : 1))( + Number.parseInt(process.env.SW_AGENT_RUNTIME_METRICS_MAX_SNAPSHOTS_PER_REPORT ?? '', 10), + ), runtimeMetricsBufferSize: ((n) => (Number.isSafeInteger(n) && n > 0 ? n : 600))( Number.parseInt( process.env.SW_AGENT_NODEJS_RUNTIME_METRICS_BUFFER_SIZE ?? diff --git a/tests/plugins/express/expected.data.yaml b/tests/plugins/express/expected.data.yaml index afddb269..a3235b38 100644 --- a/tests/plugins/express/expected.data.yaml +++ b/tests/plugins/express/expected.data.yaml @@ -122,7 +122,7 @@ segmentItems: meterItems: - serviceName: server - meterSize: 6 + meterSize: 12 meters: - meterId: name: instance_nodejs_process_cpu @@ -148,8 +148,32 @@ meterItems: name: instance_nodejs_external_memory tags: [] singleValue: gt 0 + - meterId: + name: instance_nodejs_array_buffers + tags: [] + singleValue: ge 0 + - meterId: + name: instance_nodejs_uptime + tags: [] + singleValue: gt 0 + - meterId: + name: instance_nodejs_peak_malloced_memory + tags: [] + singleValue: gt 0 + - meterId: + name: instance_nodejs_malloced_memory + tags: [] + singleValue: gt 0 + - meterId: + name: instance_nodejs_old_space_used + tags: [] + singleValue: gt 0 + - meterId: + name: instance_nodejs_new_space_used + tags: [] + singleValue: gt 0 - serviceName: client - meterSize: 6 + meterSize: 12 meters: - meterId: name: instance_nodejs_process_cpu @@ -175,3 +199,27 @@ meterItems: name: instance_nodejs_external_memory tags: [] singleValue: gt 0 + - meterId: + name: instance_nodejs_array_buffers + tags: [] + singleValue: ge 0 + - meterId: + name: instance_nodejs_uptime + tags: [] + singleValue: gt 0 + - meterId: + name: instance_nodejs_peak_malloced_memory + tags: [] + singleValue: gt 0 + - meterId: + name: instance_nodejs_malloced_memory + tags: [] + singleValue: gt 0 + - meterId: + name: instance_nodejs_old_space_used + tags: [] + singleValue: gt 0 + - meterId: + name: instance_nodejs_new_space_used + tags: [] + singleValue: gt 0 diff --git a/tests/remote/MeterSender.test.ts b/tests/remote/MeterSender.test.ts new file mode 100644 index 00000000..a2cf7809 --- /dev/null +++ b/tests/remote/MeterSender.test.ts @@ -0,0 +1,167 @@ +/*! + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +/* eslint-env jest */ + +import { GRPCChannelStatus } from '../../src/agent/core/remote/GRPCChannelStatus'; + +const mockCollect = jest.fn(); +let pendingCollectCallback: ((error: Error | null) => void) | undefined; +let sampleSequence = 0; + +const mockChannelManager = { + addChannelListener: jest.fn(), + getClientOptions: jest.fn(() => ({})), + reportError: jest.fn(), +}; + +const mockMeterData = { + setService: jest.fn().mockReturnThis(), + setServiceinstance: jest.fn().mockReturnThis(), + setTimestamp: jest.fn().mockReturnThis(), +}; + +const mockStream = { + write: jest.fn(), + end: jest.fn(), +}; + +const mockSnapshot = () => ({ collectedAt: 1_000_000 + sampleSequence++ * 500, cpu: 1 }); + +jest.mock('../../src/config/AgentConfig', () => ({ + __esModule: true, + default: { + serviceName: 'meter-service', + serviceInstance: 'meter-instance', + traceTimeout: 10000, + runtimeMetricsCollectPeriod: 1000, + runtimeMetricsReportPeriod: 1000, + runtimeMetricsBufferSize: 600, + runtimeMetricsMaxSnapshotsPerReport: 1, + }, +})); + +jest.mock('../../src/proto/language-agent/Meter_grpc_pb', () => ({ + MeterReportServiceClient: jest.fn().mockImplementation(() => ({ + collect: jest.fn((_meta, _opts, cb) => { + pendingCollectCallback = cb; + return mockStream; + }), + })), +})); + +jest.mock('../../src/agent/core/meter/RuntimeMetricsCollector', () => { + return jest.fn().mockImplementation(() => ({ + sample: jest.fn(() => mockSnapshot()), + toMeterData: jest.fn(() => [mockMeterData, { ...mockMeterData }]), + destroy: jest.fn(), + })); +}); + +jest.mock('../../src/agent/core/boot/ServiceManager', () => ({ + __esModule: true, + default: { + INSTANCE: { + findService: jest.fn(() => mockChannelManager), + }, + }, +})); + +jest.mock('../../src/logging', () => ({ + createLogger: () => ({ + warn: jest.fn(), + info: jest.fn(), + error: jest.fn(), + _isDebugEnabled: false, + }), + throttled: () => jest.fn(), +})); + +import MeterSender from '../../src/agent/core/meter/MeterSender'; +import { MeterReportServiceClient } from '../../src/proto/language-agent/Meter_grpc_pb'; + +describe('MeterSender', () => { + let sender: MeterSender; + + beforeEach(() => { + jest.useFakeTimers(); + sampleSequence = 0; + mockCollect.mockClear(); + mockChannelManager.reportError.mockClear(); + mockMeterData.setService.mockClear(); + mockMeterData.setServiceinstance.mockClear(); + mockMeterData.setTimestamp.mockClear(); + mockStream.write.mockReset(); + mockStream.end.mockReset(); + pendingCollectCallback = undefined; + sender = new MeterSender(); + sender.prepare(); + sender.statusChanged(GRPCChannelStatus.CONNECTED); + sender.boot(); + }); + + afterEach(() => { + sender.shutdown(); + jest.useRealTimers(); + }); + + it('clears reporter stub on DISCONNECT', () => { + sender.statusChanged(GRPCChannelStatus.DISCONNECT); + expect(MeterReportServiceClient).toHaveBeenCalledTimes(1); + }); + + it('uses per-snapshot collectedAt timestamps', async () => { + const senderAny = sender as unknown as { buffer: Array<{ collectedAt: number }> }; + senderAny.buffer.push({ collectedAt: 1_111_111 } as never, { collectedAt: 2_222_222 } as never); + + const firstReport = (sender as unknown as { reportBufferedMetrics: () => Promise }).reportBufferedMetrics(); + await Promise.resolve(); + pendingCollectCallback?.(null); + await firstReport; + + const secondReport = (sender as unknown as { reportBufferedMetrics: () => Promise }).reportBufferedMetrics(); + await Promise.resolve(); + pendingCollectCallback?.(null); + await secondReport; + + const timestamps = mockMeterData.setTimestamp.mock.calls.map((call) => call[0]); + expect(timestamps).toEqual(expect.arrayContaining([1_111_111, 2_222_222])); + }); + + it('drains at most one snapshot per report tick by default', async () => { + const senderAny = sender as unknown as { buffer: Array<{ collectedAt: number }> }; + senderAny.buffer.push({ collectedAt: 1 } as never, { collectedAt: 2 } as never, { collectedAt: 3 } as never); + + const reportPromise = (sender as unknown as { reportBufferedMetrics: () => Promise }).reportBufferedMetrics(); + await Promise.resolve(); + pendingCollectCallback?.(null); + await reportPromise; + + expect(senderAny.buffer.length).toBe(2); + expect(mockStream.write).toHaveBeenCalled(); + }); + + it('skips duplicate boot timers', () => { + const collectTimer = (sender as unknown as { collectTimer?: NodeJS.Timeout }).collectTimer; + const reportTimer = (sender as unknown as { reportTimer?: NodeJS.Timeout }).reportTimer; + sender.boot(); + expect((sender as unknown as { collectTimer?: NodeJS.Timeout }).collectTimer).toBe(collectTimer); + expect((sender as unknown as { reportTimer?: NodeJS.Timeout }).reportTimer).toBe(reportTimer); + }); +}); diff --git a/tests/runtime/RuntimeMetricsCollector.test.ts b/tests/runtime/RuntimeMetricsCollector.test.ts index 35b7833f..f9e437ab 100644 --- a/tests/runtime/RuntimeMetricsCollector.test.ts +++ b/tests/runtime/RuntimeMetricsCollector.test.ts @@ -20,6 +20,22 @@ /* eslint-env jest */ import RuntimeMetricsCollector from '../../src/agent/core/meter/RuntimeMetricsCollector'; +import { RuntimeSnapshot } from '../../src/agent/core/meter/RuntimeSampler'; + +const EXPECTED_METER_NAMES = [ + 'instance_nodejs_process_cpu', + 'instance_nodejs_heap_used', + 'instance_nodejs_heap_total', + 'instance_nodejs_heap_limit', + 'instance_nodejs_rss', + 'instance_nodejs_external_memory', + 'instance_nodejs_array_buffers', + 'instance_nodejs_uptime', + 'instance_nodejs_peak_malloced_memory', + 'instance_nodejs_malloced_memory', + 'instance_nodejs_old_space_used', + 'instance_nodejs_new_space_used', +]; describe('RuntimeMetricsCollector', () => { let collector: RuntimeMetricsCollector; @@ -37,21 +53,57 @@ describe('RuntimeMetricsCollector', () => { const meters = collector.toMeterData(snapshot); const names = meters.map((meter) => meter.getSinglevalue()?.getName()); - expect(names).toEqual( - expect.arrayContaining([ - 'instance_nodejs_process_cpu', - 'instance_nodejs_heap_used', - 'instance_nodejs_heap_total', - 'instance_nodejs_heap_limit', - 'instance_nodejs_rss', - 'instance_nodejs_external_memory', - ]), - ); - - expect(names).toHaveLength(6); + expect(names).toEqual(EXPECTED_METER_NAMES); for (const meter of meters) { expect(meter.getSinglevalue()?.getValue()).toBeGreaterThanOrEqual(0); } + + expect(snapshot.uptime).toBeGreaterThanOrEqual(0); + expect(snapshot.oldSpaceUsed).toBeGreaterThanOrEqual(0); + expect(snapshot.newSpaceUsed).toBeGreaterThanOrEqual(0); + }); + + it('maps extended runtime snapshot values into meter single values', () => { + const snapshot: RuntimeSnapshot = { + collectedAt: 1_700_000_000_000, + heapUsed: 100, + heapTotal: 200, + heapSizeLimit: 300, + rss: 400, + external: 50, + cpuUserPercent: 1.2, + cpuSystemPercent: 0.8, + arrayBuffers: 16, + uptime: 42.5, + peakMallocedMemory: 2048, + mallocedMemory: 3072, + oldSpaceUsed: 88, + newSpaceUsed: 12, + }; + + const meters = collector.toMeterData(snapshot); + const values: Record = {}; + for (const meter of meters) { + const name = meter.getSinglevalue()?.getName(); + if (name) { + values[name] = meter.getSinglevalue()?.getValue(); + } + } + + expect(values).toEqual({ + instance_nodejs_process_cpu: 2, + instance_nodejs_heap_used: 100, + instance_nodejs_heap_total: 200, + instance_nodejs_heap_limit: 300, + instance_nodejs_rss: 400, + instance_nodejs_external_memory: 50, + instance_nodejs_array_buffers: 16, + instance_nodejs_uptime: 42.5, + instance_nodejs_peak_malloced_memory: 2048, + instance_nodejs_malloced_memory: 3072, + instance_nodejs_old_space_used: 88, + instance_nodejs_new_space_used: 12, + }); }); }); diff --git a/tests/runtime/RuntimeSampler.test.ts b/tests/runtime/RuntimeSampler.test.ts new file mode 100644 index 00000000..1979084b --- /dev/null +++ b/tests/runtime/RuntimeSampler.test.ts @@ -0,0 +1,143 @@ +/*! + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +/* eslint-env jest */ +/* global BigInt */ + +import os from 'os'; +import v8 from 'v8'; +import RuntimeSampler from '../../src/agent/core/meter/RuntimeSampler'; + +describe('RuntimeSampler', () => { + let sampler: RuntimeSampler; + + beforeEach(() => { + sampler = new RuntimeSampler(); + }); + + afterEach(() => { + sampler.destroy(); + }); + + it('records collectedAt at sample time', () => { + jest.spyOn(Date, 'now').mockReturnValueOnce(1_700_000_000_000); + expect(sampler.sample().collectedAt).toBe(1_700_000_000_000); + }); + + it('samples array buffers, uptime, heap stats, and heap spaces', () => { + const memoryUsageSpy = jest.spyOn(process, 'memoryUsage').mockReturnValue({ + rss: 1, + heapTotal: 2, + heapUsed: 3, + external: 4, + arrayBuffers: 5, + }); + const uptimeSpy = jest.spyOn(process, 'uptime').mockReturnValue(99); + const heapStatsSpy = jest.spyOn(v8, 'getHeapStatistics').mockReturnValue({ + heap_size_limit: 1000, + peak_malloced_memory: 2000, + malloced_memory: 4096, + } as ReturnType); + const heapSpaceSpy = jest + .spyOn(v8, 'getHeapSpaceStatistics') + .mockReturnValue([ + { space_name: 'old_space', space_used_size: 80 } as v8.HeapSpaceInfo, + { space_name: 'new_space', space_used_size: 20 } as v8.HeapSpaceInfo, + ]); + + const snapshot = sampler.sample(); + + expect(snapshot.arrayBuffers).toBe(5); + expect(snapshot.uptime).toBe(99); + expect(snapshot.peakMallocedMemory).toBe(2000); + expect(snapshot.mallocedMemory).toBe(4096); + expect(snapshot.oldSpaceUsed).toBe(80); + expect(snapshot.newSpaceUsed).toBe(20); + + memoryUsageSpy.mockRestore(); + uptimeSpy.mockRestore(); + heapStatsSpy.mockRestore(); + heapSpaceSpy.mockRestore(); + }); + + it('normalizes process CPU by logical core count', () => { + jest.spyOn(os, 'cpus').mockReturnValue([{}, {}, {}, {}] as os.CpuInfo[]); + + let cpuCall = 0; + const cpuUsageSpy = jest.spyOn(process, 'cpuUsage').mockImplementation(() => { + cpuCall += 1; + if (cpuCall === 1) { + return { user: 0, system: 0 }; + } + return { user: 1_000_000, system: 500_000 }; + }); + + let hrtimeCall = 0; + const hrtimeSpy = jest.spyOn(process.hrtime, 'bigint').mockImplementation(() => { + hrtimeCall += 1; + if (hrtimeCall === 1) { + return BigInt(0); + } + return BigInt(1_000_000_000); + }); + + const cpuSampler = new RuntimeSampler(); + const snapshot = cpuSampler.sample(); + + // 1s wall, 1 core-second user + 0.5 core-second system on a 4-logical-CPU host => 25% + 12.5% + expect(snapshot.cpuUserPercent).toBeCloseTo(25); + expect(snapshot.cpuSystemPercent).toBeCloseTo(12.5); + expect(snapshot.cpuUserPercent + snapshot.cpuSystemPercent).toBeCloseTo(37.5); + + cpuSampler.destroy(); + cpuUsageSpy.mockRestore(); + hrtimeSpy.mockRestore(); + jest.restoreAllMocks(); + }); + + it('skips heap space statistics when heap space detail is disabled', () => { + const config = require('../../src/config/AgentConfig').default as { + runtimeMetricsHeapSpaceDetail?: boolean; + }; + const original = config.runtimeMetricsHeapSpaceDetail; + config.runtimeMetricsHeapSpaceDetail = false; + const heapSpaceSpy = jest.spyOn(v8, 'getHeapSpaceStatistics'); + + try { + const snapshot = sampler.sample(); + expect(heapSpaceSpy).not.toHaveBeenCalled(); + expect(snapshot.oldSpaceUsed).toBe(0); + expect(snapshot.newSpaceUsed).toBe(0); + } finally { + config.runtimeMetricsHeapSpaceDetail = original; + heapSpaceSpy.mockRestore(); + } + }); + + it('defaults missing heap spaces to zero', () => { + jest.spyOn(v8, 'getHeapSpaceStatistics').mockReturnValue([]); + + const snapshot = sampler.sample(); + + expect(snapshot.oldSpaceUsed).toBe(0); + expect(snapshot.newSpaceUsed).toBe(0); + + jest.restoreAllMocks(); + }); +}); From 71675ada0bc14c80274b2cede53f16fa8f8b93a7 Mon Sep 17 00:00:00 2001 From: songzhendong <289505773@qq.com> Date: Wed, 5 Aug 2026 23:01:18 +0800 Subject: [PATCH 2/4] fix(runtime): address meter reporting review feedback --- README.md | 14 ++--- src/agent/core/meter/MeterSender.ts | 30 ++++----- .../core/meter/RuntimeMetricsCollector.ts | 19 +++++- src/agent/core/meter/RuntimeSampler.ts | 12 +--- src/config/AgentConfig.ts | 50 ++------------- tests/config/AgentConfig.test.ts | 11 +--- tests/plugins/express/client.ts | 16 +++-- tests/plugins/express/expected.data.yaml | 8 +-- tests/plugins/express/server.ts | 4 ++ tests/remote/MeterSender.test.ts | 62 +++++++++++-------- tests/runtime/RuntimeMetricsCollector.test.ts | 57 +++++++++++------ tests/runtime/RuntimeSampler.test.ts | 19 ------ 12 files changed, 144 insertions(+), 158 deletions(-) diff --git a/README.md b/README.md index bfda9d20..29d89f7a 100644 --- a/README.md +++ b/README.md @@ -72,19 +72,19 @@ Environment Variable | Description | Default | `SW_AWS_SQS_CHECK_BODY` | Incoming SQS messages check inside the body for trace ID in order to allow linking outgoing SNS messages to incoming SQS. | `false` | | `SW_AGENT_MAX_BUFFER_SIZE` | The maximum buffer size before sending the segment data to backend | `'1000'` | | `SW_AGENT_TRACE_TIMEOUT` | The timeout for trace requests to backend services | `'10000'` | -| `SW_AGENT_NODEJS_RUNTIME_METRICS_REPORTER_ACTIVE` | Whether to report Node.js runtime metrics through MeterReportService (default: collect 1s, report 1s) | `true` | -| `SW_AGENT_NODEJS_RUNTIME_METRICS_COLLECT_PERIOD` | Runtime metric sample interval in milliseconds | `1000` | -| `SW_AGENT_NODEJS_RUNTIME_METRICS_REPORT_PERIOD` | Runtime metric report interval in milliseconds (aligned with Java JVM metrics upload interval) | `1000` | -| `SW_AGENT_NODEJS_RUNTIME_METRICS_BUFFER_SIZE` | Maximum buffered runtime metric samples before dropping oldest | `600` | +| `SW_AGENT_NODEJS_RUNTIME_METRICS_REPORTER_ACTIVE` | Whether to report Node.js runtime metrics through MeterReportService (default: collect 20s, report 20s) | `true` | +| `SW_AGENT_NODEJS_RUNTIME_METRICS_COLLECT_PERIOD` | Runtime metric sample interval in milliseconds | `20000` | +| `SW_AGENT_NODEJS_RUNTIME_METRICS_REPORT_PERIOD` | Runtime metric report interval in milliseconds (aligned with Java `meter.report_interval`) | `20000` | +| `SW_AGENT_NODEJS_RUNTIME_METRICS_UPTIME_REPORT_PERIOD` | How often to include `instance_nodejs_uptime` in a report (ms); other runtime meters still follow the report period | `30000` | -Legacy env names `SW_AGENT_RUNTIME_METRICS_*`, `SW_AGENT_NVM_METRICS_*` and `SW_AGENT_NVM_JVM_*` are still accepted as deprecated aliases. +Legacy env names `SW_AGENT_RUNTIME_METRICS_*` / `SW_AGENT_NVM_*` for reporter active, collect period, and report period are still accepted as deprecated aliases. Note that the various ignore options like `SW_IGNORE_SUFFIX`, `SW_TRACE_IGNORE_PATH` and `SW_HTTP_IGNORE_METHOD` as well as endpoints which are not recorded due to exceeding `SW_AGENT_MAX_BUFFER_SIZE` all propagate their ignored status downstream to any other endpoints they may call. If that endpoint is running the Node Skywalking agent then regardless of its ignore settings it will not be recorded since its upstream parent was not recorded. This allows the elimination of entire trees of endpoints you are not interested in as well as eliminating partial traces if a span in the chain is ignored but calls out to other endpoints which are recorded as children of ROOT instead of the actual parent. ## Node.js Runtime Metrics -The agent reports twelve process-level meters (`instance_nodejs_*`) via `MeterReportService` by default (collect 1s, report 1s). Set `SW_AGENT_NODEJS_RUNTIME_METRICS_REPORTER_ACTIVE=false` to disable. Process CPU combines `process.cpuUsage()` user + system, normalized by logical CPU count (0–100%). +The agent reports twelve process-level meters (`instance_nodejs_*`) via `MeterReportService` by default (collect 20s, report 20s; `instance_nodejs_uptime` every 30s). Set `SW_AGENT_NODEJS_RUNTIME_METRICS_REPORTER_ACTIVE=false` to disable. Process CPU combines `process.cpuUsage()` user + system, normalized by logical CPU count (0–100%). | Node.js source | Meter name | Notes | | :--- | :--- | :--- | @@ -95,7 +95,7 @@ The agent reports twelve process-level meters (`instance_nodejs_*`) via `MeterRe | `process.memoryUsage().rss` | `instance_nodejs_rss` | bytes | | `process.memoryUsage().external` | `instance_nodejs_external_memory` | bytes | | `process.memoryUsage().arrayBuffers` | `instance_nodejs_array_buffers` | bytes | -| `process.uptime()` | `instance_nodejs_uptime` | seconds | +| `process.uptime()` | `instance_nodejs_uptime` | seconds; included every 30s by default | | `v8.getHeapStatistics().peak_malloced_memory` | `instance_nodejs_peak_malloced_memory` | bytes | | `v8.getHeapStatistics().malloced_memory` | `instance_nodejs_malloced_memory` | bytes | | `v8.getHeapSpaceStatistics()` old_space | `instance_nodejs_old_space_used` | bytes | diff --git a/src/agent/core/meter/MeterSender.ts b/src/agent/core/meter/MeterSender.ts index 5db370ac..a0cb873b 100644 --- a/src/agent/core/meter/MeterSender.ts +++ b/src/agent/core/meter/MeterSender.ts @@ -38,7 +38,8 @@ export default class MeterSender implements BootService, GRPCChannelListener { private channelManager?: GRPCChannelManager; private status = GRPCChannelStatus.DISCONNECT; private reporterClient?: MeterReportServiceClient; - private readonly buffer: RuntimeSnapshot[] = []; + /** Latest gauge snapshot only — stale samples have no value after reconnect. */ + private latestSnapshot?: RuntimeSnapshot; private collectTimer?: NodeJS.Timeout; private reportTimer?: NodeJS.Timeout; private reporting?: Promise; @@ -89,23 +90,19 @@ export default class MeterSender implements BootService, GRPCChannelListener { return; } this.collectSample(); - }, config.runtimeMetricsCollectPeriod || 1000) as NodeJS.Timeout; + }, config.runtimeMetricsCollectPeriod || 20000) as NodeJS.Timeout; this.collectTimer.unref(); this.reportTimer = setInterval(() => { if (this.closed) { return; } void this.reportBufferedMetrics(); - }, config.runtimeMetricsReportPeriod || 1000) as NodeJS.Timeout; + }, config.runtimeMetricsReportPeriod || 20000) as NodeJS.Timeout; this.reportTimer.unref(); } private collectSample(): void { - const maxBufferSize = config.runtimeMetricsBufferSize || 600; - if (this.buffer.length >= maxBufferSize) { - this.buffer.shift(); - } - this.buffer.push(this.collector.sample()); + this.latestSnapshot = this.collector.sample(); } private reportBufferedMetrics(): Promise { @@ -132,7 +129,9 @@ export default class MeterSender implements BootService, GRPCChannelListener { return; } - if (this.buffer.length === 0 || this.status !== GRPCChannelStatus.CONNECTED || !this.reporterClient) { + const snapshot = this.latestSnapshot; + this.latestSnapshot = undefined; + if (!snapshot || this.status !== GRPCChannelStatus.CONNECTED || !this.reporterClient) { resolve(); return; } @@ -142,8 +141,6 @@ export default class MeterSender implements BootService, GRPCChannelListener { return; } - const maxSnapshots = config.runtimeMetricsMaxSnapshotsPerReport ?? 1; - const snapshots = this.buffer.splice(0, Math.min(this.buffer.length, maxSnapshots)); const stream = this.reporterClient.collect( new grpc.Metadata(), { deadline: Date.now() + (config.traceTimeout || 10000) }, @@ -157,14 +154,17 @@ export default class MeterSender implements BootService, GRPCChannelListener { ); try { - for (const snapshot of snapshots) { - for (const meterData of this.collector.toMeterData(snapshot)) { + let metadataWritten = false; + for (const meterData of this.collector.toMeterData(snapshot)) { + // Meter.proto: service / instance / timestamp on the first stream element only. + if (!metadataWritten) { meterData .setService(config.serviceName) .setServiceinstance(config.serviceInstance) .setTimestamp(snapshot.collectedAt); - stream.write(meterData); + metadataWritten = true; } + stream.write(meterData); } } finally { try { @@ -210,7 +210,7 @@ export default class MeterSender implements BootService, GRPCChannelListener { } this.reporting = undefined; this.reporterClient = undefined; - this.buffer.length = 0; + this.latestSnapshot = undefined; this.collector.destroy(); this.channelManager = undefined; logger.info('MeterSender destroyed and resources cleaned up'); diff --git a/src/agent/core/meter/RuntimeMetricsCollector.ts b/src/agent/core/meter/RuntimeMetricsCollector.ts index af288d1e..05b9027a 100644 --- a/src/agent/core/meter/RuntimeMetricsCollector.ts +++ b/src/agent/core/meter/RuntimeMetricsCollector.ts @@ -17,12 +17,17 @@ * */ +import config from '../../../config/AgentConfig'; import { MeterData, MeterSingleValue } from '../../../proto/language-agent/Meter_pb'; import RuntimeSampler, { RuntimeSnapshot } from './RuntimeSampler'; +const DEFAULT_UPTIME_REPORT_PERIOD_MS = 30000; + /** Maps Node.js runtime samples into MeterReportService single-value meters (instance_nodejs_*). */ export default class RuntimeMetricsCollector { private readonly sampler = new RuntimeSampler(); + /** collectedAt of the last report that included instance_nodejs_uptime. */ + private lastUptimeReportAt = 0; sample(): RuntimeSnapshot { return this.sampler.sample(); @@ -37,18 +42,30 @@ export default class RuntimeMetricsCollector { ['instance_nodejs_rss', snapshot.rss], ['instance_nodejs_external_memory', snapshot.external], ['instance_nodejs_array_buffers', snapshot.arrayBuffers], - ['instance_nodejs_uptime', snapshot.uptime], ['instance_nodejs_peak_malloced_memory', snapshot.peakMallocedMemory], ['instance_nodejs_malloced_memory', snapshot.mallocedMemory], ['instance_nodejs_old_space_used', snapshot.oldSpaceUsed], ['instance_nodejs_new_space_used', snapshot.newSpaceUsed], ]; + if (this.shouldIncludeUptime(snapshot.collectedAt)) { + gauges.splice(7, 0, ['instance_nodejs_uptime', snapshot.uptime]); + this.lastUptimeReportAt = snapshot.collectedAt; + } + return gauges.map(([name, value]) => new MeterData().setSinglevalue(new MeterSingleValue().setName(name).setValue(value)), ); } + private shouldIncludeUptime(collectedAt: number): boolean { + if (this.lastUptimeReportAt === 0) { + return true; + } + const period = config.runtimeMetricsUptimeReportPeriod || DEFAULT_UPTIME_REPORT_PERIOD_MS; + return collectedAt - this.lastUptimeReportAt >= period; + } + destroy(): void { this.sampler.destroy(); } diff --git a/src/agent/core/meter/RuntimeSampler.ts b/src/agent/core/meter/RuntimeSampler.ts index 4654e3b0..35f5863b 100644 --- a/src/agent/core/meter/RuntimeSampler.ts +++ b/src/agent/core/meter/RuntimeSampler.ts @@ -19,7 +19,6 @@ import os from 'os'; import v8 from 'v8'; -import config from '../../../config/AgentConfig'; export type RuntimeSnapshot = { collectedAt: number; @@ -59,14 +58,9 @@ export default class RuntimeSampler { const cpuScale = elapsedMicros > 0 ? 100 / elapsedMicros / this.logicalCpuCount : 0; const cpuUserPercent = cpuUsage.user * cpuScale; const cpuSystemPercent = cpuUsage.system * cpuScale; - const heapSpaceDetail = config.runtimeMetricsHeapSpaceDetail !== false; - let oldSpaceUsed = 0; - let newSpaceUsed = 0; - if (heapSpaceDetail) { - const heapSpaces = v8.getHeapSpaceStatistics(); - oldSpaceUsed = heapSpaces.find((entry) => entry.space_name === 'old_space')?.space_used_size ?? 0; - newSpaceUsed = heapSpaces.find((entry) => entry.space_name === 'new_space')?.space_used_size ?? 0; - } + const heapSpaces = v8.getHeapSpaceStatistics(); + const oldSpaceUsed = heapSpaces.find((entry) => entry.space_name === 'old_space')?.space_used_size ?? 0; + const newSpaceUsed = heapSpaces.find((entry) => entry.space_name === 'new_space')?.space_used_size ?? 0; return { collectedAt: Date.now(), diff --git a/src/config/AgentConfig.ts b/src/config/AgentConfig.ts index 67424d9d..52e4359d 100644 --- a/src/config/AgentConfig.ts +++ b/src/config/AgentConfig.ts @@ -46,25 +46,20 @@ export type AgentConfig = { runtimeMetricsReporterActive?: boolean; runtimeMetricsCollectPeriod?: number; runtimeMetricsReportPeriod?: number; - runtimeMetricsBufferSize?: number; - runtimeMetricsMaxSnapshotsPerReport?: number; - runtimeMetricsHeapSpaceDetail?: boolean; + /** How often to include instance_nodejs_uptime in a report (ms). Default 30000. */ + runtimeMetricsUptimeReportPeriod?: number; /** @deprecated use runtimeMetricsReporterActive */ nvmMetricsReporterActive?: boolean; /** @deprecated use runtimeMetricsCollectPeriod */ nvmMetricsCollectPeriod?: number; /** @deprecated use runtimeMetricsReportPeriod */ nvmMetricsReportPeriod?: number; - /** @deprecated use runtimeMetricsBufferSize */ - nvmMetricsBufferSize?: number; /** @deprecated use runtimeMetricsReporterActive */ nvmJvmReporterActive?: boolean; /** @deprecated use runtimeMetricsCollectPeriod */ nvmJvmMetricsCollectPeriod?: number; /** @deprecated use runtimeMetricsReportPeriod */ nvmJvmMetricsReportPeriod?: number; - /** @deprecated use runtimeMetricsBufferSize */ - nvmJvmMetricsBufferSize?: number; }; export function normalizeDeprecatedRuntimeMetricOptions(options: AgentConfig): AgentConfig { @@ -97,15 +92,6 @@ export function normalizeDeprecatedRuntimeMetricOptions(options: AgentConfig): A delete normalized.nvmMetricsReportPeriod; delete normalized.nvmJvmMetricsReportPeriod; - if (normalized.runtimeMetricsBufferSize === undefined) { - const bufferSize = normalized.nvmMetricsBufferSize ?? normalized.nvmJvmMetricsBufferSize; - if (bufferSize !== undefined) { - normalized.runtimeMetricsBufferSize = bufferSize; - } - } - delete normalized.nvmMetricsBufferSize; - delete normalized.nvmJvmMetricsBufferSize; - return normalized; } @@ -116,8 +102,6 @@ function clearDeprecatedRuntimeMetricFields(config: AgentConfig): void { delete config.nvmJvmMetricsCollectPeriod; delete config.nvmMetricsReportPeriod; delete config.nvmJvmMetricsReportPeriod; - delete config.nvmMetricsBufferSize; - delete config.nvmJvmMetricsBufferSize; } function applyDeprecatedRuntimeMetricConfig(config: AgentConfig, options: AgentConfig = {}): void { @@ -145,14 +129,6 @@ function applyDeprecatedRuntimeMetricConfig(config: AgentConfig, options: AgentC } } - if (options.runtimeMetricsBufferSize === undefined) { - if (config.nvmMetricsBufferSize !== undefined) { - config.runtimeMetricsBufferSize = config.nvmMetricsBufferSize; - } else if (config.nvmJvmMetricsBufferSize !== undefined) { - config.runtimeMetricsBufferSize = config.nvmJvmMetricsBufferSize; - } - } - clearDeprecatedRuntimeMetricFields(config); } @@ -261,7 +237,7 @@ const _config = { process.env.SW_AGENT_NVM_JVM_REPORTER_ACTIVE; return configured?.toLowerCase() !== 'false'; })(), - runtimeMetricsCollectPeriod: ((n) => (Number.isSafeInteger(n) && n > 0 ? n : 1000))( + runtimeMetricsCollectPeriod: ((n) => (Number.isSafeInteger(n) && n > 0 ? n : 20000))( Number.parseInt( process.env.SW_AGENT_NODEJS_RUNTIME_METRICS_COLLECT_PERIOD ?? process.env.SW_AGENT_RUNTIME_METRICS_COLLECT_PERIOD ?? @@ -271,7 +247,7 @@ const _config = { 10, ), ), - runtimeMetricsReportPeriod: ((n) => (Number.isSafeInteger(n) && n > 0 ? n : 1000))( + runtimeMetricsReportPeriod: ((n) => (Number.isSafeInteger(n) && n > 0 ? n : 20000))( Number.parseInt( process.env.SW_AGENT_NODEJS_RUNTIME_METRICS_REPORT_PERIOD ?? process.env.SW_AGENT_RUNTIME_METRICS_REPORT_PERIOD ?? @@ -281,22 +257,8 @@ const _config = { 10, ), ), - runtimeMetricsHeapSpaceDetail: ((): boolean => { - const configured = process.env.SW_AGENT_RUNTIME_METRICS_HEAP_SPACE_DETAIL; - return configured?.toLowerCase() !== 'false'; - })(), - runtimeMetricsMaxSnapshotsPerReport: ((n) => (Number.isSafeInteger(n) && n > 0 ? n : 1))( - Number.parseInt(process.env.SW_AGENT_RUNTIME_METRICS_MAX_SNAPSHOTS_PER_REPORT ?? '', 10), - ), - runtimeMetricsBufferSize: ((n) => (Number.isSafeInteger(n) && n > 0 ? n : 600))( - Number.parseInt( - process.env.SW_AGENT_NODEJS_RUNTIME_METRICS_BUFFER_SIZE ?? - process.env.SW_AGENT_RUNTIME_METRICS_BUFFER_SIZE ?? - process.env.SW_AGENT_NVM_METRICS_BUFFER_SIZE ?? - process.env.SW_AGENT_NVM_JVM_METRICS_BUFFER_SIZE ?? - '', - 10, - ), + runtimeMetricsUptimeReportPeriod: ((n) => (Number.isSafeInteger(n) && n > 0 ? n : 30000))( + Number.parseInt(process.env.SW_AGENT_NODEJS_RUNTIME_METRICS_UPTIME_REPORT_PERIOD ?? '', 10), ), }; diff --git a/tests/config/AgentConfig.test.ts b/tests/config/AgentConfig.test.ts index 08b7fd3b..d52d54d9 100644 --- a/tests/config/AgentConfig.test.ts +++ b/tests/config/AgentConfig.test.ts @@ -63,17 +63,14 @@ import { AgentConfig, normalizeDeprecatedRuntimeMetricOptions } from '../../src/ function resetRuntimeMetricConfig(): void { const mutableConfig = config as AgentConfig; mutableConfig.runtimeMetricsReporterActive = true; - mutableConfig.runtimeMetricsCollectPeriod = 1000; - mutableConfig.runtimeMetricsReportPeriod = 1000; - mutableConfig.runtimeMetricsBufferSize = 600; + mutableConfig.runtimeMetricsCollectPeriod = 20000; + mutableConfig.runtimeMetricsReportPeriod = 20000; delete mutableConfig.nvmMetricsReporterActive; delete mutableConfig.nvmJvmReporterActive; delete mutableConfig.nvmMetricsCollectPeriod; delete mutableConfig.nvmJvmMetricsCollectPeriod; delete mutableConfig.nvmMetricsReportPeriod; delete mutableConfig.nvmJvmMetricsReportPeriod; - delete mutableConfig.nvmMetricsBufferSize; - delete mutableConfig.nvmJvmMetricsBufferSize; } describe('AgentConfig deprecated runtime metric options (unit)', () => { @@ -87,13 +84,11 @@ describe('AgentConfig deprecated runtime metric options (unit)', () => { nvmMetricsReporterActive: false, nvmMetricsCollectPeriod: 2000, nvmMetricsReportPeriod: 3000, - nvmMetricsBufferSize: 42, }); expect(normalized.runtimeMetricsReporterActive).toBe(false); expect(normalized.runtimeMetricsCollectPeriod).toBe(2000); expect(normalized.runtimeMetricsReportPeriod).toBe(3000); - expect(normalized.runtimeMetricsBufferSize).toBe(42); }); it('maps nvmJvm deprecated aliases before merge', () => { @@ -101,13 +96,11 @@ describe('AgentConfig deprecated runtime metric options (unit)', () => { nvmJvmReporterActive: false, nvmJvmMetricsCollectPeriod: 2222, nvmJvmMetricsReportPeriod: 3333, - nvmJvmMetricsBufferSize: 44, }); expect(normalized.runtimeMetricsReporterActive).toBe(false); expect(normalized.runtimeMetricsCollectPeriod).toBe(2222); expect(normalized.runtimeMetricsReportPeriod).toBe(3333); - expect(normalized.runtimeMetricsBufferSize).toBe(44); }); it('keeps explicit canonical options over deprecated aliases', () => { diff --git a/tests/plugins/express/client.ts b/tests/plugins/express/client.ts index ccc1bb49..31d1ffb7 100644 --- a/tests/plugins/express/client.ts +++ b/tests/plugins/express/client.ts @@ -24,6 +24,10 @@ import express from 'express'; agent.start({ serviceName: 'client', maxBufferSize: 1000, + // Plugin meter assertions need frequent reports within the test window. + runtimeMetricsCollectPeriod: 1000, + runtimeMetricsReportPeriod: 1000, + runtimeMetricsUptimeReportPeriod: 1000, }); const app = express(); @@ -33,12 +37,12 @@ app.use('/test', testRouter); testRouter.get('/express', (req, res) => { http - .request(`http://${process.env.SERVER || 'localhost:5000'}${req.url}`, (r) => { - let data = ''; - r.on('data', (chunk) => (data += chunk)); - r.on('end', () => res.send(data)); - }) - .end(); + .request(`http://${process.env.SERVER || 'localhost:5000'}${req.url}`, (r) => { + let data = ''; + r.on('data', (chunk) => (data += chunk)); + r.on('end', () => res.send(data)); + }) + .end(); }); app.listen(5001, () => console.info('Listening on port 5001...')); diff --git a/tests/plugins/express/expected.data.yaml b/tests/plugins/express/expected.data.yaml index a3235b38..fb3d55af 100644 --- a/tests/plugins/express/expected.data.yaml +++ b/tests/plugins/express/expected.data.yaml @@ -163,7 +163,7 @@ meterItems: - meterId: name: instance_nodejs_malloced_memory tags: [] - singleValue: gt 0 + singleValue: ge 0 - meterId: name: instance_nodejs_old_space_used tags: [] @@ -171,7 +171,7 @@ meterItems: - meterId: name: instance_nodejs_new_space_used tags: [] - singleValue: gt 0 + singleValue: ge 0 - serviceName: client meterSize: 12 meters: @@ -214,7 +214,7 @@ meterItems: - meterId: name: instance_nodejs_malloced_memory tags: [] - singleValue: gt 0 + singleValue: ge 0 - meterId: name: instance_nodejs_old_space_used tags: [] @@ -222,4 +222,4 @@ meterItems: - meterId: name: instance_nodejs_new_space_used tags: [] - singleValue: gt 0 + singleValue: ge 0 diff --git a/tests/plugins/express/server.ts b/tests/plugins/express/server.ts index 4e1dc421..06dbcb5b 100644 --- a/tests/plugins/express/server.ts +++ b/tests/plugins/express/server.ts @@ -25,6 +25,10 @@ import express from 'express'; agent.start({ serviceName: 'server', maxBufferSize: 1000, + // Plugin meter assertions need frequent reports within the test window. + runtimeMetricsCollectPeriod: 1000, + runtimeMetricsReportPeriod: 1000, + runtimeMetricsUptimeReportPeriod: 1000, }); const app = express(); diff --git a/tests/remote/MeterSender.test.ts b/tests/remote/MeterSender.test.ts index a2cf7809..55b41caa 100644 --- a/tests/remote/MeterSender.test.ts +++ b/tests/remote/MeterSender.test.ts @@ -21,7 +21,6 @@ import { GRPCChannelStatus } from '../../src/agent/core/remote/GRPCChannelStatus'; -const mockCollect = jest.fn(); let pendingCollectCallback: ((error: Error | null) => void) | undefined; let sampleSequence = 0; @@ -31,11 +30,11 @@ const mockChannelManager = { reportError: jest.fn(), }; -const mockMeterData = { +const createMeterData = () => ({ setService: jest.fn().mockReturnThis(), setServiceinstance: jest.fn().mockReturnThis(), setTimestamp: jest.fn().mockReturnThis(), -}; +}); const mockStream = { write: jest.fn(), @@ -52,8 +51,6 @@ jest.mock('../../src/config/AgentConfig', () => ({ traceTimeout: 10000, runtimeMetricsCollectPeriod: 1000, runtimeMetricsReportPeriod: 1000, - runtimeMetricsBufferSize: 600, - runtimeMetricsMaxSnapshotsPerReport: 1, }, })); @@ -69,7 +66,7 @@ jest.mock('../../src/proto/language-agent/Meter_grpc_pb', () => ({ jest.mock('../../src/agent/core/meter/RuntimeMetricsCollector', () => { return jest.fn().mockImplementation(() => ({ sample: jest.fn(() => mockSnapshot()), - toMeterData: jest.fn(() => [mockMeterData, { ...mockMeterData }]), + toMeterData: jest.fn(() => [createMeterData(), createMeterData()]), destroy: jest.fn(), })); }); @@ -102,11 +99,7 @@ describe('MeterSender', () => { beforeEach(() => { jest.useFakeTimers(); sampleSequence = 0; - mockCollect.mockClear(); mockChannelManager.reportError.mockClear(); - mockMeterData.setService.mockClear(); - mockMeterData.setServiceinstance.mockClear(); - mockMeterData.setTimestamp.mockClear(); mockStream.write.mockReset(); mockStream.end.mockReset(); pendingCollectCallback = undefined; @@ -123,40 +116,57 @@ describe('MeterSender', () => { it('clears reporter stub on DISCONNECT', () => { sender.statusChanged(GRPCChannelStatus.DISCONNECT); + expect((sender as unknown as { reporterClient?: unknown }).reporterClient).toBeUndefined(); expect(MeterReportServiceClient).toHaveBeenCalledTimes(1); }); - it('uses per-snapshot collectedAt timestamps', async () => { - const senderAny = sender as unknown as { buffer: Array<{ collectedAt: number }> }; - senderAny.buffer.push({ collectedAt: 1_111_111 } as never, { collectedAt: 2_222_222 } as never); - - const firstReport = (sender as unknown as { reportBufferedMetrics: () => Promise }).reportBufferedMetrics(); - await Promise.resolve(); - pendingCollectCallback?.(null); - await firstReport; + it('uses collectedAt on the first stream element only', async () => { + const meterA = createMeterData(); + const meterB = createMeterData(); + const collector = (sender as unknown as { collector: { toMeterData: jest.Mock } }).collector; + collector.toMeterData.mockReturnValueOnce([meterA, meterB]); + (sender as unknown as { latestSnapshot?: { collectedAt: number } }).latestSnapshot = { + collectedAt: 1_111_111, + }; - const secondReport = (sender as unknown as { reportBufferedMetrics: () => Promise }).reportBufferedMetrics(); + const reportPromise = (sender as unknown as { reportBufferedMetrics: () => Promise }).reportBufferedMetrics(); await Promise.resolve(); pendingCollectCallback?.(null); - await secondReport; + await reportPromise; - const timestamps = mockMeterData.setTimestamp.mock.calls.map((call) => call[0]); - expect(timestamps).toEqual(expect.arrayContaining([1_111_111, 2_222_222])); + expect(meterA.setService).toHaveBeenCalledWith('meter-service'); + expect(meterA.setServiceinstance).toHaveBeenCalledWith('meter-instance'); + expect(meterA.setTimestamp).toHaveBeenCalledWith(1_111_111); + expect(meterB.setService).not.toHaveBeenCalled(); + expect(meterB.setServiceinstance).not.toHaveBeenCalled(); + expect(meterB.setTimestamp).not.toHaveBeenCalled(); + expect(mockStream.write).toHaveBeenCalledTimes(2); }); - it('drains at most one snapshot per report tick by default', async () => { - const senderAny = sender as unknown as { buffer: Array<{ collectedAt: number }> }; - senderAny.buffer.push({ collectedAt: 1 } as never, { collectedAt: 2 } as never, { collectedAt: 3 } as never); + it('reports only the latest snapshot and discards older ones', async () => { + (sender as unknown as { latestSnapshot?: { collectedAt: number } }).latestSnapshot = { + collectedAt: 3, + }; const reportPromise = (sender as unknown as { reportBufferedMetrics: () => Promise }).reportBufferedMetrics(); await Promise.resolve(); pendingCollectCallback?.(null); await reportPromise; - expect(senderAny.buffer.length).toBe(2); + expect((sender as unknown as { latestSnapshot?: unknown }).latestSnapshot).toBeUndefined(); expect(mockStream.write).toHaveBeenCalled(); }); + it('keeps overwriting with the newest sample between reports', () => { + const collector = (sender as unknown as { collector: { sample: jest.Mock } }).collector; + collector.sample.mockReturnValueOnce({ collectedAt: 10 }).mockReturnValueOnce({ collectedAt: 20 }); + + (sender as unknown as { collectSample: () => void }).collectSample(); + (sender as unknown as { collectSample: () => void }).collectSample(); + + expect((sender as unknown as { latestSnapshot?: { collectedAt: number } }).latestSnapshot?.collectedAt).toBe(20); + }); + it('skips duplicate boot timers', () => { const collectTimer = (sender as unknown as { collectTimer?: NodeJS.Timeout }).collectTimer; const reportTimer = (sender as unknown as { reportTimer?: NodeJS.Timeout }).reportTimer; diff --git a/tests/runtime/RuntimeMetricsCollector.test.ts b/tests/runtime/RuntimeMetricsCollector.test.ts index f9e437ab..ab24cc76 100644 --- a/tests/runtime/RuntimeMetricsCollector.test.ts +++ b/tests/runtime/RuntimeMetricsCollector.test.ts @@ -19,6 +19,7 @@ /* eslint-env jest */ +import config from '../../src/config/AgentConfig'; import RuntimeMetricsCollector from '../../src/agent/core/meter/RuntimeMetricsCollector'; import { RuntimeSnapshot } from '../../src/agent/core/meter/RuntimeSampler'; @@ -37,15 +38,41 @@ const EXPECTED_METER_NAMES = [ 'instance_nodejs_new_space_used', ]; +const EXPECTED_METER_NAMES_WITHOUT_UPTIME = EXPECTED_METER_NAMES.filter((name) => name !== 'instance_nodejs_uptime'); + +function baseSnapshot(overrides: Partial = {}): RuntimeSnapshot { + return { + collectedAt: 1_700_000_000_000, + heapUsed: 100, + heapTotal: 200, + heapSizeLimit: 300, + rss: 400, + external: 50, + cpuUserPercent: 1.2, + cpuSystemPercent: 0.8, + arrayBuffers: 16, + uptime: 42.5, + peakMallocedMemory: 2048, + mallocedMemory: 3072, + oldSpaceUsed: 88, + newSpaceUsed: 12, + ...overrides, + }; +} + describe('RuntimeMetricsCollector', () => { let collector: RuntimeMetricsCollector; + let originalUptimePeriod: number | undefined; beforeEach(() => { collector = new RuntimeMetricsCollector(); + originalUptimePeriod = config.runtimeMetricsUptimeReportPeriod; + config.runtimeMetricsUptimeReportPeriod = 30000; }); afterEach(() => { collector.destroy(); + config.runtimeMetricsUptimeReportPeriod = originalUptimePeriod; }); it('maps Node.js runtime data into nodejs meter fields', () => { @@ -65,24 +92,7 @@ describe('RuntimeMetricsCollector', () => { }); it('maps extended runtime snapshot values into meter single values', () => { - const snapshot: RuntimeSnapshot = { - collectedAt: 1_700_000_000_000, - heapUsed: 100, - heapTotal: 200, - heapSizeLimit: 300, - rss: 400, - external: 50, - cpuUserPercent: 1.2, - cpuSystemPercent: 0.8, - arrayBuffers: 16, - uptime: 42.5, - peakMallocedMemory: 2048, - mallocedMemory: 3072, - oldSpaceUsed: 88, - newSpaceUsed: 12, - }; - - const meters = collector.toMeterData(snapshot); + const meters = collector.toMeterData(baseSnapshot()); const values: Record = {}; for (const meter of meters) { const name = meter.getSinglevalue()?.getName(); @@ -106,4 +116,15 @@ describe('RuntimeMetricsCollector', () => { instance_nodejs_new_space_used: 12, }); }); + + it('includes uptime on the first report then omits until the uptime period elapses', () => { + const first = collector.toMeterData(baseSnapshot({ collectedAt: 1_000 })); + expect(first.map((m) => m.getSinglevalue()?.getName())).toEqual(EXPECTED_METER_NAMES); + + const second = collector.toMeterData(baseSnapshot({ collectedAt: 1_000 + 29_999 })); + expect(second.map((m) => m.getSinglevalue()?.getName())).toEqual(EXPECTED_METER_NAMES_WITHOUT_UPTIME); + + const third = collector.toMeterData(baseSnapshot({ collectedAt: 1_000 + 30_000 })); + expect(third.map((m) => m.getSinglevalue()?.getName())).toEqual(EXPECTED_METER_NAMES); + }); }); diff --git a/tests/runtime/RuntimeSampler.test.ts b/tests/runtime/RuntimeSampler.test.ts index 1979084b..44e50152 100644 --- a/tests/runtime/RuntimeSampler.test.ts +++ b/tests/runtime/RuntimeSampler.test.ts @@ -111,25 +111,6 @@ describe('RuntimeSampler', () => { jest.restoreAllMocks(); }); - it('skips heap space statistics when heap space detail is disabled', () => { - const config = require('../../src/config/AgentConfig').default as { - runtimeMetricsHeapSpaceDetail?: boolean; - }; - const original = config.runtimeMetricsHeapSpaceDetail; - config.runtimeMetricsHeapSpaceDetail = false; - const heapSpaceSpy = jest.spyOn(v8, 'getHeapSpaceStatistics'); - - try { - const snapshot = sampler.sample(); - expect(heapSpaceSpy).not.toHaveBeenCalled(); - expect(snapshot.oldSpaceUsed).toBe(0); - expect(snapshot.newSpaceUsed).toBe(0); - } finally { - config.runtimeMetricsHeapSpaceDetail = original; - heapSpaceSpy.mockRestore(); - } - }); - it('defaults missing heap spaces to zero', () => { jest.spyOn(v8, 'getHeapSpaceStatistics').mockReturnValue([]); From 99f1df96c0ac2b99fb2d5f2cf65e3eddcf97e488 Mon Sep 17 00:00:00 2001 From: songzhendong <289505773@qq.com> Date: Thu, 6 Aug 2026 07:44:16 +0800 Subject: [PATCH 3/4] fix(ci): restore TestUnit name and drop express indent noise Address follow-up review: rename TestUnitRemote to TestUnit with plugin path exclusion, and revert unrelated express client.ts reindent. --- .github/workflows/test.yaml | 4 ++-- tests/plugins/express/client.ts | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index dc60c1c7..ed530773 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -47,7 +47,7 @@ jobs: npm i npm run lint - TestUnitRemote: + TestUnit: runs-on: ubuntu-latest timeout-minutes: 10 strategy: @@ -73,7 +73,7 @@ jobs: - name: Unit tests on Node@${{ matrix.node-version }} run: | npm i - npx jest tests/remote/ tests/config/ tests/runtime/ --runInBand + npx jest --testPathIgnorePatterns "/tests/plugins/" --runInBand build-matrix: runs-on: ubuntu-latest diff --git a/tests/plugins/express/client.ts b/tests/plugins/express/client.ts index 31d1ffb7..7dcd4c09 100644 --- a/tests/plugins/express/client.ts +++ b/tests/plugins/express/client.ts @@ -37,12 +37,12 @@ app.use('/test', testRouter); testRouter.get('/express', (req, res) => { http - .request(`http://${process.env.SERVER || 'localhost:5000'}${req.url}`, (r) => { - let data = ''; - r.on('data', (chunk) => (data += chunk)); - r.on('end', () => res.send(data)); - }) - .end(); + .request(`http://${process.env.SERVER || 'localhost:5000'}${req.url}`, (r) => { + let data = ''; + r.on('data', (chunk) => (data += chunk)); + r.on('end', () => res.send(data)); + }) + .end(); }); app.listen(5001, () => console.info('Listening on port 5001...')); From cf42798ebf97a11248340c706d99b5fe15612799 Mon Sep 17 00:00:00 2001 From: songzhendong <289505773@qq.com> Date: Fri, 7 Aug 2026 08:25:01 +0800 Subject: [PATCH 4/4] fix(runtime): use one meter timer and restore node_modules CI ignore Collapse collect/report into a single report-period timer so CPU deltas match the publish interval, always include uptime with other gauges, and keep Jest from replacing the default node_modules ignore pattern. --- .github/workflows/test.yaml | 2 +- README.md | 12 +++--- src/agent/core/meter/MeterSender.ts | 31 +++++--------- .../core/meter/RuntimeMetricsCollector.ts | 19 +-------- src/config/AgentConfig.ts | 40 +------------------ tests/config/AgentConfig.test.ts | 7 ---- tests/plugins/express/client.ts | 2 - tests/plugins/express/server.ts | 2 - tests/remote/MeterSender.test.ts | 20 +++++++--- tests/runtime/RuntimeMetricsCollector.test.ts | 16 ++------ 10 files changed, 36 insertions(+), 115 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index ed530773..4b0fe787 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -73,7 +73,7 @@ jobs: - name: Unit tests on Node@${{ matrix.node-version }} run: | npm i - npx jest --testPathIgnorePatterns "/tests/plugins/" --runInBand + npx jest --testPathIgnorePatterns "/node_modules/" "/tests/plugins/" --runInBand build-matrix: runs-on: ubuntu-latest diff --git a/README.md b/README.md index 29d89f7a..bc31f3c5 100644 --- a/README.md +++ b/README.md @@ -72,19 +72,17 @@ Environment Variable | Description | Default | `SW_AWS_SQS_CHECK_BODY` | Incoming SQS messages check inside the body for trace ID in order to allow linking outgoing SNS messages to incoming SQS. | `false` | | `SW_AGENT_MAX_BUFFER_SIZE` | The maximum buffer size before sending the segment data to backend | `'1000'` | | `SW_AGENT_TRACE_TIMEOUT` | The timeout for trace requests to backend services | `'10000'` | -| `SW_AGENT_NODEJS_RUNTIME_METRICS_REPORTER_ACTIVE` | Whether to report Node.js runtime metrics through MeterReportService (default: collect 20s, report 20s) | `true` | -| `SW_AGENT_NODEJS_RUNTIME_METRICS_COLLECT_PERIOD` | Runtime metric sample interval in milliseconds | `20000` | -| `SW_AGENT_NODEJS_RUNTIME_METRICS_REPORT_PERIOD` | Runtime metric report interval in milliseconds (aligned with Java `meter.report_interval`) | `20000` | -| `SW_AGENT_NODEJS_RUNTIME_METRICS_UPTIME_REPORT_PERIOD` | How often to include `instance_nodejs_uptime` in a report (ms); other runtime meters still follow the report period | `30000` | +| `SW_AGENT_NODEJS_RUNTIME_METRICS_REPORTER_ACTIVE` | Whether to report Node.js runtime metrics through MeterReportService (default period 20s) | `true` | +| `SW_AGENT_NODEJS_RUNTIME_METRICS_REPORT_PERIOD` | Runtime metric sample + report interval in milliseconds (aligned with Java `meter.report_interval`) | `20000` | -Legacy env names `SW_AGENT_RUNTIME_METRICS_*` / `SW_AGENT_NVM_*` for reporter active, collect period, and report period are still accepted as deprecated aliases. +Legacy env names `SW_AGENT_RUNTIME_METRICS_*` / `SW_AGENT_NVM_*` for reporter active and report period are still accepted as deprecated aliases. Note that the various ignore options like `SW_IGNORE_SUFFIX`, `SW_TRACE_IGNORE_PATH` and `SW_HTTP_IGNORE_METHOD` as well as endpoints which are not recorded due to exceeding `SW_AGENT_MAX_BUFFER_SIZE` all propagate their ignored status downstream to any other endpoints they may call. If that endpoint is running the Node Skywalking agent then regardless of its ignore settings it will not be recorded since its upstream parent was not recorded. This allows the elimination of entire trees of endpoints you are not interested in as well as eliminating partial traces if a span in the chain is ignored but calls out to other endpoints which are recorded as children of ROOT instead of the actual parent. ## Node.js Runtime Metrics -The agent reports twelve process-level meters (`instance_nodejs_*`) via `MeterReportService` by default (collect 20s, report 20s; `instance_nodejs_uptime` every 30s). Set `SW_AGENT_NODEJS_RUNTIME_METRICS_REPORTER_ACTIVE=false` to disable. Process CPU combines `process.cpuUsage()` user + system, normalized by logical CPU count (0–100%). +The agent reports twelve process-level meters (`instance_nodejs_*`) via `MeterReportService` by default (sample and report every 20s). Set `SW_AGENT_NODEJS_RUNTIME_METRICS_REPORTER_ACTIVE=false` to disable. Process CPU combines `process.cpuUsage()` user + system, normalized by logical CPU count (0–100%). | Node.js source | Meter name | Notes | | :--- | :--- | :--- | @@ -95,7 +93,7 @@ The agent reports twelve process-level meters (`instance_nodejs_*`) via `MeterRe | `process.memoryUsage().rss` | `instance_nodejs_rss` | bytes | | `process.memoryUsage().external` | `instance_nodejs_external_memory` | bytes | | `process.memoryUsage().arrayBuffers` | `instance_nodejs_array_buffers` | bytes | -| `process.uptime()` | `instance_nodejs_uptime` | seconds; included every 30s by default | +| `process.uptime()` | `instance_nodejs_uptime` | seconds | | `v8.getHeapStatistics().peak_malloced_memory` | `instance_nodejs_peak_malloced_memory` | bytes | | `v8.getHeapStatistics().malloced_memory` | `instance_nodejs_malloced_memory` | bytes | | `v8.getHeapSpaceStatistics()` old_space | `instance_nodejs_old_space_used` | bytes | diff --git a/src/agent/core/meter/MeterSender.ts b/src/agent/core/meter/MeterSender.ts index a0cb873b..93b8d713 100644 --- a/src/agent/core/meter/MeterSender.ts +++ b/src/agent/core/meter/MeterSender.ts @@ -40,8 +40,7 @@ export default class MeterSender implements BootService, GRPCChannelListener { private reporterClient?: MeterReportServiceClient; /** Latest gauge snapshot only — stale samples have no value after reconnect. */ private latestSnapshot?: RuntimeSnapshot; - private collectTimer?: NodeJS.Timeout; - private reportTimer?: NodeJS.Timeout; + private timer?: NodeJS.Timeout; private reporting?: Promise; private collector!: RuntimeMetricsCollector; @@ -53,12 +52,12 @@ export default class MeterSender implements BootService, GRPCChannelListener { } boot(): void { - if (this.collectTimer || this.reportTimer) { - logger.warn('MeterSender timers already scheduled; skipping duplicate boot.'); + if (this.timer) { + logger.warn('MeterSender timer already scheduled; skipping duplicate boot.'); return; } - this.startTimers(); + this.startTimer(); } onComplete(): void {} @@ -84,21 +83,15 @@ export default class MeterSender implements BootService, GRPCChannelListener { ); } - private startTimers(): void { - this.collectTimer = setInterval(() => { + private startTimer(): void { + this.timer = setInterval(() => { if (this.closed) { return; } this.collectSample(); - }, config.runtimeMetricsCollectPeriod || 20000) as NodeJS.Timeout; - this.collectTimer.unref(); - this.reportTimer = setInterval(() => { - if (this.closed) { - return; - } void this.reportBufferedMetrics(); }, config.runtimeMetricsReportPeriod || 20000) as NodeJS.Timeout; - this.reportTimer.unref(); + this.timer.unref(); } private collectSample(): void { @@ -200,13 +193,9 @@ export default class MeterSender implements BootService, GRPCChannelListener { shutdown(): void { this.closed = true; - if (this.collectTimer) { - clearInterval(this.collectTimer); - this.collectTimer = undefined; - } - if (this.reportTimer) { - clearInterval(this.reportTimer); - this.reportTimer = undefined; + if (this.timer) { + clearInterval(this.timer); + this.timer = undefined; } this.reporting = undefined; this.reporterClient = undefined; diff --git a/src/agent/core/meter/RuntimeMetricsCollector.ts b/src/agent/core/meter/RuntimeMetricsCollector.ts index 05b9027a..af288d1e 100644 --- a/src/agent/core/meter/RuntimeMetricsCollector.ts +++ b/src/agent/core/meter/RuntimeMetricsCollector.ts @@ -17,17 +17,12 @@ * */ -import config from '../../../config/AgentConfig'; import { MeterData, MeterSingleValue } from '../../../proto/language-agent/Meter_pb'; import RuntimeSampler, { RuntimeSnapshot } from './RuntimeSampler'; -const DEFAULT_UPTIME_REPORT_PERIOD_MS = 30000; - /** Maps Node.js runtime samples into MeterReportService single-value meters (instance_nodejs_*). */ export default class RuntimeMetricsCollector { private readonly sampler = new RuntimeSampler(); - /** collectedAt of the last report that included instance_nodejs_uptime. */ - private lastUptimeReportAt = 0; sample(): RuntimeSnapshot { return this.sampler.sample(); @@ -42,30 +37,18 @@ export default class RuntimeMetricsCollector { ['instance_nodejs_rss', snapshot.rss], ['instance_nodejs_external_memory', snapshot.external], ['instance_nodejs_array_buffers', snapshot.arrayBuffers], + ['instance_nodejs_uptime', snapshot.uptime], ['instance_nodejs_peak_malloced_memory', snapshot.peakMallocedMemory], ['instance_nodejs_malloced_memory', snapshot.mallocedMemory], ['instance_nodejs_old_space_used', snapshot.oldSpaceUsed], ['instance_nodejs_new_space_used', snapshot.newSpaceUsed], ]; - if (this.shouldIncludeUptime(snapshot.collectedAt)) { - gauges.splice(7, 0, ['instance_nodejs_uptime', snapshot.uptime]); - this.lastUptimeReportAt = snapshot.collectedAt; - } - return gauges.map(([name, value]) => new MeterData().setSinglevalue(new MeterSingleValue().setName(name).setValue(value)), ); } - private shouldIncludeUptime(collectedAt: number): boolean { - if (this.lastUptimeReportAt === 0) { - return true; - } - const period = config.runtimeMetricsUptimeReportPeriod || DEFAULT_UPTIME_REPORT_PERIOD_MS; - return collectedAt - this.lastUptimeReportAt >= period; - } - destroy(): void { this.sampler.destroy(); } diff --git a/src/config/AgentConfig.ts b/src/config/AgentConfig.ts index 52e4359d..9bea2173 100644 --- a/src/config/AgentConfig.ts +++ b/src/config/AgentConfig.ts @@ -44,20 +44,14 @@ export type AgentConfig = { reHttpIgnoreMethod?: RegExp; traceTimeout?: number; runtimeMetricsReporterActive?: boolean; - runtimeMetricsCollectPeriod?: number; + /** Sample + report interval for runtime meters (ms). Default 20000. */ runtimeMetricsReportPeriod?: number; - /** How often to include instance_nodejs_uptime in a report (ms). Default 30000. */ - runtimeMetricsUptimeReportPeriod?: number; /** @deprecated use runtimeMetricsReporterActive */ nvmMetricsReporterActive?: boolean; - /** @deprecated use runtimeMetricsCollectPeriod */ - nvmMetricsCollectPeriod?: number; /** @deprecated use runtimeMetricsReportPeriod */ nvmMetricsReportPeriod?: number; /** @deprecated use runtimeMetricsReporterActive */ nvmJvmReporterActive?: boolean; - /** @deprecated use runtimeMetricsCollectPeriod */ - nvmJvmMetricsCollectPeriod?: number; /** @deprecated use runtimeMetricsReportPeriod */ nvmJvmMetricsReportPeriod?: number; }; @@ -74,15 +68,6 @@ export function normalizeDeprecatedRuntimeMetricOptions(options: AgentConfig): A delete normalized.nvmMetricsReporterActive; delete normalized.nvmJvmReporterActive; - if (normalized.runtimeMetricsCollectPeriod === undefined) { - const collectPeriod = normalized.nvmMetricsCollectPeriod ?? normalized.nvmJvmMetricsCollectPeriod; - if (collectPeriod !== undefined) { - normalized.runtimeMetricsCollectPeriod = collectPeriod; - } - } - delete normalized.nvmMetricsCollectPeriod; - delete normalized.nvmJvmMetricsCollectPeriod; - if (normalized.runtimeMetricsReportPeriod === undefined) { const reportPeriod = normalized.nvmMetricsReportPeriod ?? normalized.nvmJvmMetricsReportPeriod; if (reportPeriod !== undefined) { @@ -98,8 +83,6 @@ export function normalizeDeprecatedRuntimeMetricOptions(options: AgentConfig): A function clearDeprecatedRuntimeMetricFields(config: AgentConfig): void { delete config.nvmMetricsReporterActive; delete config.nvmJvmReporterActive; - delete config.nvmMetricsCollectPeriod; - delete config.nvmJvmMetricsCollectPeriod; delete config.nvmMetricsReportPeriod; delete config.nvmJvmMetricsReportPeriod; } @@ -113,14 +96,6 @@ function applyDeprecatedRuntimeMetricConfig(config: AgentConfig, options: AgentC } } - if (options.runtimeMetricsCollectPeriod === undefined) { - if (config.nvmMetricsCollectPeriod !== undefined) { - config.runtimeMetricsCollectPeriod = config.nvmMetricsCollectPeriod; - } else if (config.nvmJvmMetricsCollectPeriod !== undefined) { - config.runtimeMetricsCollectPeriod = config.nvmJvmMetricsCollectPeriod; - } - } - if (options.runtimeMetricsReportPeriod === undefined) { if (config.nvmMetricsReportPeriod !== undefined) { config.runtimeMetricsReportPeriod = config.nvmMetricsReportPeriod; @@ -237,16 +212,6 @@ const _config = { process.env.SW_AGENT_NVM_JVM_REPORTER_ACTIVE; return configured?.toLowerCase() !== 'false'; })(), - runtimeMetricsCollectPeriod: ((n) => (Number.isSafeInteger(n) && n > 0 ? n : 20000))( - Number.parseInt( - process.env.SW_AGENT_NODEJS_RUNTIME_METRICS_COLLECT_PERIOD ?? - process.env.SW_AGENT_RUNTIME_METRICS_COLLECT_PERIOD ?? - process.env.SW_AGENT_NVM_METRICS_COLLECT_PERIOD ?? - process.env.SW_AGENT_NVM_JVM_METRICS_COLLECT_PERIOD ?? - '', - 10, - ), - ), runtimeMetricsReportPeriod: ((n) => (Number.isSafeInteger(n) && n > 0 ? n : 20000))( Number.parseInt( process.env.SW_AGENT_NODEJS_RUNTIME_METRICS_REPORT_PERIOD ?? @@ -257,9 +222,6 @@ const _config = { 10, ), ), - runtimeMetricsUptimeReportPeriod: ((n) => (Number.isSafeInteger(n) && n > 0 ? n : 30000))( - Number.parseInt(process.env.SW_AGENT_NODEJS_RUNTIME_METRICS_UPTIME_REPORT_PERIOD ?? '', 10), - ), }; export default _config; diff --git a/tests/config/AgentConfig.test.ts b/tests/config/AgentConfig.test.ts index d52d54d9..c0654512 100644 --- a/tests/config/AgentConfig.test.ts +++ b/tests/config/AgentConfig.test.ts @@ -63,12 +63,9 @@ import { AgentConfig, normalizeDeprecatedRuntimeMetricOptions } from '../../src/ function resetRuntimeMetricConfig(): void { const mutableConfig = config as AgentConfig; mutableConfig.runtimeMetricsReporterActive = true; - mutableConfig.runtimeMetricsCollectPeriod = 20000; mutableConfig.runtimeMetricsReportPeriod = 20000; delete mutableConfig.nvmMetricsReporterActive; delete mutableConfig.nvmJvmReporterActive; - delete mutableConfig.nvmMetricsCollectPeriod; - delete mutableConfig.nvmJvmMetricsCollectPeriod; delete mutableConfig.nvmMetricsReportPeriod; delete mutableConfig.nvmJvmMetricsReportPeriod; } @@ -82,24 +79,20 @@ describe('AgentConfig deprecated runtime metric options (unit)', () => { it('maps deprecated programmatic aliases before merge', () => { const normalized = normalizeDeprecatedRuntimeMetricOptions({ nvmMetricsReporterActive: false, - nvmMetricsCollectPeriod: 2000, nvmMetricsReportPeriod: 3000, }); expect(normalized.runtimeMetricsReporterActive).toBe(false); - expect(normalized.runtimeMetricsCollectPeriod).toBe(2000); expect(normalized.runtimeMetricsReportPeriod).toBe(3000); }); it('maps nvmJvm deprecated aliases before merge', () => { const normalized = normalizeDeprecatedRuntimeMetricOptions({ nvmJvmReporterActive: false, - nvmJvmMetricsCollectPeriod: 2222, nvmJvmMetricsReportPeriod: 3333, }); expect(normalized.runtimeMetricsReporterActive).toBe(false); - expect(normalized.runtimeMetricsCollectPeriod).toBe(2222); expect(normalized.runtimeMetricsReportPeriod).toBe(3333); }); diff --git a/tests/plugins/express/client.ts b/tests/plugins/express/client.ts index 7dcd4c09..6088798e 100644 --- a/tests/plugins/express/client.ts +++ b/tests/plugins/express/client.ts @@ -25,9 +25,7 @@ agent.start({ serviceName: 'client', maxBufferSize: 1000, // Plugin meter assertions need frequent reports within the test window. - runtimeMetricsCollectPeriod: 1000, runtimeMetricsReportPeriod: 1000, - runtimeMetricsUptimeReportPeriod: 1000, }); const app = express(); diff --git a/tests/plugins/express/server.ts b/tests/plugins/express/server.ts index 06dbcb5b..a2aa011f 100644 --- a/tests/plugins/express/server.ts +++ b/tests/plugins/express/server.ts @@ -26,9 +26,7 @@ agent.start({ serviceName: 'server', maxBufferSize: 1000, // Plugin meter assertions need frequent reports within the test window. - runtimeMetricsCollectPeriod: 1000, runtimeMetricsReportPeriod: 1000, - runtimeMetricsUptimeReportPeriod: 1000, }); const app = express(); diff --git a/tests/remote/MeterSender.test.ts b/tests/remote/MeterSender.test.ts index 55b41caa..c1a3018d 100644 --- a/tests/remote/MeterSender.test.ts +++ b/tests/remote/MeterSender.test.ts @@ -49,7 +49,6 @@ jest.mock('../../src/config/AgentConfig', () => ({ serviceName: 'meter-service', serviceInstance: 'meter-instance', traceTimeout: 10000, - runtimeMetricsCollectPeriod: 1000, runtimeMetricsReportPeriod: 1000, }, })); @@ -168,10 +167,21 @@ describe('MeterSender', () => { }); it('skips duplicate boot timers', () => { - const collectTimer = (sender as unknown as { collectTimer?: NodeJS.Timeout }).collectTimer; - const reportTimer = (sender as unknown as { reportTimer?: NodeJS.Timeout }).reportTimer; + const timer = (sender as unknown as { timer?: NodeJS.Timeout }).timer; sender.boot(); - expect((sender as unknown as { collectTimer?: NodeJS.Timeout }).collectTimer).toBe(collectTimer); - expect((sender as unknown as { reportTimer?: NodeJS.Timeout }).reportTimer).toBe(reportTimer); + expect((sender as unknown as { timer?: NodeJS.Timeout }).timer).toBe(timer); + }); + + it('collects then reports on the same timer tick', async () => { + const collector = (sender as unknown as { collector: { sample: jest.Mock } }).collector; + collector.sample.mockReturnValueOnce({ collectedAt: 42 }); + + jest.advanceTimersByTime(1000); + await Promise.resolve(); + pendingCollectCallback?.(null); + await Promise.resolve(); + + expect(collector.sample).toHaveBeenCalled(); + expect(mockStream.write).toHaveBeenCalled(); }); }); diff --git a/tests/runtime/RuntimeMetricsCollector.test.ts b/tests/runtime/RuntimeMetricsCollector.test.ts index ab24cc76..a260704d 100644 --- a/tests/runtime/RuntimeMetricsCollector.test.ts +++ b/tests/runtime/RuntimeMetricsCollector.test.ts @@ -19,7 +19,6 @@ /* eslint-env jest */ -import config from '../../src/config/AgentConfig'; import RuntimeMetricsCollector from '../../src/agent/core/meter/RuntimeMetricsCollector'; import { RuntimeSnapshot } from '../../src/agent/core/meter/RuntimeSampler'; @@ -38,8 +37,6 @@ const EXPECTED_METER_NAMES = [ 'instance_nodejs_new_space_used', ]; -const EXPECTED_METER_NAMES_WITHOUT_UPTIME = EXPECTED_METER_NAMES.filter((name) => name !== 'instance_nodejs_uptime'); - function baseSnapshot(overrides: Partial = {}): RuntimeSnapshot { return { collectedAt: 1_700_000_000_000, @@ -62,17 +59,13 @@ function baseSnapshot(overrides: Partial = {}): RuntimeSnapshot describe('RuntimeMetricsCollector', () => { let collector: RuntimeMetricsCollector; - let originalUptimePeriod: number | undefined; beforeEach(() => { collector = new RuntimeMetricsCollector(); - originalUptimePeriod = config.runtimeMetricsUptimeReportPeriod; - config.runtimeMetricsUptimeReportPeriod = 30000; }); afterEach(() => { collector.destroy(); - config.runtimeMetricsUptimeReportPeriod = originalUptimePeriod; }); it('maps Node.js runtime data into nodejs meter fields', () => { @@ -117,14 +110,11 @@ describe('RuntimeMetricsCollector', () => { }); }); - it('includes uptime on the first report then omits until the uptime period elapses', () => { + it('includes uptime with every report', () => { const first = collector.toMeterData(baseSnapshot({ collectedAt: 1_000 })); expect(first.map((m) => m.getSinglevalue()?.getName())).toEqual(EXPECTED_METER_NAMES); - const second = collector.toMeterData(baseSnapshot({ collectedAt: 1_000 + 29_999 })); - expect(second.map((m) => m.getSinglevalue()?.getName())).toEqual(EXPECTED_METER_NAMES_WITHOUT_UPTIME); - - const third = collector.toMeterData(baseSnapshot({ collectedAt: 1_000 + 30_000 })); - expect(third.map((m) => m.getSinglevalue()?.getName())).toEqual(EXPECTED_METER_NAMES); + const second = collector.toMeterData(baseSnapshot({ collectedAt: 1_001 })); + expect(second.map((m) => m.getSinglevalue()?.getName())).toEqual(EXPECTED_METER_NAMES); }); });