Skip to content

feat(tcp): add praxis_tcp_connections_total counter metric - #1037

Merged
shaneutt merged 2 commits into
praxis-proxy:mainfrom
tsisodia10:feat/tcp-connections-total-counter
Sep 1, 2026
Merged

feat(tcp): add praxis_tcp_connections_total counter metric#1037
shaneutt merged 2 commits into
praxis-proxy:mainfrom
tsisodia10:feat/tcp-connections-total-counter

Conversation

@tsisodia10

Copy link
Copy Markdown
Contributor

Increment a Prometheus counter on every accepted TCP connection, labeled by listener name. Gives operators visibility into connection throughput per listener over time.

What does this PR do?

Adds a praxis_tcp_connections_total Prometheus counter that increments once per accepted TCP connection, labeled by listener name. This enables operators to compute connection rates (connections/sec) per listener in dashboards.

Changes:

  • New protocol/src/tcp/metrics.rs module with record_tcp_connection_accepted()
  • Counter incremented in process_new() after overload checks pass
  • Example config: examples/configs/observability/tcp-connections-total.yaml
  • Unit test + integration tests (counter increments, correct label, example forwarding)

Which issue(s) does this relate to?

Fixes #1025

Checklist

  • Signed off all commits (git commit -s)
  • Tests added or updated
  • Documentation updated (if applicable)
  • make lint && make test passes locally

Does this introduce a breaking change?

No. The counter is purely additive and only emitted when the admin interface is enabled.

@tsisodia10
tsisodia10 requested review from a team August 24, 2026 15:52
@tsisodia10
tsisodia10 requested a review from shaneutt as a code owner August 24, 2026 15:52

@praxis-bot praxis-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PR Review: feat(tcp): add praxis_tcp_connections_total counter metric

Clean, well-scoped addition of a Prometheus counter for TCP connection throughput. The implementation correctly follows established metrics patterns (is_recorder_installed() guard, SharedString labels, placement after overload checks). The listener_label extraction avoids a redundant listener_label_for call.

Test coverage is solid: unit test for the no-recorder path, integration test verifying counter value after 3 connections (>= 3), and two example config tests exercising counter emission and traffic forwarding end-to-end.

Severity Count
Critical 0
Large 0
Medium 1

1 inline comment posted.

Comment thread protocol/src/tcp/mod.rs Outdated
@tsisodia10
tsisodia10 force-pushed the feat/tcp-connections-total-counter branch 2 times, most recently from 3a45dad to 2048fbe Compare August 25, 2026 14:31
@aslakknutsen

aslakknutsen commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Docs covered in #1036

@aslakknutsen
aslakknutsen enabled auto-merge (squash) August 27, 2026 15:52
@shaneutt shaneutt self-assigned this Aug 28, 2026
@shaneutt shaneutt moved this from Next to Review in Core Proxy Aug 28, 2026
@shaneutt shaneutt added this to the v0.5.4 milestone Aug 28, 2026
@praxis-bot
praxis-bot marked this pull request as draft August 28, 2026 12:11
auto-merge was automatically disabled August 28, 2026 12:11

Pull request was converted to draft

@praxis-bot praxis-bot added the holding-pattern Waiting for discussions or contributor updates in order to proceed label Aug 28, 2026
@aslakknutsen

Copy link
Copy Markdown
Contributor

Hmm.. I think this was just moved to Draft due to the policy change.

@tsisodia10 Could you resolve the conflicts?

@aslakknutsen
aslakknutsen marked this pull request as ready for review September 1, 2026 08:32
@aslakknutsen aslakknutsen removed the holding-pattern Waiting for discussions or contributor updates in order to proceed label Sep 1, 2026

@praxis-bot praxis-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PR Review

Re-review: new commits since last review (2026-08-25).

Summary: The new commits introduced a merge from main that appears to have corrupted protocol/src/tcp/metrics.rs. The function record_tcp_connection_accepted is missing its closing brace, and duplicate SPDX headers and import lines were introduced. CI has only run metadata checks (DCO, commit messages), not build/lint/test, so these compilation errors are uncaught.

Overall: The PR cannot compile in its current state. Needs fixes before further review.

Severity Count
Critical 1
Large 0
Medium 2

Comment thread protocol/src/tcp/metrics.rs
Comment thread protocol/src/tcp/metrics.rs Outdated
Comment thread protocol/src/tcp/metrics.rs Outdated
@shaneutt shaneutt assigned aslakknutsen and unassigned shaneutt Sep 1, 2026
@shaneutt shaneutt modified the milestones: v0.5.4, v0.6.0 Sep 1, 2026
@tsisodia10
tsisodia10 force-pushed the feat/tcp-connections-total-counter branch from 3985bca to 2048fbe Compare September 1, 2026 16:24
Increment a Prometheus counter on every accepted TCP connection,
labeled by listener name. Gives operators visibility into connection
throughput per listener over time.

Closes praxis-proxy#1025

Signed-off-by: Twinkll Sisodia <tsisodia@redhat.com>
@tsisodia10
tsisodia10 force-pushed the feat/tcp-connections-total-counter branch from 2048fbe to 3bfe23d Compare September 1, 2026 17:00
@aslakknutsen
aslakknutsen enabled auto-merge (squash) September 1, 2026 18:19

@shaneutt shaneutt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Appreciate a patch that is mostly tests. 🫡

Thank you for the PR 🖖

@shaneutt
shaneutt disabled auto-merge September 1, 2026 19:17
@shaneutt
shaneutt merged commit ef3247d into praxis-proxy:main Sep 1, 2026
27 checks passed
@github-project-automation github-project-automation Bot moved this from Review to Done in Core Proxy Sep 1, 2026
@shaneutt shaneutt modified the milestones: v0.6.0, v0.5.4 Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

praxis_tcp_connections_total counter

4 participants