-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat: Integrating the web dashboard feature #3505
Conversation
As per our product DNA, we aim, by integrating the web dashboard (xk6-dashboard) directly into k6, to enhance and streamline the user experience, making real-time and end-of-test visualizations immediately accessible as part of the k6 command-line experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @szkiba,
thanks for your contribution. Congrats for the progress with the dashboard 👏
I've left some comments.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #3505 +/- ##
=======================================
Coverage 72.96% 72.96%
=======================================
Files 280 280
Lines 20938 20949 +11
=======================================
+ Hits 15277 15286 +9
- Misses 4691 4693 +2
Partials 970 970
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
Will wait until the rest of the environment variables are implemented, and we resolve some of the discussions we're having internally to approve. But looks good to me, great job @szkiba 👏🏻
Thank you @oleiade What environment variables are you referring to?
see: https://github.com/grafana/xk6-dashboard#environment In addition, the see: https://github.com/grafana/k6/blob/feat/3504-dashboard-integration/cmd/config.go#L45 |
This is my bad @szkiba, I came in with the wrong assumption. You're right, they should likely remain defined in the extension itself 👍🏻 |
Co-authored-by: Ivan <2103732+codebien@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current code looks good to me.
But, unfortunately, there is something in the UX that sounds unexpected.
If I send an interrupt (CTRL+c) while k6 is running and the dashboard is opened in the browser then k6 gets stuck.
@szkiba is it expected?
I tested running the following command:
K6_WEB_DASHBOARD=true go run . run -d 1m ./examples/http_get.js
@codebien Actually, this is a feature (as long as the browser is open, k6 does not stop after the test run), but it really does not handle the abort situation well. I will fix it and make a patch release soon. update: done, v0.7.1 handle the test abort situation. xk6-dashboard dependency upgraded to v0.7.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The interrupt logic it's now fixed for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
As per our product DNA, we aim, by integrating the web dashboard (xk6-dashboard) directly into k6, to enhance and streamline the user experience, making real-time and end-of-test visualizations immediately accessible as part of the k6 command-line experience.
This is not yet the final version of the integration. The goal is to integrate xk6-dashboard v0.7.0, but that release does not exist yet (waiting for dependencies).
I created the PR to ask for a review and to get feedback on whether the integration would be good this way, or if the team had something else in mind.