fralalonde / dipstick

Configurable metrics toolkit for Rust applications
Apache License 2.0
99 stars 9 forks source link

Feature: CancelGuard #74

Closed vorner closed 5 years ago

vorner commented 5 years ago

Hello

I've noticed that usually when I want to have something cancelable (eg. I have a CancelHandle or anything that implements Cancel), I often want to wrap it into a guard that cancels on drop.

I wonder if it makes sense to:

I can send the pull request if you like the idea.

fralalonde commented 5 years ago

The behavior would certainly be desired. How is the Guard different from having blanket impl Drop for Cancel? Currently, buffered Scopes impl Drop to flush their buffer as a final action. Would the Guard pattern be preferable there too?

vorner commented 5 years ago

I think in three ways:

In other word, the guard makes it an explicit request to „activate“ the cancel on drop.

fralalonde commented 5 years ago

Oh, I got my "blanket" wrong. So, would the Handle need to be explicitly wrapped with a Guard by the client?

I had also thought about making Flush-On-Drop behavior optional but would have done so with an Output attribute flag, e.g. output.flush_on_drop(false).new_scope(). The flag value would be copied to the Scope upon creation and looked at in the Drop impl to decide if flushshould be called or not. This way the same type (Scope) is returned not matter the behavior.

vorner commented 5 years ago

Flush on drop probably doesn't hurt if it happens and you didn't want it. Cancel on drop does hurt when you don't want it.

Anyway, this is what we have now:

let observe_cancel = bucket
    .observe(bucket.gauge("answer"), |_| 42))
    .on_flush();
// A bulk of computation or something happens here
observe_cancel.cancel();

What I propose is this:

let _guard = bucket
    .observe(bucket.gauge("answer"), |_| 42))
    .on_flush()
    .into_guard(); // Provided method in Cancel, that just takes self and calls CancelGuard::new(self)
// The computation happens here
// Canceled here by drop
fralalonde commented 5 years ago

Ok, if you have a PR I'll merge it