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

Refactor how cloud options are specified in the script file #1155

Closed
sniku opened this issue Sep 12, 2019 · 5 comments · Fixed by #3348
Closed

Refactor how cloud options are specified in the script file #1155

sniku opened this issue Sep 12, 2019 · 5 comments · Fixed by #3348

Comments

@sniku
Copy link
Collaborator

sniku commented Sep 12, 2019

This is a proposal for renaming options.ext.loadimpact to options.cloud.

// current structure
export let options = {
    ext: {
        loadimpact: {
            name: "My test name",
            projectID: 12345,
            staticIPs: true,
        }
    }
};

// proposed structure
export let options = {
    cloud: {
        name: "My test name",
        projectID: 12345,
        staticIPs: true,
    }
};

The command used to run cloud tests is k6 cloud. It's logical to expect that options for cloud would be specified in options.cloud rather than options.ext.loadimpact.

Connected issues: #587, #883

@sniku sniku added the refactor label Sep 12, 2019
@na-- na-- added evaluation needed proposal needs to be validated or tested before fully implementing it in k6 ux labels Sep 12, 2019
@na-- na-- added this to the v1.0.0 milestone Apr 22, 2020
@na-- na-- modified the milestones: v1.0.0, TBD Nov 9, 2022
@markjmeier
Copy link

With the upcoming release of Grafana Cloud k6 - I want to propose bumping priority on this. We should have caught this before the last cycle, but it was missed.

Users signing up and using k6 through Grafana Cloud will have no context around loadimpact, so this is very likely to cause confusion. considering that the issue has been open for 4 years, we should take a stab at fixing it finally - because it is important debt.

I would propose, at minimum, that we:

  1. support cloud as an alternative option name to loadimpact
  2. silently support loadimpact for existing customers (determine timeline to remove later)
  3. start emitting a WARN for customers still using loadimpact
  4. In the event that someone sets both, give cloud priority over loadimpact

I see a note that this is blocked. Is that the full implementation as laid out originally? Can we refactor within what we already have to also support options.ext.cloud before taking on the blocked effort?

cc: @dgzlopes

@dgzlopes
Copy link
Member

During our planning call, we agreed on something similar to your proposal. After this is completed, the next steps would be to build a consensus for a long-long-term solution (what's happening with ext?) and implement the solution.

@allansson
Copy link
Contributor

Since this is a breaking change I'd like to address an inconsistency in the current options type that we potentially could fix.

There are two properties that does not follow the camel case naming convention used by JS and other properties in the options: drop_metrics and drop_tags.

If we cannot outright drop them, a potential solution would be to add dropMetrics and dropTags but still support the old properties and then add @deprecated to the old properties in our type definitions.

@mstoykov
Copy link
Contributor

@allansson most of the options including those two are not part of k6 OSS, and it just copies them around. These two are entirely and only parsed by the cloud backend. So while related to this, k6 OSS can't really do anything about them.

@allansson
Copy link
Contributor

@allansson most of the options including those two are not part of k6 OSS, and it just copies them around. These two are entirely and only parsed by the cloud backend. So while related to this, k6 OSS can't really do anything about them.

Hmm, I see. I'll continue my quest for consistency elsewhere then. Thanks for the quick answer! 🙏

@olegbespalov olegbespalov moved this from Mid term - Q1 2024 to Short term - Q4 2023 in k6 open-source public roadmap Jan 24, 2024
@github-project-automation github-project-automation bot moved this from Short term to Released in k6 open-source public roadmap Feb 13, 2024
@olegbespalov olegbespalov removed the evaluation needed proposal needs to be validated or tested before fully implementing it in k6 label Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

8 participants