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

Support more types for message template tags in SentryLogger #1945

Merged
merged 5 commits into from
Sep 26, 2022

Conversation

babyjohn42
Copy link
Contributor

@babyjohn42 babyjohn42 commented Sep 23, 2022

Adds support for all primitive types (long integers, etc.), and enums when converting objects to strings to be used for tags. Applies to the SentyLogger used by Sentry.Extensions.Logging and packages that depend on it such as ASP.NET Core.

@babyjohn42
Copy link
Contributor Author

Implements #1944 if desired

@SimonCropp
Copy link
Contributor

@babyjohn42 thanks. this will be included in the next release

@mattjohnsonpint
Copy link
Contributor

I added a changelog entry. Looks good otherwise. Thanks!

@SimonCropp
Copy link
Contributor

I added a changelog entry. Looks good otherwise. Thanks!

@mattjohnsonpint lol. u just beat me t it

@SimonCropp
Copy link
Contributor

@mattjohnsonpint should we also support short and the unsigned variants?

@SimonCropp SimonCropp enabled auto-merge (squash) September 23, 2022 09:45
@mattjohnsonpint
Copy link
Contributor

@mattjohnsonpint should we also support short and the unsigned variants?

Actually, now that I think about it, it should probably support any primitive or enum value. It shouldn't support other object types though, because they are tags. Complex objects don't make sense for tags. I'll make a small adjustment to this PR.

@mattjohnsonpint mattjohnsonpint changed the title Support long for message template tags in SentryLogger Support more types for message template tags in SentryLogger Sep 23, 2022
@babyjohn42
Copy link
Contributor Author

babyjohn42 commented Sep 23, 2022

Thanks all, I definitely agree that the primitive types make a ton of sense to be handled as tags in this way. I just wasn't sure if there were concerns with how Sentry.Extensions.Logging handled primitive types versus the other logging integrations, so I only included long in my initial commit. Thanks for the feedback and adjustment!

@mattjohnsonpint
Copy link
Contributor

Having some issues with our PR validation pipeline, but this will be merged once resolved. Thanks.

@SimonCropp SimonCropp merged commit bd6f566 into getsentry:main Sep 26, 2022
@babyjohn42 babyjohn42 deleted the feat/support-long-in-ilogger branch October 7, 2022 14:41
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.

Support 'long' built-in type for message template parameter in Sentry.Extensions.Logging
4 participants