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

Structured stacktrace? #9

Open
aca opened this issue Aug 25, 2023 · 4 comments
Open

Structured stacktrace? #9

aca opened this issue Aug 25, 2023 · 4 comments

Comments

@aca
Copy link

aca commented Aug 25, 2023

Current stacktrace just append strings, which is not good to read.
Can oops have some structured stacktrace implementation like https://github.com/rotisserie/eris ?

    "root":{
      "message":"error internal server",
      "stack":[
        "main.main:/Users/roti/go/src/github.com/rotisserie/eris/examples/logging/example.go:143",
        "main.ProcessResource:/Users/roti/go/src/github.com/rotisserie/eris/examples/logging/example.go:85",
        "main.ProcessResource:/Users/roti/go/src/github.com/rotisserie/eris/examples/logging/example.go:82",
        "main.GetRelPath:/Users/roti/go/src/github.com/rotisserie/eris/examples/logging/example.go:61"
      ]
    },
@samber
Copy link
Owner

samber commented Aug 25, 2023

I would say that internally, we definitely need a structured stacktrace, but the way we output the logs should be customizable.

Option 1:

What about a custom slog.Valuer, json.Marshaler, etc... ?

I think we can add some interfaces such as:

type LogValuerFormatter interface {
   Format(OopsError) slog.Valuer
}

type JsonFormatter interface {
   Format(OopsError) ([]byte, error)
}

type MapFormatter interface {
   Format(OopsError) map[string]any
}

type StdFormatter interface {
   Format(err OopsError, s fmt.State, verb rune)
}

const (
    DefaultLogValuerFormatter LogValuerFormatter = ...
    DefaultJsonFormatter         JsonFormatter           = ...
    DefaultMapFormatter          MapFormatter           = ...
    DefaultStdFormatter            StdFormatter            = ...
)

Option 2:

It might be the job of the logger to handle the error and format it.

Returning a slice would allow the logger to print it as a string.

Option 3:

A mix of both options 1 and 2.

This is probably the best option since oops.OopsError must implement fmt.Formatter + slog.Valuer + json.Marshaler from stdlib, and we don't want to import external dependencies by default (zap, logrus...).

WDYT ?

@aca
Copy link
Author

aca commented Oct 1, 2023

I'm sorry but I just hoped oops stacktrace is more readable for slog.TextHandler/slog.JsonHandler (the only logger I care since slog is in std) by default. If it's structured, It would also be possible get statistical results on the error with log.
I don't have much thought about customizability.

Instead of

"stacktrace": "Oops: permission denied\n  --- at github.com/samber/oops/examples/slog/example.go:30 d()\n  --- at github.com/samber/oops/examples/slog/example.go:34 c()\nThrown: something failed\n  --- at github.com/samber/oops/examples/slog/example.go:42 b()\n  --- at github.com/samber/oops/examples/slog/example.go:46 a()\n  --- at github.com/samber/oops/examples/slog/example.go:85 main()"

something like

"stacktrace": [
    "github.com/samber/oops/examples/slog/example.go:30:d()",
    "github.com/samber/oops/examples/slog/example.go:34:c()",
    "github.com/samber/oops/examples/slog/example.go:42:b()",
    "github.com/samber/oops/examples/slog/example.go:46:a()"
]

@samber
Copy link
Owner

samber commented Oct 2, 2023

The format would be:

[
   {
      "message":"error internal server",
      "stacktrace":[
          "github.com/samber/oops/examples/slog/example.go:30:d()",
          "github.com/samber/oops/examples/slog/example.go:34:c()",
          "github.com/samber/oops/examples/slog/example.go:42:b()",
          "github.com/samber/oops/examples/slog/example.go:46:a()"
      ]
   }
]

You can start with the update on the stacktrace package then LogValuerFormatter. I can help on JsonFormatter, MapFormatter and StdFormatter.

@maticmeznar
Copy link

Has there been any progress on cleaning up the stacktrace for structured formatting?

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

3 participants