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

Keep order of arguments in SlogHandler #611

Open
jgrieger opened this issue Jan 4, 2024 · 2 comments
Open

Keep order of arguments in SlogHandler #611

jgrieger opened this issue Jan 4, 2024 · 2 comments
Labels
proposal Proposal to add a new feature to pterm proposal-accepted Proposals which are accepted for implementation in the future

Comments

@jgrieger
Copy link

jgrieger commented Jan 4, 2024

Affects v0.12.74

Expected behavior

SlogHandler keeps order of arguments when printing, similar to slog.JSONHandler or slog.TextHandler

Actual behavior

The order of arguments being printed seems to be random.
See the following example with slog.TextHandler output compared to SlogHandler output:

First run:

time=2024-01-04T15:27:52.845+01:00 level=INFO msg="this is an args test" compiler=gc "Go version"=go1.21.5 platform=windows arch=amd64 machine=my-host
2024-01-04 15:27:52 INFO  this is an args test
                      ├ compiler: gc
                      ├ Go version: go1.21.5
                      ├ platform: windows
                      ├ arch: amd64
                      └ machine: my-host

Second run:

time=2024-01-04T15:27:54.497+01:00 level=INFO msg="this is an args test" compiler=gc "Go version"=go1.21.5 platform=windows arch=amd64 machine=my-host
2024-01-04 15:27:54 INFO  this is an args test
                      ├ platform: windows
                      ├ arch: amd64
                      ├ machine: my-host
                      ├ compiler: gc
                      └ Go version: go1.21.5

The arguments order of slog.TextHandler output is kept, the arguments order of SlogHandler output varies.

@jgrieger jgrieger added the bug Something isn't working label Jan 4, 2024
@MarvinJWendt MarvinJWendt added proposal Proposal to add a new feature to pterm and removed bug Something isn't working labels Jan 5, 2024
@MarvinJWendt MarvinJWendt changed the title SlogHandler does not keep order of arguments Keep order of arguments in SlogHandler Jan 5, 2024
@MarvinJWendt MarvinJWendt added the proposal-accepted Proposals which are accepted for implementation in the future label Jan 5, 2024
@MarvinJWendt
Copy link
Member

MarvinJWendt commented Jan 5, 2024

Thanks for making this issue.

I am wondering if we should sort the arguments alphabetically, or if we should keep the original order.

  • Sorting it alphabetically would make it easier to find a specific argument in a long list, but the order would not be customizable.
  • Keeping the original order would be closer to the raw slog implementation, but it would require more refactoring and the output would be harder to read (sometimes).

A third option would be to make the behavior configurable. Maybe an option in the Logger like SortArguments, which sorts the arguments alphabetically if set to true and if false, the original order is kept.

EDIT: I'll most likely go with option 3.

@jgrieger
Copy link
Author

jgrieger commented Jan 5, 2024

Thanks for making this issue.

I am wondering if we should sort the arguments alphabetically, or if we should keep the original order.

  • Sorting it alphabetically would make it easier to find a specific argument in a long list, but the order would not be customizable.
  • Keeping the original order would be closer to the raw slog implementation, but it would require more refactoring and the output would be harder to read (sometimes).

A third option would be to make the behavior configurable. Maybe an option in the Logger like SortArguments, which sorts the arguments alphabetically if set to true and if false, the original order is kept.

EDIT: I'll most likely go with option 3.

Agreed 💯. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Proposal to add a new feature to pterm proposal-accepted Proposals which are accepted for implementation in the future
Projects
None yet
Development

No branches or pull requests

2 participants