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

Function cannot read tag value from options.tags object directly #2278

Closed
knittl opened this issue Dec 1, 2021 · 4 comments · Fixed by #2631
Closed

Function cannot read tag value from options.tags object directly #2278

knittl opened this issue Dec 1, 2021 · 4 comments · Fixed by #2631

Comments

@knittl
Copy link
Contributor

knittl commented Dec 1, 2021

Here's an interesting find I had yesterday: It is not possible to read a tag from the options.tags object directly. You will always end up with undefined. However, if you serialise (JSON.stringify) and parse again, you can access the values.

I encountered the problem in an exported custom handleSummary function, but it is reproducible in the same way in the default function. Maybe there's a canonical (better) way to access the (user-defined) tags in the handleSummary function?

Here is a simple snippet to reproduce this issue. I assume this is caused by the fact that options.tags can be updated by the runtime – e.g. command line flag --tag tagname=tagvalue – and are not a "real" (?) JavaScript object, but rather go objects? So likely a go-js interop issue and not goja issue? Interestingly though, that JSON.stringify seems to be unaffected by this.

export const options = {
  tags: { mytag: 'myvalue' },
};

console.log(options.tags.mytag); // in init context shows correctly as "myvalue"

export default function() {
  console.log(JSON.stringify(options.tags)); // shows object correctly, including all properties with values
  console.log(options.tags.mytag); // undefined
  console.log(options.tags['mytag']); // undefined
  console.log(JSON.parse(JSON.stringify(options.tags)).mytag); // shows correctly as "myvalue"
}

I ran the above script in k6 versions 0.26.0 through 0.35.0 via Docker containers and all exhibit the same behavior. It's not a big problem, since I can work around the issue by roundtripping through JSON.stringify/parse, but it definitely is a surprising behavior which I wouldn't expect as a user.

If you need further info, please let me know.

@knittl knittl changed the title Function cannot directly get tag value from options.tags object Function cannot read tag value from options.tags object directly Dec 1, 2021
@na-- na-- added bug evaluation needed proposal needs to be validated or tested before fully implementing it in k6 js-compat labels Dec 1, 2021
@knittl
Copy link
Contributor Author

knittl commented Dec 2, 2021

Maybe noteworthy: Scenario-specific tags don't suffer from this:

export const options = {
  tags: { mytag: 'myvalue' },      
  scenarios: {
    bug: {
      executor: 'shared-iterations',
      tags: { scntag: 'scnvalue' },
    },
  },
};

export default function() {
  console.log(options.tags.mytag); // missing, prints undefined
  console.log(options.scenarios.bug.tags.scntag); // correctly prints "scnvalue"
}

This might be explained by the fact that scenario tags cannot be set via the CLI.

@mstoykov
Copy link
Contributor

mstoykov commented Dec 2, 2021

Hi @knittl,

The issue is that options get overwritten after they get merged with everything else (cli flags, env and so on). This overwriting though for some reason is really complicated and in this particular case options.tags is just set to its internal type that is a wrapper around a map, not just a map. As such it does not have properties such as mytag.

It does however implement marshalling to JSON which exposes the internal map so going to JSON and back to js object in practice makes it serialize the internal map and then that gets back as a simple map instead.

In the case of the scenarios the thing is just that the underlying data structure in go is just a map, so it just gets accessed.

I am pretty sure that at least for one of all the options this adds a race condition or something - which is pretty bad.

Arguably the instead of the complicated reflect based code we should just json.Marshal the options and be done with it, but there are other problems with this whole set of the options and arguably this should just have an API that can give you what you need like mentioned in #2259.

Currently, you should be able to use the new (as of v0.35.0) k6/execution.vu.tags to get the tags' value.

I don't really know what the next step here is, but I kind of think it will be best if we first get the API to get the whole options in some okay form and then start to give warnings to people who try to use the options and possibly even deprecate that or stop trying to at least set it 🤷

@na-- na-- removed the evaluation needed proposal needs to be validated or tested before fully implementing it in k6 label Dec 3, 2021
@knittl
Copy link
Contributor Author

knittl commented Dec 5, 2021

@mstoykov, thanks for the answer. Is there a reason why it needs to be the internal type/wrapper and not a regular map? Or why this internal type cannot provide access to its inner map values?

Thanks for the hint about k6/execution, I haven't thought about this before. It is a viable solution for my problem and I'm going to use it. JSON.parse(JSON.stringify(options.tags)) wasn't too bad either as workaround, but a bit awkward and confusing.

Warning users when they try to read options.tags[someKey] is a good first step to avoid confusion in the future. If users are going to be prevented from accessing any options properties, then all of them should/must be exposed via k6/exeuction. I'm currently relying on being able to serialize the effective options object for my test reports (reading a single tag was an independent problem).

@na--
Copy link
Member

na-- commented Aug 15, 2022

We just realized that we inadvertently fixed this as a side-benefit of the refactoring in #2631 😅 So it will be released in k6 v0.40.0 in a few weeks 🎉 That PR was a precursor for a much bigger metrics refactoring PR (#2594, which unfortunately got delayed for v0.41.0) and we just now realized it had other actual benefits it provided... 😅

In any case, I'd still advise any k6 users to use the much more predictable and accessible k6/execution API instead instead of trying to access the exported options directly. It provides both test.options and vu.tags, and it's globally available by default.

FWIW, another bugfix that will land in k6 v0.40.0 is #2571, which fixed the bug of having global variables in the main script being mistakenly made into superglobals for other imported files. This has been a problem for some users that relied on the bug (e.g. #2623) and importing k6/execution seems much easier to me than messing with globalThis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants