many: metrics option for stage durations#317
Conversation
|
Ready for re-reviews. This is how it looks now, I think the pipeline should probably go after the stage name, but stage name is delivered with comma already so I would need to parse it or something: |
I don't understand your idea, but having the pipeline at the beginning makes sense. Pipelines are encapsulating stages and are the higher-level component, so it makes sense to present the pipeline name before the stage name. TBH, it may make sense to present it in a tree-like view, where all stages are nested under the pipeline, rather than having an unordered list of stages from various pipelines.
An unfortunate thing IMHO is that the stage is called a |
|
[...]
The json is based on a spec from @achilleas-k, iirc we was open for tweaks, we only use it internally so far and it would be a bit painful but probably still okay to make changes at this point (rather than later) |
It is ordered by the execution time. Let me know guys how should I continue this. As I stated already, I would like to use this same code in composer and send the data as trace logs so we can have statistics for the hosted image builder service. |
Sorry, I missed that. I think that the core of the problem from my PoV is that all pipelines are mixed. If the sorting based on execution time were to stay, but the stages were grouped by pipeline name, that would be much better. I'm not sure if and how that would affect your next steps for the service. |
There was a problem hiding this comment.
Any presentation issues can, IMHO, be done as followups I have some ideas as well about how we could make this prettier or more useful to a command line user.
For example, totals per pipeline; in a prett(y|ier) table (or not) or exported seperately as trace data.
I could argue about the --with prefix of the argument since historically (as far as we have history) this implies that an additional file would be written into the output directory. I won't :)
|
Rebased for new github workflows. |
|
Rebased, renamed the option to |
thozza
left a comment
There was a problem hiding this comment.
Code-wise, the PR looks good to me.
However, the last commit says many: statistics option for stage durations, while it adds --with-metrics option. Moreover, metrics and statistics are both used in different packages. It would probably make sense to use a single common name to describe the feature everywhere, unless I'm missing some good reason to use different names in different places.
|
Oh that is a good catch, this was a leftover from "statistics" to "metrics" renaming. I searched and replaced all of them hopefully, as well as instances in commit messages. Rebased. |
|
Rebased, also renamed the remaining struct type name and the |
When something needs to be printed after the progress bar is done, it was not possible to do so without messing up the terminal. This patch adds a mutex around the buffer used to store output while the progress bar is active. This allows to safely print after stopping the progress bar.
This commit introduces a new metrics option for the image building stages, allowing users to get more insights into the duration of each stage.
This PR introduces a new statistics option for the image building stages, allowing users to get more insights into the duration of each stage.
Split out from: #303
I need to re-test, got a bunch of conflicts and I am not confident that the code is okay. Feel free to drop comments tho.