ref(managed): Add Managed::zip function. by pierceroberts · Pull Request #6088 · getsentry/relay

pierceroberts · 2026-06-13T05:34:44Z

Added a generic Managed::merge helper that consumes two managed values and returns a single Managed<(T, S)>,
transferring outcome responsibility into the merged wrapper while preserving metadata from the first value.

Also added a unit test covering the new behavior: the merged value retains both inner values, emits outcomes
for both through the first managed handle when dropped, and prevents the second managed value from emitting
duplicate outcomes after the merge.

I also ran:

pre-commit --files
cargo test -p relay-server:

test result: ok. 383 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.67s

also ran the new test individually:

   cargo test --package relay-server --lib --all-features -- managed::managed::tests::test_merge_into_tuple --exact --nocapture 

    Finished `test` profile [unoptimized] target(s) in 0.78s
     Running unittests src/lib.rs (target/debug/deps/relay_server-ca0b589ee2561a09)

running 1 test
test managed::managed::tests::test_merge_into_tuple ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 406 filtered out; finished in 0.00s

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

Dav1dde · 2026-06-15T07:01:05Z

+    where
+        S: Counted,
+    {
+        let (first, meta) = first.destructure();


Should be able to build this with existing primitives, e.g. map, similar to merge_with.

That would also assert that the Counted impls for (T, S) matches T + S

I updated this to use map, take a look!

Dav1dde · 2026-06-15T07:03:43Z

+    {
+        let (first, meta) = first.destructure();
+        let second = second.accept(|second| second);
+        Managed::from_parts((first, second), meta)


There is a problem here where the meta of the two managed instances does not match. We should at least add a debug_assert to assert they are matching.

Because I changed this to use map, I deleted the use of meta. I believe this is now obsolete.
Let me know if you think differently. Thanks!

Dav1dde · 2026-06-15T07:04:54Z

+    ///
+    /// The returned instance uses the metadata from `first`. `second` is accepted, transferring
+    /// outcome responsibility to the merged instance.
+    pub fn merge<S>(first: Self, second: Managed<S>) -> Managed<(T, S)>


Should be next to the merge_with method.

loewenheim · 2026-06-15T07:17:25Z

+    ///
+    /// The returned instance uses the metadata from `first`. `second` is accepted, transferring
+    /// outcome responsibility to the merged instance.
+    pub fn merge<S>(first: Self, second: Managed<S>) -> Managed<(T, S)>


I'd argue a function that turns F<A> and F<B> into F<(A,B)> should be called zip. The new function's signature also doesn't match with merge_with.

zip is a good name, the initial idea of merge was a signature of merge(A, B, f) -> S where f: FnOnce(A, B) -> S but I think zip is a powerful enough as the merge could be implemented as zip().map()

Okay sounds good. I can rename the function. zip makes sense.

…test.

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit c1e3f13. Configure here.}

cursor · 2026-06-18T07:22:02Z

+
+            let second = second.accept(|second| second);
+            (first, second)
+        })


Zip skips metadata equality check

Medium Severity

Managed::zip accepts two Managed values with no check that they share the same Meta, yet it routes all later outcomes through first’s metadata after second is accepted. Mismatched scoping or outcome aggregators can mis-attribute or drop quota outcomes for the second value in release builds.

^{Reviewed by Cursor Bugbot for commit c1e3f13. Configure here.}

Would be great if you can assert that at least the scoping of the Meta matches and document the behaviour that zip will use the meta of the first instance.

Bonus: if you add a test for it

pierceroberts · 2026-06-18T20:28:49Z

Alright I got the main pipelines to pass @Dav1dde and I made the changes. Let me know what you think! Thanks.
It does look like there are some integration tests for python that are failing silently:

requests.exceptions.ConnectionError: Connection refused by Responses - the call doesn't match any registered mock.

https://github.com/getsentry/relay/actions/runs/27785392311/job/82223873618?pr=6088#step:7:166

Dav1dde

Thanks, looks great!

Documenting the behaviour of what happens to the meta is important to me to make it clear it's not an oversight or "random", the debug_assert is useful because we should never merge two Managed instances from different origins.

Dav1dde · 2026-06-19T11:40:00Z

+
+            let second = second.accept(|second| second);
+            (first, second)
+        })


Would be great if you can assert that at least the scoping of the Meta matches and document the behaviour that zip will use the meta of the first instance.

Bonus: if you add a test for it

Dav1dde · 2026-06-19T11:40:54Z

+        first.map(|first, recordkepping| {
+            for (category, quantity) in second.quantities() {
+                recordkepping.modify_by(category, quantity as isize);


Suggested change

first.map(|first, recordkepping| {

for (category, quantity) in second.quantities() {

recordkepping.modify_by(category, quantity as isize);

first.map(|first, records| {

for (category, quantity) in second.quantities() {

records.modify_by(category, quantity as isize);

Just me nitpicking to mirror the implementation of merge_with.

I can do that. It was also a spelling error 😅

Dav1dde · 2026-06-19T11:45:41Z

Changed the title to make the CI check happy

Init managed::merged function.

99c1964

pierceroberts requested a review from a team as a code owner June 13, 2026 05:34

Dav1dde requested changes Jun 15, 2026

View reviewed changes

loewenheim reviewed Jun 15, 2026

View reviewed changes

Move fn near merge_with, rename to zip, update fn to use map, update …

c1e3f13

…test.

cursor Bot reviewed Jun 18, 2026

View reviewed changes

run make style-rust lint-rust.

8558392

pierceroberts requested review from Dav1dde and loewenheim June 18, 2026 20:28

Dav1dde added the skip-changelog label Jun 19, 2026

Dav1dde reviewed Jun 19, 2026

View reviewed changes

Dav1dde changed the title ~~Init managed::merged function.~~ ref(managed): Add Managed::zip function. Jun 19, 2026

Conversation

pierceroberts commented Jun 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Legal Boilerplate

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loewenheim Jun 15, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jun 18, 2026

Choose a reason for hiding this comment

Zip skips metadata equality check

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pierceroberts commented Jun 18, 2026

Uh oh!

Dav1dde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dav1dde commented Jun 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pierceroberts commented Jun 13, 2026 •

edited

Loading

loewenheim Jun 15, 2026 •

edited

Loading