Macro defaults: enter_on_poll & scope

[taqtiqa-mark]

I've reached the Model stage of the pipeline is #113. This involves setting up defaults....

Current macro implementation has some confusing, to my mind, defaults. Described here.

Proposal is for these defaults (macro impl details follow, but you get the idea):

• enter_on_poll:
  • None (sync)
  • Some(true) (async)
• span-scope:
  • Local (sync, is current default),
  • Local (async) & this follows from enter-on-poll default (above) being true (correct?)

If enter_on_poll=false then convention is for default span-scope to be Threads.

My expectation is that if you just want to add #[trace], it is likely you're new or (more likely) an occasional user needing to debug something now - you're already having a lousy day, and we shouldn't force you to dive into docs to find out what you need to do to get the help you need... in these cases enter-on-poll is likely to be what you want when debugging? Also if you're new to async we don't hide the effect of polling.

Regular users will already know what they want.

I believe these defaults are consistent with the rust-lang goal of 'empowering everyone' here it is new/occasional users. Also for these users their productivity is enhanced - at a cost to regular users who want a different default. However thos regular/power users are still empowered to do what they want, just with some additional cost of having to enter their desired parameters.

Thoughts?

[andylokandy]

I appreciate you bringing these up. I haven't been able to come up with an overwheelming better design. But I could exhibit some ideas behind the current design for later evolution/refactoring.

Before v0.4, minitrace has two separate macros, respectively trace and trace_async. And on v0.4, we figured out a way to merge these two macros, namely by detecting async-trait output and elaborating async fn into fn -> impl Future. From the API aspect, I tried to keep them consistent in both sematic(rendered as a single span) and usage(need local parent context on the first call).

Thus, If enter_on_poll becomes the default behavior, the consistency will be broken: fn records only one span for each call while async fn may record more than one fragile span; fn need local parent context on the first call while async fn needs local parent context on each poll.

[andylokandy]

cc @zhongzc

[taqtiqa-mark]

Not sure I understand:

Thus, If enter_on_poll becomes the default behavior, the consistency will be broken:

To clarify: You'd like there to be consistency, by default, in the number of spans generated by one call of a sync function and and one call of an async function. Correct?

Related: Am I correct in understanding that enter_on_poll is only applicable to async functions?

[andylokandy]

To clarify: You'd like there to be consistency, by default, in the number of spans generated by one call of a sync function and and one call of an async function. Correct?

Correct

Am I correct in understanding that enter_on_poll is only applicable to async functions

Correct

[taqtiqa-mark]

To clarify: You'd like there to be consistency, by default, in the number of spans generated by one call of a sync function and and one call of an async function. Correct?

Correct

Ok, I see the symmetry in having that as default behavior. However, the point of using async is to have your code run concurrently, or in parallel, with other functions. From the perspective of a user that has just changed:

#[trace]
fn io_bound(){ ... }

into

#[trace]
async fn io_bound(){ ... }

I believe they would expect to see, say in Jaeger, multiple spans for io_bound, interleaved with other functions, and would likely, incorrectly, believe they have made an implementation error whereby their async fn is only run once.
POLS (principle of least surprise) suggests enter_on_poll=true as the async default. From that the other defaults follow.

What is the use case/scenario where having async functions appear once adds value?

[andylokandy]

What is the use case/scenario where having async functions appear once adds value?

The first request that I know so far is https://github.com/tikv/minitrace-rust/issues/64#issuecomment-936271952

[taqtiqa-mark]

What is the use case/scenario where having async functions appear once adds value?

The first request that I know so far is #64 (comment)

Thanks. That was informative. I understood those comments as requesting the option to make async function appear to be sync. That option is now present.

However, I did not understand them to be making the case the option should be the default. In fact they are use-case/context specific. Further, in the 6-months that issue was open only two (2) users showed up with a need for that option.

I'd tend to agree with @zhongzc comment in that issue, and take this to be the use of tracing in 80% of cases:

I'm not trying to convince you, but that outcome is really what I am hoping for as from that graph I can figure out that:

• When were futures suspended and when were they resumed
• How long was the real execution time and how long was the waiting time, not just the complete time

However, I am trying to convince you to go back to the default behavior. for the reasons @zhongzc outlined, and the one I set out earlier.

[andylokandy]

ping @zhongzc

[taqtiqa-mark]

Voided by PR #127 rejection.