Skip to content

feat(tcp): add praxis_tcp_connection_duration_seconds histogram metric - #1036

Merged
shaneutt merged 3 commits into
praxis-proxy:mainfrom
tsisodia10:feat/tcp-connection-duration-metric
Aug 28, 2026
Merged

feat(tcp): add praxis_tcp_connection_duration_seconds histogram metric#1036
shaneutt merged 3 commits into
praxis-proxy:mainfrom
tsisodia10:feat/tcp-connection-duration-metric

Conversation

@tsisodia10

Copy link
Copy Markdown
Contributor

Record TCP connection lifetime (accept to close) as a Prometheus histogram labeled by listener name. The metric is emitted on all disconnect paths and exposed via the admin /metrics endpoint when enabled.

What does this PR do?

Adds a praxis_tcp_connection_duration_seconds Prometheus histogram that records the lifetime of each TCP connection (from accept to close), labeled by listener name. This gives operators visibility into how long TCP connections live across different listeners.

Changes:

  • New protocol/src/tcp/metrics.rs module with record_tcp_connection_duration()
  • Records duration at all disconnect exit paths in PingoraTcpProxy::process_new()
  • Uses the existing listener_label_for() infrastructure for accurate per-listener labeling
  • Example config: examples/configs/observability/tcp-connection-metrics.yaml
  • Unit tests (no-panic without recorder) and integration tests (metric emission + correct labels)

Which issue(s) does this relate to?

Fixes #1027

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 metric is purely additive and only emitted when the admin interface is enabled.

@tsisodia10
tsisodia10 requested review from a team August 24, 2026 15:11
@tsisodia10
tsisodia10 requested a review from shaneutt as a code owner August 24, 2026 15:11
@praxis-bot-app

Copy link
Copy Markdown

Tool authorship detected:

  • bf2a5ab: Co-authored-by: Cursor <cursoragent@cursor.com>

This project doesn't allow commits which are authored or signed-off by tools.
Commits need to be authored by and signed-off by the human(s) responsible for the PR, with their name and contact.
Please see our development documentation for more details.

@tsisodia10
tsisodia10 force-pushed the feat/tcp-connection-duration-metric branch from bf2a5ab to 64348c0 Compare August 24, 2026 15:12

@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.

TCP Connection Duration Metric Review

One finding identified in the metric recording coverage.

Comment thread protocol/src/tcp/proxy.rs
@shaneutt shaneutt self-assigned this Aug 25, 2026
@shaneutt shaneutt moved this from Next to Review in AI Gateway - Model Serving Aug 25, 2026
@shaneutt shaneutt added this to the v0.5.4 milestone Aug 25, 2026
@tsisodia10
tsisodia10 force-pushed the feat/tcp-connection-duration-metric branch 3 times, most recently from 2162569 to 501476d Compare August 25, 2026 13:42

@aslakknutsen aslakknutsen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sni_timeout and filter_rejection have measurable connection times and will be in connection_close logs, but not recorded as part of histogram.

Intentional?

Would it perhaps make sense to add a label for the connection close type?

Comment thread protocol/src/tcp/proxy.rs
Comment thread protocol/src/tcp/proxy.rs
@tsisodia10

Copy link
Copy Markdown
Contributor Author

sni_timeout and filter_rejection have measurable connection times and will be in connection_close logs, but not recorded as part of histogram.

Intentional?

Would it perhaps make sense to add a label for the connection close type?

Good catch - not intentional, these paths should be recorded too.

On the reason label: makes sense. I'll add a reason label with values like completed, sni_timeout, filter_rejection, connect_failure, and peeked_write_error so operators can slice the histogram by close type.

@tsisodia10
tsisodia10 force-pushed the feat/tcp-connection-duration-metric branch from 9aece91 to bf5d7a8 Compare August 26, 2026 13:42
@aslakknutsen

Copy link
Copy Markdown
Contributor

Worth a mention in

### HTTP Request Metrics
?

Comment thread tests/integration/tests/suite/tcp_connection_metrics.rs
@tsisodia10
tsisodia10 force-pushed the feat/tcp-connection-duration-metric branch from bf5d7a8 to 934f454 Compare August 26, 2026 14:03
@tsisodia10

Copy link
Copy Markdown
Contributor Author

Worth a mention in

### HTTP Request Metrics

?

Added under ### TCP Connection Metrics section. Also updated the intro to reflect three metric categories.

@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.

TCP Connection Duration Metric Re-review

One new finding since the previous review. The earlier connect_failure exit path coverage issue appears resolved in the current code.

Comment thread docs/operating/observability.md
auto-merge was automatically disabled August 27, 2026 12:22

Head branch was pushed to by a user without write access

@tsisodia10
tsisodia10 force-pushed the feat/tcp-connection-duration-metric branch from 5991bb4 to 934f454 Compare August 27, 2026 12:22
Record TCP connection lifetime (accept to close) as a Prometheus
histogram labeled by listener name and close reason. The metric is
emitted on all disconnect paths (completed, sni_timeout,
filter_rejection, connect_failure, peeked_write_error) and exposed
via the admin /metrics endpoint when enabled.

Closes praxis-proxy#1027

Signed-off-by: Twinkll Sisodia <tsisodia@redhat.com>
@tsisodia10
tsisodia10 force-pushed the feat/tcp-connection-duration-metric branch from 201b364 to a177efc Compare August 27, 2026 12:38
@aslakknutsen
aslakknutsen enabled auto-merge (squash) August 27, 2026 13:19
auto-merge was automatically disabled August 27, 2026 13:58

Head branch was pushed to by a user without write access

@praxis-bot-app

Copy link
Copy Markdown

Commit message format: the following commits do not follow conventional commits:

  • 88d9488: style(protocol): rustfmt TCP connection duration metric

Expected format: type(scope): summary (max 72 chars). Types: build, chore, ci, docs, feat, fix, perf, refactor, test.

@tsisodia10
tsisodia10 force-pushed the feat/tcp-connection-duration-metric branch from 88d9488 to f2b988f Compare August 27, 2026 13:59
@praxis-bot-app

Copy link
Copy Markdown

Unsigned commits: f2b988f. Please sign your commits.

Signed-off-by: Twinkll Sisodia <tsisodia@redhat.com>
@tsisodia10
tsisodia10 force-pushed the feat/tcp-connection-duration-metric branch from f2b988f to 8564bea Compare August 27, 2026 14:02
@aslakknutsen
aslakknutsen enabled auto-merge (squash) August 27, 2026 15:46
@shaneutt
shaneutt disabled auto-merge August 28, 2026 12:06
@shaneutt
shaneutt merged commit 2a9a8a4 into praxis-proxy:main Aug 28, 2026
27 checks passed
@github-project-automation github-project-automation Bot moved this from Review to Done in AI Gateway - Model Serving Aug 28, 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_connection_duration_seconds histogram

4 participants