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

feat(logging): exposing breadcrumbs gathered with errors.Wrap()/Wrapf()/etc when trySkip() fails #1404

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

vingarzan
Copy link

The code, very nicely, gathers breadcrumbs as it goes deep in the structures while generating. Unfortunately, since these were not exposed during logging, it was very hard to determine where the issues originated in a complex OAD.

Added also a couple of names to some errors, for more "bread-crumbing".

}
g.log.WithOptions(zap.AddCallerSkip(1)).Info(msg,
zapPosition(l),
zap.String("reason_error", reason),
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This, unfortunately, stripped the msg (and frame) from the errors. %w seems to at least nicely unwrap the msg at least, for a good compromise. The frame might also be valuable, but for a different target user.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use %+v to unwrap full chain of errors if I correctly understand the problem

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I tried that also, but it was too verbose. This seemed a good compromise.

@ernado
Copy link
Member

ernado commented Feb 17, 2025

Please use conventional commit names, like feat(gen): expose breadcrumbs in logging

@vingarzan vingarzan force-pushed the logging-improvements branch from faa7401 to 4bf4718 Compare February 17, 2025 18:28
@vingarzan
Copy link
Author

Please use conventional commit names, like feat(gen): expose breadcrumbs in logging

Sorry about that, I misread the guide on it...

@ernado
Copy link
Member

ernado commented Feb 18, 2025

Sorry, it is still not per conventional commits 🙂

https://www.conventionalcommits.org/en/v1.0.0/#summary

You are adding a new feature, so you can use prefix feat.
To describe a scope, you can use feat(scope), like feat(logging).

For tests, use test:, for cleaning up, go mod, dependency bumps, linter issues, etc you can use chore.

For updating CI/CD I use ci, like ci(e2e): update.

…()/etc when trySkip() fails

The code, very nicely, gathers breadcrumbs as it goes deep in the structures while generating. Unfortunately, since these were not exposed during logging, it was very hard to determine where the issues originated in a complex OAD.
@vingarzan vingarzan force-pushed the logging-improvements branch from 4bf4718 to 39ab34d Compare February 18, 2025 08:41
@vingarzan vingarzan changed the title logging:gen: exposing (as DEBUG/INFO) breadcrumbs gathered with errors.Wrap()/Wrapf()/etc when trySkip() fails feat(logging): exposing breadcrumbs gathered with errors.Wrap()/Wrapf()/etc when trySkip() fails Feb 18, 2025
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