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

Parse instance scenarios into a flat WS URL list #888

Closed
inancgumus opened this issue May 17, 2023 · 2 comments · Fixed by #906
Closed

Parse instance scenarios into a flat WS URL list #888

inancgumus opened this issue May 17, 2023 · 2 comments · Fixed by #906
Assignees
Labels
enhancement New feature or request remote remote browser related
Milestone

Comments

@inancgumus
Copy link
Member

inancgumus commented May 17, 2023

The k6-browser can connect to browsers in a round-robin fashion by retrieving data from the K6_INSTANCE_SCENARIOS environment variable.

Problem

Currently, we consider a list of WS URLs.

However, instead of receiving a flat list of WS URLs, we will receive the following minified JSON via the K6_INSTANCE_SCENARIOS environment variable:

K6_INSTANCE_SCENARIOS="[{\"id\":\"one\",\"browsers\":[{\"handle\":\"ws://1...\"},{\"handle\":\"ws://2...\"}]},{\"id\":\"two\",\"browsers\":[{\"handle\":\"ws://3...\"}]}]"

The pre-minified JSON looks like the following (where id is a scenario ID/name):

[
    {
        "id": "one",
        "browsers": [
            { "handle": "ws://1..." },
            { "handle": "ws://2..." }
        ]
    },
    {
        "id": "two",
        "browsers": [
            { "handle": "ws://3..." }
        ]
    }
]

Solution

Currently, we don't support other browsers than Chromium. And we don't need to respect the scenario details to pool WS URLs in each scenario. So we can parse the source JSON into the following, a flat list of WS URLs by joining all scenarios into one as follows:

[ "ws://1...", "ws://2...", "ws://3..." ]

With this, we can still use the same feature set in the k6-browser without changing anything else. And we can be ready when we support other browsers (since we'll be getting WS URLs in scenarios based on environmental details like browser types).

Suggestion

Change the IsRemoteBrowser function to parse the scenario-based JSON data into a flat list of WS URLs.

@inancgumus inancgumus added enhancement New feature or request remote remote browser related labels May 17, 2023
@inancgumus inancgumus added this to the v0.10.0 milestone May 17, 2023
@ankur22
Copy link
Collaborator

ankur22 commented May 17, 2023

Now we are reading the WS URL in the mapBrowserToGoja function. The scope is per VU lifecycle? So we're parsing the map once every vu when it's probably ok to parse once and use when we need.

A possible solution to this could be:

  1. Parse the ENV variable once. So maybe when we create the RootModule?
  2. Keep the list of WS URLS in the RootModule
  3. Pass a reference to the list to the VUmodule when called to NewInstanceModule (same way we do for pidRegistry)
  4. And then for each iteration we can call something like.... vu.IsRemoteBrowser() Which would return true/false if it's remote, and if it is remote then it will return one WS URL randomly from the initially parsed list.

@inancgumus
Copy link
Member Author

inancgumus commented May 17, 2023

Hey, @ankur22! Thanks for tackling this 🙇

At this point, our primary goal is to ensure everything works smoothly with the BE data. To do this, we can easily transform the instance scenario JSON data into a more straightforward list of WS URLs. Simple as that!

I really like the solution you proposed! 😊 However, I think it might be a good idea to tackle them separately, perhaps in a different issue.

@ankur22 ankur22 assigned ankur22 and unassigned ankur22 May 17, 2023
@ankur22 ankur22 self-assigned this May 23, 2023
ankur22 added a commit that referenced this issue May 24, 2023
This new method parses K6_BROWSER_WS_URL differently. Instead of a list
of ws urls, it is a scenarios object, which needs to be unmarshalled
and all ws urls extracted. The end result is the same -- a list of ws
urls.

This is a step closer to being able to work with a set of chrome
browsers each with different characteristics that are set in the
scenario.

This commit contains the addition of this new method. Eventually this
method will be used instead of the existing newRemoteRegistry method.

Closes: #888
ankur22 added a commit that referenced this issue May 25, 2023
This new method parses K6_BROWSER_WS_URL differently. Instead of a list
of ws urls, it is a scenarios object, which needs to be unmarshalled
and all ws urls extracted. The end result is the same -- a list of ws
urls.

This is a step closer to being able to work with a set of chrome
browsers each with different characteristics that are set in the
scenario.

This commit contains the addition of this new method. Eventually this
method will be used instead of the existing newRemoteRegistry method.

Closes: #888
ankur22 added a commit that referenced this issue May 25, 2023
This new method parses K6_BROWSER_WS_URL differently. Instead of a list
of ws urls, it is a scenarios object, which needs to be unmarshalled
and all ws urls extracted. The end result is the same -- a list of ws
urls.

This is a step closer to being able to work with a set of chrome
browsers each with different characteristics that are set in the
scenario.

This commit contains the addition of this new method. Eventually this
method will be used instead of the existing newRemoteRegistry method.

Closes: #888
ankur22 added a commit that referenced this issue May 26, 2023
This new method parses K6_BROWSER_WS_URL differently. Instead of a list
of ws urls, it is a scenarios object, which needs to be unmarshalled
and all ws urls extracted. The end result is the same -- a list of ws
urls.

This is a step closer to being able to work with a set of chrome
browsers each with different characteristics that are set in the
scenario.

This commit contains the addition of this new method. Eventually this
method will be used instead of the existing newRemoteRegistry method.

Closes: #888
ankur22 added a commit that referenced this issue May 30, 2023
This new method parses K6_BROWSER_WS_URL differently. Instead of a list
of ws urls, it is a scenarios object, which needs to be unmarshalled
and all ws urls extracted. The end result is the same -- a list of ws
urls.

This is a step closer to being able to work with a set of chrome
browsers each with different characteristics that are set in the
scenario.

This commit contains the addition of this new method. Eventually this
method will be used instead of the existing newRemoteRegistry method.

Closes: #888
inancgumus added a commit that referenced this issue Jun 1, 2023
This is needed because we're receiving a sanitized (quoted) sceneario
JSON data from remote.

Related: #906 and #888.
inancgumus added a commit that referenced this issue Jun 1, 2023
This is needed because we receive the remote's sanitized (quoted) scenario JSON data.

Related: #906 and #888.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request remote remote browser related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants