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.

Sign up for GitHub

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

Jump to bottom

Report usages of the deprecated 'ext.loadimpact' option #4084

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Report usages of the deprecated 'ext.loadimpact' option #4084

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion cmd/test_load.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,11 +265,19 @@ func loadSystemCertPool(logger logrus.FieldLogger) {
func (lct *loadedAndConfiguredTest) buildTestRunState(
configToReinject lib.Options,
) (*lib.TestRunState, error) {
// This might be the full derived or just the consodlidated options
// This might be the full derived or just the consolidated options
if err := lct.initRunner.SetOptions(configToReinject); err != nil {
return nil, err
}

// Here, where we get the consolidated options, is where we check if any
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added it here because it looks like a considerably safe place among all the places configuration is read/written and manipulated in general, but any suggestion will be more than welcome.

Copy link
Contributor Author

@joanlopez joanlopez Dec 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Also, note that this will be reported despite it's a local (k6 run) or a remote execution (k6 cloud), but I guess it's fine because a script that holds ext.loadimpact is still a candidate of experiencing issues if we remove such support.

In any case, if we want to get more accurate metrics (only cloud executions), we can always account reports with this and a test run id.

// of the deprecated options is being used, and we report it.
if _, isPresent := configToReinject.External["loadimpact"]; isPresent {
if err := lct.preInitState.Usage.Strings("deprecated_options", "ext.loadimpact"); err != nil {
return nil, err
}
}

// it pre-loads system certificates to avoid doing it on the first TLS request.
// This is done async to avoid blocking the rest of the loading process as it will not stop if it fails.
go loadSystemCertPool(lct.preInitState.Logger)
Expand Down
Loading