Record how long a client-terminated request had been in flight

[jitheshtr]

Motivation

NettyMessageProcessor logs that a request was aborted because the channel became inactive, but not how long that request had been running. Without an elapsed time there is no way to distinguish a client that gave up after a fixed deadline from one that died early.

NettyMetrics.clientEarlyTerminationCount already counts these aborts, and their duration does eventually reach RestRequestMetrics.nioRoundTripTimeInMs — but that histogram is bucketed per request type and mixes aborts with successes, so it cannot answer "do clients abort at a fixed deadline". Only the abort-only distribution is new here.

What changed

1. RestRequestMetricsTracker.getTimeSinceRequestReceivedInMs() — a new accessor. No new timestamp is introduced: NettyRequest's constructor already calls nioMetricsTracker.markRequestReceived(), so the arrival time was recorded and simply had no reader. It returns 0 rather than throwing when the request was never marked received, unlike its markFirstByteSent()/markRequestCompleted() siblings, because its caller is a diagnostic on an error path.
2. New NettyMessageProcessor.ClientTerminatedRequestTimeInMs histogram, recorded from the two client-termination paths — channelInactive() and the idle timeout in userEventTriggered() — through a helper that is one shot per request. Which of the two observes the request still open is transport dependent, so recording once is what keeps the count correct rather than an artifact of the transport.
3. Both abort log lines gain the elapsed time. On channelInactive() it is appended, so any prefix-based log matching keeps working; the idle path previously logged no per-request line at all.

Item 1 is the one worth attention — it adds a public method to a shared API module.

Scope of the metric

onRequestAborted() has nine call sites and the histogram is fed from the two that are client terminations. The other seven are server-side or protocol errors and do not reach it, because onRequestAborted() closes the request before the deferred channelInactive() task runs.

Measured against real TCP sockets on NioEventLoopGroup, with a corroborating counter each time to show the path really ran. NettyServer prefers Epoll where available and falls back to NIO, but both defer fireChannelInactive through SingleThreadEventLoop, which is the property this turns on:

Cause	Recorded	Corroboration
Client disconnect	1	clientEarlyTerminationCount=1
Idle timeout	1	idleConnectionCloseCount=1, clientEarlyTerminationCount=1
Server-side exception	0	processorExceptionCaughtCount=1
Duplicate-request protocol error	0	requestArrivalRate=1

One residual gap: exceptionCaught() forwards an arbitrary pipeline Exception, which can include a client-caused I/O failure. Those are not recorded, so the histogram is a lower bound on client terminations. Classifying them is a larger change than this one.

Risk Assessment

Durability: no risk. Metrics and logging only. It touches no write, named-blob PUT, TTL, or delete path; no blob metadata storage, read, or indexing; no ordering or atomicity; no callback semantics — nothing is reported to a client earlier or later than before; no ByteBuf release, Closeable.close(), or stream lifecycle; no retry or idempotency behaviour; no corruption-detection path. Every checklist item is unchecked.

The one-shot flag is reset in resetState(), which runs immediately before every request is constructed, so a keepalive channel records once per request rather than once per connection.

The idle path adds one error line per idle abort, alongside the info line that already fires there.

Testing Done

Command	Result
./gradlew :ambry-rest:test	97 tests, 0 failures
./gradlew :ambry-api:test	233 tests, 0 failures, 6 skipped

testIdleChannelAbortOnRealEventLoopRecordsTimeInFlight drives a LocalChannel on a real event loop, because EmbeddedChannel runs fireChannelInactive inline while a real loop defers it — so the EmbeddedChannel tests alone cannot distinguish the correct behaviour from a metric that silently misses every idle abort. It was run 8 times in a row to check it is not timing sensitive.

The two EmbeddedChannel abort tests bound the recorded value below by the time the request was deliberately held open and above by the test's own duration, so a hardcoded zero, a dropped subtraction, or a read of the not-yet-populated roundTripTimeInMs all fail. testTimeSinceRequestReceived covers the new accessor directly, including the unmarked-returns-zero branch.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 23.52941% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.74%. Comparing base (52ba813) to head (2bd059b).
⚠️ Report is 410 commits behind head on master.

Files with missing lines	Patch %	Lines
...a/com/github/ambry/rest/NettyMessageProcessor.java	15.38%	11 Missing ⚠️
...m/github/ambry/rest/RestRequestMetricsTracker.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             master    #3287       +/-   ##
=============================================
- Coverage     64.24%   50.74%   -13.51%     
+ Complexity    10398     8683     -1715     
=============================================
  Files           840      938       +98     
  Lines         71755    80488     +8733     
  Branches       8611     9687     +1076     
=============================================
- Hits          46099    40840     -5259     
- Misses        23004    36252    +13248     
- Partials       2652     3396      +744     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.