Thank you Doug for the awesome work here, we're very excited about this kind of macros for tracing systems!
Yeah I can see how body macros can get tricky and thus the "only one" limitation to avoid, what I assume to be, the complexity of having to type check the whole function body again and again if multiple body macros were to be combined? I think that limitation is probably fair...
Sadly it'll affect any macro that wants to make use of task-locals, but I think that is okey. At least for the tracing APIs these are convenience macros, so one can always go back to writing the withSpan
by hand if some other body replacement macro would necessarily have to work on a function.
Thought experiment:
Is there some way we get away without "wrapping the user-provided function body" for @Traced
I attempted to see if we could get away without "body replacement" if we for example exposed "unsafe task local value push/pop" APIs, for purposes of implementation of such wrapper macros, however those also run into trouble.
Task locals are actually implemented by a pair of "push and pop" the value, and that's implemented using a push
and defer { pop }
pattern around the wrapped code. So we could consider exposing such unsafe APIs (getting the pop order wrong WILL immediately crash at runtime), and try to implement @Traced
as:
func myMath(a: Int, b: Int) -> Int {
var context: ServiceContext // get "current" from task-local
// ...
_unsafeTaskLocalPush(ServiceContext.$current, value: context) // fictional
let span = startSpan("Doing complicated math", context: context)
defer {
span.end()
_unsafeTaskLocalPop(ServiceContext.$current) // fictional
}
// user code ~~~
return a + b
}
So... for the bare-bones thing that could work if we exposed such unsafe APIs, however... the tracing APIs require more from us; E.g. we're required to record errors in a span like this:
do {
// user code
catch {
span.setError(error)
span.setStatus ...
throw error
}
So, we're unable to express this with a preamble anyway.
This leads me to conclude that the @Traced
will be forced into staying a body replacement macro. But on the other hand -- it only is "convenience" and people can drop to writing with withSpan
relatively easily, so if more body macros were to compete over being necessary for a function -- it is not a huge problem to remove the traced macro.
Back to questions posed by the proposal though.
I lean towards the same type-checking scheme as currently proposed... so that the macro is able to introduce a span
member, though I see the problems it may cause to tooling.
I wonder if @ahoppen might be able to chime in how macros relate to lsp based tools? Would sourcekit-lsp and plugins based on it, like vscode, be able to query the compiler after the file has been compiled and realize the type of span
if it were introduced by a body replacement macro? If yes, that's awesome news and let's definitely stick to this. If no... then we might want to think some more but it does seem like the mode we'd like to go for with whole body replacements I guess.
It is not a blocker for tracing per se, one can always write the withSpan manually. But it is very common to provide additional attributes to a span, so it is kind of nice to allow users access to it... If we don't, then I'm thinking we'll very likely be forced someday to stuff the Span also into a task local which would be unfortunate, as we've so far managed to not do so
Would it be still allowed to @Body @Preamble func henlo() {}
?
Thanks again Doug for the pitch, it's looking great!