-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[chore] Method that sorts Datapoint Slice by Attribute key #23068
[chore] Method that sorts Datapoint Slice by Attribute key #23068
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also regenerate all golden files so we can see what the expected change is.
internal/coreinternal/golden/testdata/sort-datapoint-slice/before.yaml
Outdated
Show resolved
Hide resolved
name: nsxt.node.network.io | ||
sum: | ||
aggregationTemporality: 2 | ||
dataPoints: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have some additional data points that exercise the sorting of attributes within a map? Further, can we have some that require comparison beyond the first key?
- a
- b, a
- c, b
- c, b, a
- c, a, d
startTimeUnixNano: "1111111111111111111" | ||
timeUnixNano: "1111111111111111111" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to omit these fields for this test, as they only distract from the point of the test.
This is already all the changes when running the sort datapoint slice. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already all the changes when running the sort datapoint slice.
Ok, I suppose this makes sense because we generally want data points to have the same attribute keys.
Since this PR doesn't improve the golden files, I think it makes sense to extend it to cover resources and scopes, where I expect we'll see meaningful improvements.
For data points, I think a followup PR will make sense, which layers on sorting by value, since the values tend to be static or even enumerated.
Thanks @BominRahmani, @dehaansa! |
Adds a method that allows Metrics Datapoint slices of Golden files to be sorted by their respective attribute key. This is in the efforts to bring Golden files 1 step closer to being more deterministic.
There is a test folder (sort-datapoint-slice) in golden/testdata that shows an example of what this accomplishes.