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

Stop sorting keys. #4

Merged
merged 1 commit into from
Jun 10, 2015
Merged

Stop sorting keys. #4

merged 1 commit into from
Jun 10, 2015

Conversation

betawaffle
Copy link
Contributor

I don't know why this was done originally, but sorting the keys breaks most multi-dimensional metrics. The test cases seem to have been very lucky.

@jaqx0r
Copy link
Contributor

jaqx0r commented Jun 10, 2015

The sorting was to make the tests stable :) I can improve the comparison
in the tests only to avoid the sort.

Can you show me an example that is breaking so I can test correctness?

On 10 June 2015 at 01:30, Andrew Hodges notifications@github.com wrote:

I don't know why this was done originally, but sorting the keys breaks

most multi-dimensional metrics. The test cases seem to have been very lucky.

You can view, comment on, or merge this pull request online at:

#4
Commit Summary

  • Stop sorting keys.

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#4.

jaqx0r added a commit that referenced this pull request Jun 10, 2015
@jaqx0r jaqx0r merged commit b1aff6b into google:master Jun 10, 2015
@betawaffle betawaffle deleted the stop-sorting-keys branch June 10, 2015 11:01
@betawaffle
Copy link
Contributor Author

Sure.

  • Define a metric with two (or more) dimensions, in my case, the keys were page and edge.
  • Use values that sort differently the keys, in my case, page was a number like 1234567 and edge was a word like comments.

Both the keys and the values were being sorted before being zipped together, causing edge to be matched with 1234567 and page to be matched with comments.

counter my_multidimensional_metric by page, edge

/some pattern with an (?P<edge>\w+) and a (?P<page>\d+)/ {
  my_multidimensional_metric[$page][$edge]++
}

The above code results in a metric that looks like this:

# TYPE my_multidimensional_metric counter
my_multidimensional_metric{edge="1234567",page="comments",prog="example.mtail",instance="example.com"} 1 1433934627

@jaqx0r
Copy link
Contributor

jaqx0r commented Jun 11, 2015

Ohhh that is a total facepalm. Thanks!

On 10 June 2015 at 21:12, Andrew Hodges notifications@github.com wrote:

Sure.

  • Define a metric with two (or more) dimensions, in my case, the keys
    were page and edge.
  • Use values that sort differently the keys, in my case, page was a
    number like 1234567 and edge was a word like comments.

Both the keys and the values were being sorted before being zipped
together, causing edge to be matched with 1234567 and page to be matched
with comments.

counter my_multidimensional_metric by page, edge

/some pattern with an (?P\w+) and a (?P\d+)/ {
my_multidimensional_metric[$page][$edge]++
}

The above code results in a metric that looks like this:

TYPE my_multidimensional_metric counter

my_multidimensional_metric{edge="1234567",page="comments",prog="example.mtail",instance="example.com"} 1 1433934627


Reply to this email directly or view it on GitHub
#4 (comment).

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

Successfully merging this pull request may close these issues.

2 participants