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

[pkg/ottl] Add Sort converter sorting array value #34200

Closed
kaisecheng opened this issue Jul 22, 2024 · 7 comments
Closed

[pkg/ottl] Add Sort converter sorting array value #34200

kaisecheng opened this issue Jul 22, 2024 · 7 comments
Assignees
Labels

Comments

@kaisecheng
Copy link
Contributor

kaisecheng commented Jul 22, 2024

Component(s)

pkg/ottl

Is your feature request related to a problem? Please describe.

I wanna sort an array in attribute in ascending or descending order to improve readability and simplify data processing.

Describe the solution you'd like

The Sort() function should accept an input array and return a new array sorted in either ascending or descending order.

Sort(attributes["device.tags"], "asc")
Sort(attributes["device.tags"], "desc")

The function should support sorting arrays of different types, including integers, floats, strings, and booleans. For boolean arrays, elements will be converted to strings for comparison. For arrays containing mixed types (e.g., integers and strings), all elements will be converted to strings for comparison.

Describe alternatives you've considered

No response

Additional context

No response

@kaisecheng kaisecheng added enhancement New feature or request needs triage New item requiring triage labels Jul 22, 2024
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@kaisecheng
Copy link
Contributor Author

@evan-bradley I can work on this feature. Do you think the description is clear enough to start?

@evan-bradley evan-bradley added priority:p2 Medium and removed needs triage New item requiring triage labels Jul 26, 2024
@evan-bradley
Copy link
Contributor

This makes sense to me. @TylerHelmuth let us know if you have any concerns.

@TylerHelmuth
Copy link
Member

The values will only be compared as strings, but not actually changed, right?

@kaisecheng
Copy link
Contributor Author

The values will only be compared as strings, but not actually changed, right?

Yes. The underlying values are not changed. Sort() makes a copy. Numeric array compare as numbers. Bools, strings and mixed types compare as strings. Other types are returned as-is

@TylerHelmuth
Copy link
Member

Ok cool. I'd like to see a PR for this feature.

TylerHelmuth pushed a commit that referenced this issue Sep 5, 2024
**Description:** <Describe what has changed.>

Add `Sort` function to sort array to ascending order or descending order

`Sort(target, Optional[order])`

The Sort Converter preserves the data type of the original elements
while sorting.
The behavior varies based on the types of elements in the target slice:

| Element Types | Sorting Behavior                    | Return Value |
|---------------|-------------------------------------|--------------|
| Integers | Sorts as integers | Sorted array of integers |
| Doubles | Sorts as doubles | Sorted array of doubles |
| Integers and doubles | Converts all to doubles, then sorts | Sorted
array of integers and doubles |
| Strings | Sorts as strings | Sorted array of strings |
| Booleans | Converts all to strings, then sorts | Sorted array of
booleans |
| Mix of integers, doubles, booleans, and strings | Converts all to
strings, then sorts | Sorted array of mixed types |
| Any other types | N/A | Returns an error |


Examples:

- `Sort(attributes["device.tags"])`
- `Sort(attributes["device.tags"], "desc")`

**Link to tracking Issue:** <Issue number if applicable>


#34200

**Testing:** <Describe what testing was performed and which tests were
added.>

- Unit tests
- E2E tests

**Documentation:** <Describe the documentation added.>

readme for Sort function

---------

Co-authored-by: Evan Bradley <[email protected]>
f7o pushed a commit to f7o/opentelemetry-collector-contrib that referenced this issue Sep 12, 2024
**Description:** <Describe what has changed.>

Add `Sort` function to sort array to ascending order or descending order

`Sort(target, Optional[order])`

The Sort Converter preserves the data type of the original elements
while sorting.
The behavior varies based on the types of elements in the target slice:

| Element Types | Sorting Behavior                    | Return Value |
|---------------|-------------------------------------|--------------|
| Integers | Sorts as integers | Sorted array of integers |
| Doubles | Sorts as doubles | Sorted array of doubles |
| Integers and doubles | Converts all to doubles, then sorts | Sorted
array of integers and doubles |
| Strings | Sorts as strings | Sorted array of strings |
| Booleans | Converts all to strings, then sorts | Sorted array of
booleans |
| Mix of integers, doubles, booleans, and strings | Converts all to
strings, then sorts | Sorted array of mixed types |
| Any other types | N/A | Returns an error |


Examples:

- `Sort(attributes["device.tags"])`
- `Sort(attributes["device.tags"], "desc")`

**Link to tracking Issue:** <Issue number if applicable>


open-telemetry#34200

**Testing:** <Describe what testing was performed and which tests were
added.>

- Unit tests
- E2E tests

**Documentation:** <Describe the documentation added.>

readme for Sort function

---------

Co-authored-by: Evan Bradley <[email protected]>
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

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

No branches or pull requests

3 participants