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

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

Open
na-- opened this issue Dec 6, 2021 · 2 comments
Open

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

na-- opened this issue Dec 6, 2021 · 2 comments
Labels
breaking change for PRs that need to be mentioned in the breaking changes section of the release notes cloud evaluation needed proposal needs to be validated or tested before fully implementing it in k6 refactor ux

Comments

@na--
Copy link
Member

na-- commented Dec 6, 2021

--show-logs / K6_SHOW_CLOUD_LOGS is an option that determines if the local k6 process in a k6 cloud test run will connect to the k6 cloud logs server and retrieve the collected logs from the remote k6 instances executing the load test.

k6/cmd/cloud.go

Lines 367 to 368 in ee8d9bd

flags.BoolVar(&showCloudLogs, "show-logs", showCloudLogs,
"enable showing of logs when a test is executed in the cloud")

if showCloudLogsEnv, ok := os.LookupEnv("K6_SHOW_CLOUD_LOGS"); ok {

k6/cmd/cloud.go

Lines 302 to 309 in ee8d9bd

if showCloudLogs {
go func() {
logger.Debug("Connecting to cloud logs server...")
if err := cloudConfig.StreamLogsToLogger(globalCtx, logger, refID, 0); err != nil {
logger.WithError(err).Error("error while tailing cloud logs")
}
}()
}

There are a few issues with it:

  • its name is somewhat misleading, since the actual option has more to do with retrieving the logs than displaying them; for example k6 cloud --show-logs --log-output=none will retrieve the logs, but it certainly won't show them, since the --log-output=none option will prevent that... Multiple log outputs #2287 will make this even more complicated...
  • its environment variable is K6_SHOW_CLOUD_LOGS and doesn't match other k6 cloud environment variables, which start with the K6_CLOUD prefix
  • finally, it's configured in an ad-hoc way in cmd/cloud.go, while the rest of the cloud options are in cloudapi/config.go, including the actual WebSocket URL that will be used to retrieve the cloud logs! 😲
    LogsTailURL null.String `json:"-" envconfig:"K6_CLOUD_LOGS_TAIL_URL"`

So, yeah, all of these things probably have to be improved, starting with a better name that more closely reflects the nature of the option and doesn't conflict with other log-related options...

@na-- na-- added ux cloud refactor evaluation needed proposal needs to be validated or tested before fully implementing it in k6 breaking change for PRs that need to be mentioned in the breaking changes section of the release notes labels Dec 6, 2021
@mstoykov
Copy link
Contributor

mstoykov commented Dec 6, 2021

Discussion around the name for some more context

@na--
Copy link
Member Author

na-- commented Dec 6, 2021

Definitely not one of my better suggestions... 😊 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change for PRs that need to be mentioned in the breaking changes section of the release notes cloud evaluation needed proposal needs to be validated or tested before fully implementing it in k6 refactor ux
Projects
None yet
Development

No branches or pull requests

2 participants