fix(metrics): poll S3 client metrics periodically by ziwon · Pull Request #1957 · awslabs/mountpoint-s3

ziwon · 2026-09-08T00:33:43Z

What changed and why?

S3 CRT client gauges (s3.client.*) were sampled only when make_meta_request() ran. The metrics publisher still flushed every five seconds, but it only refreshed process metrics before publish(). For meta requests that last longer than that interval, the CRT gauges were therefore frozen after the first sample.

This change moves sampling onto the existing publisher cycle:

MetricsSinkHandle::register_poller stores a Send + Sync callback in a publisher-lifecycle registry. Registration does not go through the shutdown/recv_timeout channel, so it cannot reset or delay the five-second wait.
On each timeout and on shutdown, the publisher runs poll_process_metrics(), then the registered client pollers, then publish().
After the object client is built, run and the FS examples that install metrics register a clone of the client. S3CrtClient is Clone via Arc, so the final drain can safely sample after the FUSE session ends.
The poller registry is shared only by the publisher thread and MetricsSinkHandle. Dropping the handle joins the final publication and then releases the callbacks and captured client clones; the process-global metrics recorder does not retain them.
ObjectClient::poll_client_metrics is a default no-op, overridden by S3CrtClient. Mock and other non-CRT clients keep working without CRT-specific behavior.
The polling call immediately after make_meta_request() is removed. Metric names, gauge/histogram types, and changed-value emission behavior are unchanged.

Fixes #1410

Does this change impact existing behavior?

Yes, in the intended way: s3.client.* gauges now update on every five-second publication cycle even when no new meta request is created. Long-running requests no longer leave those gauges stuck at the value from request start. Existing ObjectClient implementations inherit the new no-op method and do not need to override it.

Does this change need a changelog entry? Does it require a version change?

Yes. Unreleased notes were added for mountpoint-s3, mountpoint-s3-fs, and mountpoint-s3-client. Version numbers are left for the next release cut.

Tests

Deterministic unit tests require no AWS credentials, network access, or real S3 service:

A registered poller runs on repeated publisher timeouts with no meta requests, each poll occurs before the matching publish(), and shutdown performs one final poll and publication.
Registering pollers during the wait does not reset or stop the cadence and does not trigger an immediate publication.
Dropping MetricsSinkHandle releases registered callbacks even while the metrics sink remains alive.
S3CrtClient::poll_client_metrics, constructed with NoSigning, emits the existing s3.client.* gauges under a local recorder.
Mock ObjectClient::poll_client_metrics remains a no-op.

After rebasing onto current upstream main, make pre-pr-check succeeded: formatting, cargo check --all-targets --all-features, repository-standard clippy with warnings denied, and 941 nextest tests passed with 1 skipped. This PR does not include an end-to-end AWS/--log-metrics mount run.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).

tadiwa-aizen · 2026-09-30T10:28:18Z


        // Issue the HTTP request using the CRT's S3 meta request API
        let meta_request = self.s3_client.make_meta_request(options)?;
-        Self::poll_client_metrics(&self.s3_client);


Nice that the CRT poll is no longer inside make_meta_request, one less thing on the request path.

tadiwa-aizen · 2026-09-30T10:30:11Z


+    /// Sample client-level metrics into the process metrics facade.
+    ///
+    /// The metrics publisher invokes this on each publication cycle, immediately before publishing.


this reads like it's called automatically, but nothing in this crate calls it. It only runs if the caller sets that up.. . Maybe: "Callers should invoke this periodically (e.g. from a metrics publisher) to emit s3.client.* metrics."

Good point — I updated the docs to make it explicit that callers are responsible for invoking this periodically.

tadiwa-aizen · 2026-09-30T10:44:07Z

