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

Log features #235

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

diegolovison
Copy link
Contributor

With the current implementation ( by color ) the output in Jenkins is the following. This PR aims to improve that by adding the logging level.

qDup with the code on master will product the following on Jenkins

�[m�[37m18:28:23.018 run-hello-world-setup:2012@mwperf-server03: stop-env: 
�[m�[37m18:28:23.417 starting 3 scripts
�[m�[37m18:28:23.417 hyperfoil-start-in-container:2057@mwperf-server03: script-cmd: hyperfoil-start-in-container
�[m�[37m18:28:23.418 run-benchmark:2059@mwperf-server03: script-cmd: run-benchmark
�[m�[37m18:28:23.418 run-application:2061@mwperf-server03: script-cmd: run-application
�[m�[37m18:28:23.418 hyperfoil-start-in-container:2057@mwperf-server03: hyperfoil-start-in-container
�[m�[37m18:28:23.418 run-application:2061@mwperf-server03: run-application
�[m�[37m18:28:23.418 run-benchmark:2059@mwperf-server03: run-benchmark
�[m�[37m18:28:23.830 run-application:2061@mwperf-server03: cd /tmp/quarkus-quickstarts/getting-started-reactive
�[m�[37m18:28:24.424 run-application:2061@mwperf-server03: regex: getting-started-reactive.*started in
�[m�[37m18:28:24.425 reached APP_RUNNING
�[m�[37m18:28:24.425 run-benchmark:2059@mwperf-server03: wait-for: APP_RUNNING
�[m�[37m18:28:24.425 run-application:2061@mwperf-server03: signal: APP_RUNNING
�[m�[37m18:28:34.253 hyperfoil-start-in-container:2057@mwperf-server03: regex: ControllerVerticle deployed
�[m�[37m18:28:34.254 reached HYPERFOIL_CONTROLLER_STARTED
�[m�[37m18:28:34.254 run-benchmark:2059@mwperf-server03: wait-for: HYPERFOIL_CONTROLLER_STARTED
�[m�[37m18:28:34.254 hyperfoil-start-in-container:2057@mwperf-server03: signal: HYPERFOIL_CONTROLLER_STARTED
�[m�[31m18:28:34.255 File does not exist for upload: /var/jenkins_home/workspace/test/assets/example.hf.yaml
�[m�[31m18:28:34.255 run-benchmark:2059@mwperf-server03: failed to upload /var/jenkins_home/workspace/test/assets/example.hf.yaml to /tmp/example.hf.yaml

@willr3
Copy link
Collaborator

willr3 commented Jan 17, 2025

Why do you want to log the command before it is executed?
We have a mode --stream-logging where qDup will log the command when it is sent and log each line of output as it arrives. This might be able to provide what you want.
We do not log the command before the output finishes by default so that the resulting run.log is readable the same as a bash history. I suspect there is a different problem you are trying to solve that can be accomplished without changing the default behavior for all users.

Part of the roadmap is to change qDup logging to jboss-logging to facilitate native compilation with quarkus. Perhaps as part of that we could make the log configuration customizable to more easily change the log level when invoking qDup?

@willr3
Copy link
Collaborator

willr3 commented Jan 17, 2025

qDup creates two different logs. the run.log and qdup.log.
We want the run.log to look similar to a user's terminal in terms of commands followed by their output.
The qdup.log can include more information about code execution inside qdup

@diegolovison
Copy link
Contributor Author

Why do you want to log the command before it is executed?

#231

We have a mode --stream-logging where qDup will log the command when it is sent and log each line of output as it arrives.

Can it be the default behavior?

Perhaps as part of that we could make the log configuration customizable to more easily change the log level when invoking qDup?

Can the logging level be the default behavior?

@willr3
Copy link
Collaborator

willr3 commented Jan 17, 2025

Can it be the default behavior?

Stream logging works when qDup does not have parallel execution but it is difficult to read when there are multiple parallel scripts. Most of our "production" use cases have multiple parallel scripts so we do not have stream-logging as the default behavior.

Can the logging level be the default behavior?

I do not understand what exactly you mean by this. Do you mean changing the trace logging to info logging? That was put at trace level because it was effectively logging method entry and that is not something that is normally logged at info level.
If you mean the change to add logging level to run.log then I not think that is something we should change because it changes the purpose of the run.log. I think logging the level is suitable for qdup.log but that does not log to console. The console output is designed to look like terminal output merging the multiple remote terminals into one stream.

@willr3
Copy link
Collaborator

willr3 commented Jan 17, 2025

we could add the log level to the /tmp/qdup.log logging?

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.

2 participants