mirrord
mirrord copied to clipboard
feat: initial progress reporting
Initial version of progress reporting via tracing spans, closes #389.
Reporting is done in 4 flavors:
- standard mode: fancy reporting with spinners
- simple mode: simple messages printed
- json: not implmented
- off: the layer is completely disabled
The tasks start when a span is first entered, and end when the span is closed (dropped), the reason for this is async code where a span is repeatedly entered whenever the futures are polled.
In standard mode the same spinner is reused for all nested spans, and the ongoing task is only done when the outermost span is closed.
standard mode example
https://user-images.githubusercontent.com/25967296/190286758-cbd82564-7f60-4fe5-8bc9-4b0d8b9cabd7.mp4
The agent setup ended up being too quick when I actually got to try it out so I had to add artificial network delays for the vid above...
I also introduced a mirrord-common
library for stuff like this that are used from both the cli and the layer.
Todo
- I cannot seem to pass env vars to the layer from the cli, so by default the layer has standard reporting enabled as well.
- We only check if a span is closed, if it's closed, success is printed, even if the task at hand failed. I'm not sure how to handle this, I'd say it's probably not a huge issue as the error is immediately printed, but a false success message is still ugly or probably even confusing.
- Each time I write a tracing layer I need to rediscover how tracing works, so the code is a bit messy and requires a cleanup.
The example looks amazing! We'll try to review it ASAP!
@tamasfe Hey, wanted to check if you were planning to get back to this one anytime soon? Feel free if you want us to pick it up where you left off.
@tamasfe Hey, wanted to check if you were planning to get back to this one anytime soon? Feel free if you want us to pick it up where you left off.
Sorry, I didn't mean to vanish. I felt burnt-out lately and avoided Github altogether.
I realized that this feature isn't really mirrord-specific, so I decided to overengineer it as a separate library for the following reasons:
- No control over the thread that prints the spinner line
- Only one line can be shown at a time
- It's not possible to lock stdout, so the spinner might overwrite messages
My library allows for multiple lines at the same time while retains all their state and allows for finer control so that a thread is not always running for no reason. I also wanted indentation of subtasks.
Also I'm not sure if tracing is the right approach anymore. Yes, it's somewhat convenient, but at the same time it was not designed for this use-case. Once a subscriber is registered, it cannot be removed. I think progress reporting like this is niche enough not to be part of the main logging library.
I don't think I'll get back to this before next week. If you like this approach I can continue then or polish the mentioned library and open a separate PR with that. Either way, feel free to take it over if you have the capacity, I don't want to hold anyone up.
We're in no rush to get this merged, so if you're planning to get back to it sometimes in the near future we'll leave it to you. I think we do like the current approach.
Closing in favor of #537.