xapi-project / xen-api

The Xapi Project's XenAPI Server
http://xenproject.org/developers/teams/xapi.html
Other
346 stars 283 forks source link

Instrument task related functionality #5735

Closed GabrielBuica closed 2 months ago

GabrielBuica commented 3 months ago

Instruments task related functions.

Some other improvements:

contificate commented 3 months ago

Pedantically, I'd prefer we worked towards avoiding infix usage of @@ as it's an eyesore.

We can use let (let*) = (@@) and write let* __context = e in e'. Would be interested to hear what others think (even if the topic of syntax is as bikeshedding as it gets).

lindig commented 3 months ago

I don't have strong opinions on @@; I think the usage here was appropriate to wrap existing code. With the suggestion, we might run into the problem that let* is used for other purposes but we would use let@, I assume.

GabrielBuica commented 3 months ago

I quite like the visual of @@ for with_tracing functions as they are mostly called at the start of the functions and you know that the entire scope will be inside at span. The downside is that it's not easy to follow that the argument is being changed: span/__context/dbg/etc.

As far as I am aware let* is used as a bind operator and from a quick search through the codebase let@ is defined as let ( let@ ) f x = f x . So, if we agree that a custom let operator is more readable I'd use the later.

Vincent-lau commented 3 months ago

Looks good to me, but might need another eye from some one who's familiar with tracing as it is a relatively large change