k6
k6 copied to clipboard
Support non-indexable high-cardinality metric tags/metadata
This is built on top of https://github.com/grafana/k6/pull/2594 and should close https://github.com/grafana/k6/issues/2584 and fix https://github.com/grafana/k6/issues/2653 when it's done.
A bunch of tests still remain to be ported or written, and some of the outputs need to be adjusted, but it should be suitable for initial review even now. Just keep in mind this: https://github.com/grafana/k6/issues/2584#issuecomment-1219618111
Actually, besides addressing PR comments and maybe a few more tests, I am not sure if anything else is needed in this PR here :thinking: So I'll remove the draft status of this PR, it's officially ready for reviews :tada:
For anyone watching, after some internal discussions, we are likely not going to merge this PR as it is.
We are cannibalizing it for parts and trying to merge the parts nobody objects to (most of the Go internals and refactoring, and the Metadata
map in metrics.Sample
) in k6 v0.41.0, while leaving the rest (any JS API changes that are not purely refactoring) for further consideration in the future. See https://github.com/grafana/k6/issues/2584#issuecomment-1269775631 for more details.
I rebased the code with the new changes from master. There is a backup branch in case you would check the original version.
Hmm :thinking: it would have probably been simpler and safer to keep the old branch in this PR and just close it (without merging) and open a new PR with the new version. Because, when you have a PR with a branch, you can safely delete and then restore the branch without any fears. But if the free-standing backup-non-indexable-tags
branch is deleted, it won't be recoverable, so it will just clutter the branches list.
I also removed the previously added
metrics.TagRawURL
after an internal discussion. We will open a dedicated issue for it. There is also a small change in the latest fixup from the original version.
The reason the original code was like that was to optimize it slightly. In your code, strconv.Itoa()
and strconv.FormatBool()
will always be called, even when they are not needed (e.g. when the tags are disabled). It's a very minor optimization, since these tags are enabled by default, so the readability improvements are probably more important.
@olegbespalov can you please comment on the new PR?
@codebien, just for my understanding, will this PR be closed? Shouldn't I check them anymore? :thinking:
Yes, we are going to rollback this PR to the previous history and close then move in two more defined PRs.
Closing this for the reasons stated in https://github.com/grafana/k6/pull/2654#issuecomment-1269779558 and https://github.com/grafana/k6/issues/2584#issuecomment-1269775631
In short, we merged the internal parts of this PR in other PRs (https://github.com/grafana/k6/pull/2716, https://github.com/grafana/k6/pull/2726, https://github.com/grafana/k6/pull/2727) and we will continue iterating on a better solution for the JS APIs that expose the high-cardinality metrics.Sample.Metadata
for user scripts.