[akasakariko]

Summary

• Increase all five client-info attribution fields from 255 to 512 UTF-8 bytes so longer UTM values are preserved
• Retain the ordinary string limit, UTF-8 boundary truncation, and overflow error reporting
• Extend the existing attribution regression test and update the API documentation and changelog

Bugzilla bug 2079526

Review follow-up

• In ff82808, query attribution overflow errors through the string metric object and remove the unnecessary MetricType import
• Add a crate-private synchronous member for the test's local Glean instance and have the existing public getter delegate to it without changing the public API
• Rebase onto e6d54f3 and preserve both changelog entries when resolving the conflict
• The range-diff confirms that the review follow-up code is unchanged by the rebase

Latest local validation

Validated the final revision ff82808 on Linux arm64 through Colima with Rust 1.90.0, an unprivileged user, and faketime available
The Makefile selected the built-in Rust test runner with default threading, including doctests

• cargo test --locked -p glean-core --lib attribution_and_distribution_are_correctly_stored passes
• CI=1 RUST_LOG=glean_core=debug make test-rust passes with 438 passed and 10 ignored, including the attribution regression and mps_delay
• make lint-rust passes without warnings or errors
• make fmt-rust passes and leaves the working tree unchanged
• cargo test --locked -p glean-core --features sqlite --lib --test string --test ping_maker passes with 175 passed and 2 ignored
• git diff --check and the diff check against the new base pass

macOS validation limitation

On macOS 26.4.1 arm64 with Rust 1.98.1 and nextest 0.9.150, the final revision's CI=1 RUST_LOG=glean_core=debug make test-rust reports 433 passed, 1 failed, and 5 skipped
The failure remains the 5-second timeout in the first mps_delay initialization subprocess, with exit code 124
The same timeout also reproduced on the unchanged previous PR head 9e93367 before the rebase
On the final revision, cargo nextest run --locked --all -E 'test(mps_delay)' passes when run separately with CI=1
The root cause of the macOS full-suite timeout remains undetermined, and the passing Linux run is not a claim that it is fixed
No assertions were weakened, timeouts extended, or tests disabled to obtain the passing results

[chutten]

Looks to be in good repair, but I have some questions.

Also: rather than running all those custom cargo test lines, please follow the guidance from our contribution documentation: https://mozilla.github.io/glean/dev/contributing.html

[chutten]

What is this use statement for?

[akasakariko]

MetricType provides meta(), which the overflow assertions use to pass each field's metadata to test_get_num_recorded_errors

Added a comment at the import to explain that dependency

[chutten]

Oh, I see. You shouldn't need that. You should use the metric object's test_get_num_recorded_errors member instead of calling error_recording's function.

[akasakariko]

@chutten Addressed in ff82808

The attribution test now calls the string metric's test_get_num_recorded_errors_sync member and no longer imports MetricType
The test owns a local Glean instance, whereas the existing public getter uses the global singleton, so the crate-private synchronous member accepts that instance explicitly
The public getter delegates to the same implementation and its API is unchanged

I also rebased onto e6d54f3 and resolved the changelog conflict while preserving both entries
The range-diff confirms that the review follow-up code is unchanged by the rebase

On the final revision, Linux arm64 through Colima with Rust 1.90.0 passes CI=1 RUST_LOG=glean_core=debug make test-rust with 438 passed and 10 ignored, including mps_delay
The standard lint and formatting targets and the SQLite compatibility checks also pass

The PR description records the remaining macOS full-suite timeout, which also reproduced on the unchanged previous PR head 9e93367
The isolated timing test passes on the final revision, but the full-suite timeout remains undiagnosed
No assertions were weakened or timeouts extended

Ready for another look

[akasakariko]

@chutten Both inline questions are addressed in 9e93367

Follow-up validation after installing Homebrew libfaketime 0.9.13 and GNU coreutils 9.12 on macOS 26.4.1 arm64:

• Rust 1.98.1: CI=1 make test-rust-with-logs reports 429 passed and 10 ignored
• Rust 1.90.0: CI=1 RUST_TEST_THREADS=1 make test-rust reports 429 passed and 10 ignored
• Both single-threaded full-suite runs execute and pass mps_delay, including its timing assertions
• The isolated mps_delay test also passes
• Earlier make lint-rust and make fmt-rust checks passed on Rust 1.90.0 at this same commit

The test process PATH includes /opt/homebrew/opt/coreutils/libexec/gnubin to provide GNU timeout
CI=1 prevents the test from returning early when faketime is missing

With default test threading, both toolchains still hit the existing 5-second timeout in the first mps_delay init subprocess, with exit code 124
The cause remains undetermined, so these default full-suite runs are recorded as failures
An initial overlap between two workspace runs also caused a shared documentation-generation temporary-file conflict, which was eliminated by running the toolchains one after the other

No test assertions, timeouts, or repository files were changed for this validation
The PR description now records the passing single-threaded runs and the unresolved default-threading timeout

[akasakariko]

@chutten Sorry about that — I have now followed the contribution documentation and rerun the standard Makefile targets on the unchanged revision 9e93367

• CI=1 make test-rust passes with 426 passed and 5 skipped using the nextest runner automatically selected by the Makefile, with default parallelism
• The attribution regression test and mps_delay both execute and pass
• make lint-rust passes without warnings or errors
• make fmt-rust passes without changing any files
• git diff --check passes

This run used macOS Tahoe arm64, stable Rust, cargo-nextest, Homebrew libfaketime, and GNU timeout on PATH
Setting CI=1 ensures that the timing test cannot return early because faketime is missing

The earlier default-threading timeout with the built-in Rust test runner remains unexplained and was not rerun through that runner in this follow-up
The passing nextest result is not a claim that the earlier timeout has been fixed
The PR description now distinguishes the latest validation from those earlier runs

Both inline questions were already addressed in 9e93367
No source code, assertions, timeouts, or repository configuration were changed for this follow-up
Ready for another review

[chutten]

Ah, my mistake for thinking that eacute would be a single byte of UTF-8.

Switching to the metric's own fn ought to be the only change left.

[akasakariko]

@chutten The remaining review change is addressed in ff82808

The attribution test now calls the metric object's test_get_num_recorded_errors_sync member and no longer needs the MetricType import
The crate-private synchronous member accepts the test's local Glean instance, while the existing public getter delegates to it without changing the API

I also rebased onto e6d54f3 and resolved the changelog conflict while preserving both entries

Validation on the final revision, using Linux arm64 through Colima with Rust 1.90.0

• CI=1 RUST_LOG=glean_core=debug make test-rust passes with 438 passed and 10 ignored, including mps_delay
• make lint-rust and make fmt-rust pass
• SQLite compatibility checks pass with 175 passed and 2 ignored

The macOS full-suite mps_delay timeout remains undiagnosed and is documented in the PR description
It also reproduced on the unchanged previous PR head 9e93367, while the isolated timing test passes on the final revision
No assertions were weakened or timeouts extended

Ready for another review