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

Logger implementation should log non-error messages as breadcrumbs. #41

Open
empz opened this issue Jun 8, 2021 · 6 comments
Open

Logger implementation should log non-error messages as breadcrumbs. #41

empz opened this issue Jun 8, 2021 · 6 comments

Comments

@empz
Copy link

empz commented Jun 8, 2021

The default behaviour of any Sentry SDK is to capture non-error logs as breadcrumbs and send them all together as soon as an error happens.

I see SentryService extends Logger but the methods log, debug, verbose and warn are actually calling captureMessage and I think they should be storing that log as a breadcrumb in the current scope.

This way, replacing the default NestJS logger with this library would be a no-brainer and we'd get proper log messages before an exception on Sentry.

@ntegral
Copy link
Owner

ntegral commented Jun 8, 2021

Will add this to the backlog thanks

@empz
Copy link
Author

empz commented Jun 9, 2021

Would you accept a PR? I've already forked and implemented it.

@ntegral
Copy link
Owner

ntegral commented Jun 14, 2021

Thanks.. I'll review it @empz

@ntegral
Copy link
Owner

ntegral commented Jan 5, 2022

This feature as been added in the latest version 3.0.7

@dg-eparizzi
Copy link

This feature as been added in the latest version 3.0.7

I see you've added a parameter to SentryService to log a non-error message as breadcrumb. Still this is not optimal as I'm using the SentryService as the default logger and I need to go and add that extra argument on each log call....
I understand you don't want to set this as the default (although it makes way more sense this way) to avoid breaking changes, but wouldn't be a global config much easier to set up?

Somewhere in the forRoot config we should have the option to say "store non-error messages as breadcrumbs".

@bawahakim
Copy link

@dg-eparizzi Created a PR here, but I did it in the forked version from travelerdev. I think this would fit your use case. I can also create a PR here if relevant

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

4 participants