Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Add tracing for debuging the js procedure slowness #1552

Merged
merged 2 commits into from
Jun 20, 2022

Conversation

Blu-J
Copy link
Contributor

@Blu-J Blu-J commented Jun 17, 2022

While debuging why configSet was slow, was using this trace. @chrisguida thought it would be nice if others could trace this too.

@Blu-J Blu-J requested a review from dr-bonez June 17, 2022 18:14
@chrisguida
Copy link
Contributor

This will be extremely useful during our switch from docker to JS

Comment on lines 67 to 75
tracing::trace!(
"Procedure execute {} {} - {:?}",
match self {
PackageProcedure::Docker(_) => "docker",
PackageProcedure::Script(_) => "JS",
},
pkg_id,
name
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeesh this is a lot of visual space for a trace log. Is there any way to make it more innocuous?
I may encourage display instances for Package Procedure and then you can eliminate the case statement and probably smash this to a single line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the idea. It did reduce the vertical garbage

Copy link
Contributor

@ProofOfKeags ProofOfKeags left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make display instance for package procedure. If Display would be wrong, write a custom function for rendering. This should make the match statement go away here and reduce visual clutter to a single line, which is appropriate for a trace log.

@Blu-J Blu-J merged commit 1f5e6db into master Jun 20, 2022
@Blu-J Blu-J deleted the chore/tracing-for-js branch July 11, 2022 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants