Add WorkflowTracer Through some Runtime Internals

[steve-the-edwards]

This can allow users to inject some tracing into the workflow runtime internals that are not reachable by the WorkflowInterceptor via a cheap and easy API.

[pyricau]

since we're trying to trace otherwise very fast operations, I would suggest to stick to static strings and avoid any string concat to avoid accidentally increasing time. Especially since workerType and key aren't strings already.

[steve-the-edwards]

Done

[steve-the-edwards]

• Add tests for tracer

[zach-klippenstein]

How much overhead is there to adding a no-op try/finally? I realize the only other alternative is duplicating the block here, which is probably also bad but in different ways.

[steve-the-edwards]

hmm good question. I'd have to look at the decompiled byte code I guess

[steve-the-edwards]

given that its inline if I branch it to only use the try / finally when the tracer is non-null, what is the other concern you referred to? I think this would be fine as it would be inlined and then the unused branch shaken out of the code?

[pyricau]

then the unused branch shaken out of the code?

There's no shaking out at compile time as whether WorkflowTracer is null or not is determined at runtime.

I'm not aware of any try / finally cost, however the resulting bytecode here shows we're checking whether WorkflowTracer is null twice, which also means we're reaching into the workflow tracer field twice.

This avoids the double null check:

public inline fun <T> WorkflowTracer?.trace(label: String, block: () -> T): T {
  return if (this == null) {
    block()
  } else {
    beginSection(label)
    try {
      block()
    } finally {
      endSection()
    }
  }
}

Hopefully we're never running into a case where a workflow tracer ref changes in between a begin and end, but if it did this code here is more correct (symmetry)

[steve-the-edwards]

Done.

[zach-klippenstein]

Nit: WorkflowRunner( can move up to previous line now that it's wrapped.

[pyricau]

• Remove this function and update the documentation on the other one to call out that we shouldn't use interpolated strings for label, so keep costs low.

[pyricau]

Should these checks be debug only? Or toggeable off to delete their runtime cost?

I guess we'll see them show up in traces if they're a problem, so good job adding this

[steve-the-edwards]

Just waiting for verification using our internal test suite.