Skip to content

fix: single callback for memory/handle collector#1118

Merged
ferhatelmas merged 2 commits into
masterfrom
ferhat/single-mem-collector
May 21, 2026
Merged

fix: single callback for memory/handle collector#1118
ferhatelmas merged 2 commits into
masterfrom
ferhat/single-mem-collector

Conversation

@ferhatelmas
Copy link
Copy Markdown
Member

@ferhatelmas ferhatelmas commented May 21, 2026

What kind of change does this PR introduce?

refactor

What is the current behavior?

Each memory/handle metric is using a separate observable callback which does calculation which could be shared.

What is the new behavior?

Do a single batch callback instead that does computation once and shares between metrics.

@ferhatelmas ferhatelmas requested a review from a team as a code owner May 21, 2026 08:01
Copilot AI review requested due to automatic review settings May 21, 2026 08:01
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the ExternalMemoryCollector to register a single OpenTelemetry batch callback for multiple observable gauges, ensuring process.memoryUsage() is read only once per collection cycle.

Changes:

  • Replaced three per-gauge observable callbacks with one meter.addBatchObservableCallback covering external, ArrayBuffers, and RSS gauges.
  • Added a Vitest unit test to validate that all gauges are observed from a single process.memoryUsage() call.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/internal/monitoring/system/memory-collector.ts Consolidates memory metric collection into a single batch observable callback to avoid multiple process.memoryUsage() reads.
src/internal/monitoring/system/memory-collector.test.ts Adds test coverage ensuring the collector observes all gauges and calls process.memoryUsage() only once per batch invocation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coveralls
Copy link
Copy Markdown

coveralls commented May 21, 2026

Coverage Report for CI Build 26217125033

Coverage increased (+0.2%) to 75.171%

Details

  • Coverage increased (+0.2%) from the base build.
  • Patch coverage: 15 of 15 lines across 2 files are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 10356
Covered Lines: 8196
Line Coverage: 79.14%
Relevant Branches: 5988
Covered Branches: 4090
Branch Coverage: 68.3%
Branches in Coverage %: Yes
Coverage Strength: 370.07 hits per line

💛 - Coveralls

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread src/internal/monitoring/system/handles-collector.ts Outdated
@ferhatelmas ferhatelmas force-pushed the ferhat/single-mem-collector branch from ab00845 to 8c43b1c Compare May 21, 2026 08:59
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
@ferhatelmas ferhatelmas force-pushed the ferhat/single-mem-collector branch from 8c43b1c to 3d7b1a9 Compare May 21, 2026 09:07
@ferhatelmas ferhatelmas requested a review from Copilot May 21, 2026 09:07
@ferhatelmas ferhatelmas changed the title fix: single callback for memory collector fix: single callback for memory/handle collector May 21, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread src/internal/monitoring/system/handles-collector.ts Outdated
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
@ferhatelmas ferhatelmas force-pushed the ferhat/single-mem-collector branch from 3d7b1a9 to 2df3e3b Compare May 21, 2026 09:18
@ferhatelmas ferhatelmas merged commit 817f1c8 into master May 21, 2026
21 checks passed
@ferhatelmas ferhatelmas deleted the ferhat/single-mem-collector branch May 21, 2026 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants