Tail logs in k6 #1599

mstoykov · 2020-08-20T14:11:54Z

No description provided.

codecov-commenter · 2020-08-20T14:25:09Z

Codecov Report

Merging #1599 into master will decrease coverage by 0.96%.
The diff coverage is 36.09%.

@@            Coverage Diff             @@
##           master    #1599      +/-   ##
==========================================
- Coverage   74.90%   73.94%   -0.97%     
==========================================
  Files         164      165       +1     
  Lines       14156    14517     +361     
==========================================
+ Hits        10604    10734     +130     
- Misses       3014     3220     +206     
- Partials      538      563      +25

Impacted Files	Coverage Δ
cmd/run.go	`9.73% <0.00%> (-0.09%)`	⬇️
cmd/ui.go	`24.85% <0.00%> (+0.41%)`	⬆️
cmd/cloud.go	`8.23% <8.69%> (+0.18%)`	⬆️
stats/cloud/logs.go	`32.87% <32.87%> (ø)`
stats/cloud/config.go	`60.00% <33.33%> (-1.54%)`	⬇️
stats/cloud/cloud_easyjson.go	`35.14% <39.54%> (+1.41%)`	⬆️
stats/cloud/collector.go	`79.29% <0.00%> (+0.54%)`	⬆️
lib/executor/vu_handle.go	`95.49% <0.00%> (+1.80%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5703bfa...bc49c3b. Read the comment docs.

imiric · 2020-08-21T09:23:19Z

stats/cloud/logs.go

+
+				continue
+			}
+


Nit: are these additional newlines because of a linter? The ones before continue look particularly strange to me. :-/

Do you mean before continue - that is nlreturn
this one specifically I think is whitespace, but not sure ;)

Yeah, I'd vote for disabling nlreturn then, the extra newlines look out of place to me.

stats/cloud/logs.go

cmd/cloud.go

imiric

@mstoykov I tested it on prod now and it works great, but I think we should do something about the log output obscuring the progress bar. With k6 run the progress bars stay pinned to the bottom and any log output is printed above them. With the current k6 cloud the progress bar output is mixed with the logs, and with a lot of events scrolling by, the progress bar is only visible when the logs are quiet. Even scrolling back doesn't show it as the log output overwrites it.

imiric

LGTM, besides that env var nitpick. Thankfully that progress bar fix was straightforward and I like that both run and cloud are sharing the same progress bar rendering, even though that UI code could use some cleanup.

cmd/cloud.go

na-- · 2020-09-02T11:07:11Z

cmd/cloud.go

+}
+
+//nolint:gochecknoglobals
+var cloudLogsCmd = &cobra.Command{


Given the issues we've had, why not delay this the command to get old logs for k6 v0.29.0? As far as I understand, it's not going to work very well now, right? And it's far from essential, especially when we still have k6cloudlogs doing essentially the same thing? So, personally, I'd vote for not including this now.

This is a possibility, but likely I will be able to make the changes ... today or tomorrow.
But I agree that maybe it should be in a separate PR ...

I don't doubt we can make the changes, but why not wait a version for things to be a bit more stable? There's no rush to have this command, the other use case in this PR is much more important...

cmd/cloud.go

na-- · 2020-09-03T08:56:19Z

cmd/cloud.go

+		// we are going to set it  ... so we always parse it instead of it breaking the command if
+		// the cli flag is removed


hmm that's a good point, I haven't considered it in my "fix #883" ~~ideas~~ dreams and it's probably going to slightly complicate matters... 😞

cmd/cloud.go

stats/cloud/logs.go

cmd/cloud.go

stats/cloud/logs.go

na--

LGTM

na-- · 2020-09-03T11:38:23Z

stats/cloud/logs.go

+func (m *msg) Log(logger logrus.FieldLogger) {
+	var level string
+
+	for _, stream := range m.Streams {


Hmm this could potentially cause messages to arrive out-of-order, right? I doubt it's a big issue, if each msg encapsulates only a second, since each stream would be a separate instance?

I thought of this some minutes ago ... as /query_range apperantly doesn't return logs in any given order as well. I don't even know if tail does ... my testing - shows that - yes it does seem to come in order, but more investigation is required.

I fear I will need to get all the logs and then to order them and then print them :( But I propose that is a separate PR - this is unlikely to be a problem unless you have a lot of logs and arguably that probably is already pretty bad experience (also likely to lead to dropped logs due to k6 not being able to tail fast enough)

👍 fine for a separate PR, if needed...

na-- · 2020-09-03T11:52:50Z

This doesn't work well in a non-TTY mode 😭 Try running k6 -v cloud github.com/loadimpact/k6/samples/logging.js 2>&1 | cat and see... Though maybe that falls under #1580 😕

mstoykov · 2020-09-03T12:17:05Z

I don't think this PR is the place to fix progressbar issues ;)

na-- · 2020-09-03T12:24:22Z

Probably so, but this PR makes it worse, try to run the same example with k6 v0.27.1... So, yeah, the cause is #1580, but we definitely need to fix this before v0.28.0 is officially released

mstoykov · 2020-09-03T12:38:50Z

Again ... this is not an issue with this PR ... I am even not of the opinion this needs fixing in v0.28.0 or at least not any more than it needed until now - try running a script which runs for some time with v0.27.1.
Yes the cloud is worse as it takes longer to run, but on a 30 minute test I would argue the difference isn't there.

na-- · 2020-09-03T12:40:05Z

I think you simply didn't run the example I suggested you run above, it isn't that it takes longer, it simply looks nasty... Anyway, I'll try to fix these things in a new PR

mstoykov · 2020-09-03T12:47:10Z

I did run it ... the reason it looks nasty is because of 2 things:

the test takes long enough for there to be a couple of progressbar printings (arguably with more lines than required around them), or more accurately because the cloud needs to do stuff it takes a while to start which leads to even more progressbar printings.
because now there are cloud logs, this makes the progressbars to be printed ... even more
The same behavior can be seen if ran locally, just the test you proposed ... finishes locally way too fast for progressbars to be printed more than once or twice

mstoykov requested review from imiric and na-- August 20, 2020 14:11

imiric reviewed Aug 21, 2020

View reviewed changes

mstoykov requested a review from imiric August 21, 2020 14:47

imiric reviewed Aug 25, 2020

View reviewed changes

cmd/cloud.go Outdated Show resolved Hide resolved

imiric suggested changes Aug 25, 2020

View reviewed changes

mstoykov mentioned this pull request Aug 25, 2020

Override with env variables the cloud config #1583

Closed

mstoykov requested a review from imiric August 25, 2020 14:42

imiric previously approved these changes Aug 26, 2020

View reviewed changes

cmd/cloud.go Outdated Show resolved Hide resolved

na-- reviewed Sep 2, 2020

View reviewed changes

Update logrus to v1.6.0

c6d5a97

mstoykov dismissed imiric’s stale review via 78338ef September 2, 2020 11:56

mstoykov force-pushed the tailLogsInK6 branch from 030f27c to 78338ef Compare September 2, 2020 11:56

mstoykov requested review from na-- and imiric September 2, 2020 11:57

na-- added this to the v0.28.0 milestone Sep 2, 2020

na-- reviewed Sep 2, 2020

View reviewed changes

cmd/cloud.go Outdated Show resolved Hide resolved

mstoykov requested a review from na-- September 2, 2020 15:35

na-- reviewed Sep 3, 2020

View reviewed changes

stats/cloud/logs.go Outdated Show resolved Hide resolved

basic tailing from k6 cloud of the cloud logs

bc49c3b

mstoykov force-pushed the tailLogsInK6 branch from acfd812 to bc49c3b Compare September 3, 2020 10:37

na-- approved these changes Sep 3, 2020

View reviewed changes

mstoykov merged commit 0737194 into master Sep 3, 2020

mstoykov deleted the tailLogsInK6 branch September 3, 2020 12:39

na-- mentioned this pull request Jan 26, 2021

Console output when execution in cloud #978

Closed

na-- mentioned this pull request Feb 5, 2021

Configuration issues #883

Open

mstoykov mentioned this pull request Mar 16, 2021

Document k6 cloud show logs options grafana/k6-docs#238

Closed

mstoykov mentioned this pull request Apr 15, 2021

k6 cloud gets log error on long running tests #1966

Closed

mstoykov mentioned this pull request Dec 6, 2021

Deprecate and/or rename the --show-logs option in k6 cloud #2288

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tail logs in k6 #1599

Tail logs in k6 #1599

mstoykov commented Aug 20, 2020

codecov-commenter commented Aug 20, 2020 •

edited

Loading

imiric Aug 21, 2020

mstoykov Aug 21, 2020

imiric Aug 21, 2020

imiric left a comment

imiric left a comment

na-- Sep 2, 2020

mstoykov Sep 2, 2020

na-- Sep 2, 2020

na-- Sep 3, 2020

na-- left a comment

na-- Sep 3, 2020

mstoykov Sep 3, 2020

na-- Sep 3, 2020

na-- commented Sep 3, 2020

mstoykov commented Sep 3, 2020

na-- commented Sep 3, 2020

mstoykov commented Sep 3, 2020

na-- commented Sep 3, 2020

mstoykov commented Sep 3, 2020 •

edited

Loading

		// we are going to set it ... so we always parse it instead of it breaking the command if
		// the cli flag is removed

Tail logs in k6 #1599

Tail logs in k6 #1599

Conversation

mstoykov commented Aug 20, 2020

codecov-commenter commented Aug 20, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imiric left a comment

Choose a reason for hiding this comment

imiric left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

na-- left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

na-- commented Sep 3, 2020

mstoykov commented Sep 3, 2020

na-- commented Sep 3, 2020

mstoykov commented Sep 3, 2020

na-- commented Sep 3, 2020

mstoykov commented Sep 3, 2020 • edited Loading

codecov-commenter commented Aug 20, 2020 •

edited

Loading

mstoykov commented Sep 3, 2020 •

edited

Loading