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

Breaking changes v13 - labels as arguments considered as object #499

Open
nirapx opened this issue Mar 28, 2022 · 2 comments · May be fixed by #553
Open

Breaking changes v13 - labels as arguments considered as object #499

nirapx opened this issue Mar 28, 2022 · 2 comments · May be fixed by #553

Comments

@nirapx
Copy link

nirapx commented Mar 28, 2022

Hi,
When I'm setting labels as arguments and the first argument is an array:

someCounter.labels(arr, string1, string2)

it is considered as an object and throws Added label "0" is not included in initial labelset.
I found that there is a condition if (typeof args[0] === 'object') but it might also check that it is not an array.

I've upgraded from v12 to v14 (but it happens in v13 also).

Thanks

@zbjornson
Copy link
Collaborator

I don't think I understand what the use case and expectation are here. metric.labels() is supposed to accept an object of key-value pairs, or a series of strings that match up one-to-one with the metric's label names. An array shouldn't be passed here. Can you clarify please?

@nirapx
Copy link
Author

nirapx commented Mar 29, 2022

Yes.
When we used this library in v12 we could pass just arguments such as metric.labels(arg1, arg2, arg3).
One of the args was an array: metrics.labels(arr, arg2, arg3).
After upgrading to v13/14 the ability to pass an object was added also and as a result, the code above was broken.

The new code checks if the type of the first argument is an object but it returns true for array also.

zbjornson added a commit that referenced this issue Mar 11, 2023
This was undocumented and untested, and broke in 13.1.0.

Fixes #499
@zbjornson zbjornson linked a pull request Mar 11, 2023 that will close this issue
@zbjornson zbjornson self-assigned this Mar 11, 2023
zbjornson added a commit that referenced this issue Mar 11, 2023
This was undocumented and untested, and broke in 13.1.0.

Fixes #499
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.

2 participants