[PR #4971] [signal, management, client, cmd] Create shared pprof package and allow pprof port configuration through NB_PPROF_ADDR #24619

Open
opened 2026-08-05 06:08:55 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/4971
Author: @ujaandas
Created: 12/18/2025
Status: 🔄 Open

Base: mainHead: pprof-configurable-signal-mgmt


📝 Commits (8)

  • 6dbf296 [signal] add NB_PPROF_ADDR envvar support
  • 6e4d355 [management] add NB_PPROF_ADDR envvar support
  • 8c2c251 [util] add shared pprof package
  • e948749 [signal] use shared pprof package
  • 39ccc02 [management] use shared pprof package
  • 8eb0f82 [relay] use shared pprof package
  • 684fd30 [client] use shared pprof package
  • 309a410 [util] fix typo in util/pprof/pprof_off.go

📊 Changes

10 files changed (+17 additions, -56 deletions)

View changed files

📝 client/cmd/root.go (+1 -0)
📝 management/cmd/root.go (+1 -0)
📝 management/main.go (+0 -7)
relay/cmd/pprof.go (+0 -33)
📝 relay/cmd/root.go (+1 -0)
📝 signal/cmd/root.go (+1 -0)
📝 signal/cmd/run.go (+1 -13)
📝 signal/main.go (+2 -1)
📝 util/pprof/pprof.go (+2 -2)
util/pprof/pprof_off.go (+8 -0)

📄 Description

Describe your changes

Signal and Management servers previously hard‑coded a pprof listener on localhost:6060, which caused port conflicts when multiple services ran in the same namespace (ie; in the same pod or as a NixOS service). This PR refactors both to use the same pprof.go helper pattern already present in Client/Relay.

Does this change build semantics though? To test it, I ran it with -tags pprof. If it does, it shouldn't be a huge change to just slap what pprof.go does directly into the relevant functions.

Fixes #4923

Stack

Branch: pprof-configurable-signal-mgmt

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

The behavior is consistent with existing Client/Relay profiling logic, so no new user‑facing configuration is introduced beyond the existing NB_PPROF_ADDR.

Summary by CodeRabbit

  • Refactor

    • Centralized profiling support into a shared utility and adjusted initialization across components.
    • Removed automatic local profiling server startup in some services; profiling is now opt-in.
  • Chores

    • Introduced a build-time toggle to exclude profiling when not needed.
    • Changed the default profiling address from localhost:6969 to localhost:6060.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/netbirdio/netbird/pull/4971 **Author:** [@ujaandas](https://github.com/ujaandas) **Created:** 12/18/2025 **Status:** 🔄 Open **Base:** `main` ← **Head:** `pprof-configurable-signal-mgmt` --- ### 📝 Commits (8) - [`6dbf296`](https://github.com/netbirdio/netbird/commit/6dbf296e39ced6094ac0c6113f00c23f28f82b72) [signal] add NB_PPROF_ADDR envvar support - [`6e4d355`](https://github.com/netbirdio/netbird/commit/6e4d3554917dae507bd000952f6b88a167d1a093) [management] add NB_PPROF_ADDR envvar support - [`8c2c251`](https://github.com/netbirdio/netbird/commit/8c2c251e950205b5126e1d28a9896047b186588b) [util] add shared pprof package - [`e948749`](https://github.com/netbirdio/netbird/commit/e94874980da1598142d78c3766305ddfa23725b3) [signal] use shared pprof package - [`39ccc02`](https://github.com/netbirdio/netbird/commit/39ccc02fd3d0209dddd94e0795a9b5255f371121) [management] use shared pprof package - [`8eb0f82`](https://github.com/netbirdio/netbird/commit/8eb0f82eac167c75a1c3d1be84af41d91861421a) [relay] use shared pprof package - [`684fd30`](https://github.com/netbirdio/netbird/commit/684fd308cfd7574520fbab9ea782d870431f2319) [client] use shared pprof package - [`309a410`](https://github.com/netbirdio/netbird/commit/309a410d5e4dba50ccff728bda935ca3aaf1aca9) [util] fix typo in util/pprof/pprof_off.go ### 📊 Changes **10 files changed** (+17 additions, -56 deletions) <details> <summary>View changed files</summary> 📝 `client/cmd/root.go` (+1 -0) 📝 `management/cmd/root.go` (+1 -0) 📝 `management/main.go` (+0 -7) ➖ `relay/cmd/pprof.go` (+0 -33) 📝 `relay/cmd/root.go` (+1 -0) 📝 `signal/cmd/root.go` (+1 -0) 📝 `signal/cmd/run.go` (+1 -13) 📝 `signal/main.go` (+2 -1) 📝 `util/pprof/pprof.go` (+2 -2) ➕ `util/pprof/pprof_off.go` (+8 -0) </details> ### 📄 Description ## Describe your changes Signal and Management servers previously hard‑coded a pprof listener on `localhost:6060`, which caused port conflicts when multiple services ran in the same namespace (ie; in the same pod or as a NixOS service). This PR refactors both to use the same `pprof.go` helper pattern already present in Client/Relay. Does this change build semantics though? To test it, I ran it with `-tags pprof`. If it does, it shouldn't be a huge change to just slap what `pprof.go` does directly into the relevant functions. ## Issue ticket number and link Fixes #4923 ## Stack <!-- branch-stack --> Branch: `pprof-configurable-signal-mgmt` ### Checklist - [ ] Is it a bug fix - [ ] Is a typo/documentation fix - [x] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) The behavior is consistent with existing Client/Relay profiling logic, so no new user‑facing configuration is introduced beyond the existing NB_PPROF_ADDR. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Centralized profiling support into a shared utility and adjusted initialization across components. * Removed automatic local profiling server startup in some services; profiling is now opt-in. * **Chores** * Introduced a build-time toggle to exclude profiling when not needed. * Changed the default profiling address from localhost:6969 to localhost:6060. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
saavagebueno added the pull-request label 2026-08-05 06:08:55 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#24619