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

export Sanitizer and handle time, errors, stringers #204

Conversation

veqryn
Copy link
Contributor

@veqryn veqryn commented Dec 6, 2023

Goal

Right now, the only way to convert arbitrary any to something bugsnag can show well, is to use the MetaData.AddStruct method.
Except it has 3 problems:

  • It ideally wants a struct
  • At no point are the filters set correctly
  • errors, time.Time, time.Duration, and lots of other things just show up as an empty {} in bugsnag

Design

This problem can be solved by exporting Sanitizer, and adding some special handling for well known types.

Changeset

See diff

Testing

Add tests pass, and I am using this locally

@clr182 clr182 added the backlog We hope to fix this feature/bug in the future label Feb 2, 2024
@clr182
Copy link
Contributor

clr182 commented Feb 2, 2024

Hi @veqryn

Apologies about the delay in response time and thank you for opening this PR with us.

The changes made in this PR seem reasonable. When priority allows will we look to merge this into our go notifier. We will be sure to update you once we have more information to share.

@DariaKunoichi
Copy link
Contributor

Hi @veqryn

Your change is ready to be merged in #215. Except we are still keeping "sanitizer" class private.
Did you notice a problem with filters not getting applied?
For filters to work you need to set "ParamsFilters" in configuration.
Filtering does not occur on each metadata add step - but at the very end, in the "MarshalJSON" function for payload.

If there is still a problem with filters please create an issue with more detailed description.

@veqryn
Copy link
Contributor Author

veqryn commented Feb 28, 2024

Hi @DariaKunoichi ,
Can I return sanitizer to being private in my PR, and then let this PR get merged?
It is nice to have contributor PR's get merged, as then we get credit and it encourages future contributions.
thanks

@DariaKunoichi
Copy link
Contributor

Hi, @veqryn of course you can return it to private in your own branch.
But unfortunately I still have to cherry pick it in a different PR (and fix CI tests) because our CI will not run for external contributor's PR.
You are still credited when I cherry-pick commits from your fork.
Your commits will remain in history and you'll be credited in the release notes.

@veqryn
Copy link
Contributor Author

veqryn commented Mar 5, 2024

@DariaKunoichi ok. thank you

@mclack
Copy link

mclack commented Mar 14, 2024

Hi @veqryn

This has now been addressed in #215, which is part of the bugsnag-go v2.3.0 release.

Thank you for your contributions!

@mclack mclack closed this Mar 14, 2024
@mclack mclack added released This feature/bug fix has been released and removed backlog We hope to fix this feature/bug in the future labels Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released This feature/bug fix has been released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants