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

Tags sort! in aggregation breaks with frozen arrays #380

Open
ianks opened this issue Sep 25, 2024 · 1 comment
Open

Tags sort! in aggregation breaks with frozen arrays #380

ianks opened this issue Sep 25, 2024 · 1 comment

Comments

@ianks
Copy link

ianks commented Sep 25, 2024

When aggregation is enabled, statsd-instrument attempts to Array#sort!. This breaks things when frozen arrays are used, as it attempts to mutate the array.

This blew up in production for me since our test suite doesn't enable aggregation, since doing so would cause assert_statsd_* assertions to fail.

Ideally, we wouldn't need to call Array#sort because that incurs an allocation. In my specific case, the array was a simple TAGS = ["foo:bar"].freeze, so sorting was not even necessary. If we must sort, maybe a good middle ground would be something like:

if tags.length > 1
  tags.sort
else
  tags
end

Thoughts?

cc: @pedro-stanaka since you've been working on this feature

@pedro-stanaka
Copy link
Contributor

pedro-stanaka commented Sep 25, 2024

We need sorting because in terms of aggregation:

[foo:bar, baz:bloo] is the same as [baz:bloo, foo:bar].

With regards to your solution, I believe it is fine to do like that. And even smart, tbh.

Would you mind putting up a PR?

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

No branches or pull requests

2 participants