+    ///
+    /// Intended to be invoked by the metrics publisher on each publication cycle, not when
+    /// individual meta requests are created.
+    pub fn poll_client_metrics(&self) {


This is the same as the ObjectClient::poll_client_metrics impl below, which just calls it. It looks like it's only here because mount_from_config.rs doesn't import ObjectClient.

Maybe would we add that import and drop this method, having the trait impl call self.inner.emit_client_metrics() directly?

Agreed — I removed the duplicate inherent method and now call the inner emitter directly from the trait implementation. I also imported ObjectClient in mount_from_config.

tadiwa-aizen

Thanks for this! The unit tests cover the pieces well. I've left a few nit comments on the code, and I'll test it manually on a real mount to confirm the behaviour.

S3 CRT client gauges were sampled only when a meta request started, so values for requests lasting longer than the five-second publication period went stale. Sample them on every publisher cycle, immediately before publish, and stop polling at make_meta_request. Fixes awslabs#1410 Signed-off-by: Aaron Y. <yngpil.yoon@gmail.com>

Signed-off-by: Aaron Y. <yngpil.yoon@gmail.com>

ziwon · 2026-10-03T18:24:09Z

Thanks again, @tadiwa-aizen! I rebased onto the latest main and resolved the changelog conflicts. I also made the publisher tests less timing-sensitive by ignoring events before the first poll, waiting for poll events directly, and using a deadline instead of a fixed 350 ms window.

The two affected tests passed 30 consecutive runs each, and cargo check --all-targets --all-features passes on Rust 1.98. Ready for another look when you have a chance.

tadiwa-aizen

I just tested that this works. Before this change, the metric s3.client.num_auto_ranged_put_network_io was only read once when an upload started (0), then not updated again during the upload. Now it updates every 5s (25 during the upload, 0 after), which is correct and more appropriate as per metrics semantics: "Metrics will be collected by Mountpoint and flushed to the logs every five seconds" (doc/LOGGING.md). LGTM!

ziwon · 2026-10-04T08:56:48Z

Thanks for verifying this on a real mount! Glad to hear it’s working as intended.

ziwon requested a deployment to PR integration tests September 8, 2026 00:33 — with GitHub Actions Waiting

ziwon force-pushed the fix/poll-s3-client-metrics-periodically branch from 4ebb941 to 30b1eb4 Compare September 8, 2026 00:56

ziwon requested a deployment to PR integration tests September 8, 2026 00:56 — with GitHub Actions Waiting

tadiwa-aizen reviewed Sep 30, 2026

View reviewed changes

tadiwa-aizen requested changes Sep 30, 2026

View reviewed changes

ziwon requested a deployment to PR integration tests October 1, 2026 00:15 — with GitHub Actions Waiting

ziwon force-pushed the fix/poll-s3-client-metrics-periodically branch from 024120b to 61f45a2 Compare October 1, 2026 00:16

ziwon requested a deployment to PR integration tests October 1, 2026 00:16 — with GitHub Actions Waiting

ziwon added 3 commits October 4, 2026 03:16

refactor(metrics): remove duplicate client polling method

c28bfbd

Signed-off-by: Aaron Y. <yngpil.yoon@gmail.com>

test(metrics): avoid timing-sensitive publisher assertions

e1ac40e

Signed-off-by: Aaron Y. <yngpil.yoon@gmail.com>

ziwon force-pushed the fix/poll-s3-client-metrics-periodically branch from 61f45a2 to e1ac40e Compare October 3, 2026 18:23

ziwon requested a deployment to PR integration tests October 3, 2026 18:23 — with GitHub Actions Waiting

tadiwa-aizen approved these changes Oct 3, 2026

View reviewed changes

Navigation Menu

fix(metrics): poll S3 client metrics periodically - #1957

fix(metrics): poll S3 client metrics periodically#1957
ziwon wants to merge 3 commits into
awslabs:mainfrom
ziwon:fix/poll-s3-client-metrics-periodically

ziwon commented Sep 8, 2026 •

edited

Loading

Uh oh!

tadiwa-aizen Sep 30, 2026

Uh oh!

tadiwa-aizen Sep 30, 2026 •

edited

Loading

Uh oh!

ziwon Oct 1, 2026

Uh oh!

tadiwa-aizen Sep 30, 2026 •

edited

Loading

Uh oh!

ziwon Oct 1, 2026

Uh oh!

tadiwa-aizen left a comment

Uh oh!

ziwon commented Oct 3, 2026

Uh oh!

tadiwa-aizen left a comment •

edited

Loading

Uh oh!

ziwon commented Oct 4, 2026

Uh oh!

This branch is waiting to be deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ziwon commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Does this change impact existing behavior?

Does this change need a changelog entry? Does it require a version change?

Tests

Uh oh!

tadiwa-aizen Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

tadiwa-aizen Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ziwon Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

tadiwa-aizen Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ziwon Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

tadiwa-aizen left a comment

Choose a reason for hiding this comment

Uh oh!

ziwon commented Oct 3, 2026

Uh oh!

tadiwa-aizen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ziwon commented Oct 4, 2026

Uh oh!

This branch is waiting to be deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ziwon commented Sep 8, 2026 •

edited

Loading

tadiwa-aizen Sep 30, 2026 •

edited

Loading

tadiwa-aizen Sep 30, 2026 •

edited

Loading

tadiwa-aizen left a comment •

edited

Loading