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

Simple change: Use Default Sentry Settings #36

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

Conversation

FxllenCode
Copy link

Solves #35

Uses the Default sentry settings as stated in Documentation of the Sentry crate.

Allows replacement of info such as env and release by using environmental variables, as described in the docs:

https://docs.sentry.io/platforms/rust/configuration/options/

Very simple change but may solve some issues.

Example of addition below:

image

Thanks!

@intgr
Copy link
Owner

intgr commented Apr 12, 2022

Hrm. I'm not sure this changes anything for the environment tag. If we don't pass explicit ClientOptions, Sentry will use defaults anyway -- you should be able to use environment variables to control these values in either case?

PS: This project uses rustfmt to format source code. That's why the CI checks failed.

@intgr
Copy link
Owner

intgr commented Apr 12, 2022

Although setting the release tag is certainly useful.

And if you can figure out how to get the active Rocket environment from Rocket, that would be even better. It seems complicated during the Build phase, but should be possible.

@FxllenCode
Copy link
Author

FxllenCode commented Apr 12, 2022

@intgr I think I understand what you want - you want release to be auto-generated from Rocket? I believe that is already done by the Sentry library automatically. I'll look into it though!

Edit:

I see, I will do fix the environment! Based on the docs, you can get it from the configuration with the active() implementation. I'll also format with rustfmt, thanks!

@intgr
Copy link
Owner

intgr commented Apr 14, 2022

I think I understand what you want - you want release to be auto-generated from Rocket?

Sorry for the confusion. I was referring to the line that you already added: release: sentry::release_name!()

Looks like Sentry adds this tag regardless, but using Sentry's own recommended code snippet is better.

I see, I will do fix the environment!

I spent a few minutes and couldn't figure it out, how to get the current active environment name from Rocket.

@FxllenCode
Copy link
Author

I spent a few minutes and couldn't figure it out, how to get the current active environment name from Rocket.

Yeah, it was straight forward in the v3 of Rocket, but I'm not sure how to do it in the v5-rc version. I'll keep looking, surely it's possible?

@FxllenCode
Copy link
Author

FxllenCode commented Apr 14, 2022

Got it working!

When built with cargo run, no release tag:

image

When built with release tag:

image

Using rocket::config:

https://api.rocket.rs/v0.5-rc/rocket/config/struct.Config.html

However, for customization sake, I may add a feature to allow full customization using the way I suggest above, but defaulting to these settings if it is not inputted.

@FxllenCode
Copy link
Author

cc: @intgr

Copy link
Owner

@intgr intgr left a comment

Choose a reason for hiding this comment

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

Oops, sorry for not getting back to you earlier.

dsn,
sentry::ClientOptions {
release: sentry::release_name!(),
environment: Some(String::from(Config::DEFAULT_PROFILE).into()),
Copy link
Owner

@intgr intgr Apr 18, 2022

Choose a reason for hiding this comment

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

AFAICT this only tells you the default profile at build-time -- whether the application was built with assertions disabled or enabled -- not the profile actually used at the time of Rocket launching. That's what Sentry already does by default:

Defaults to either "development" or "production" depending on the debug_assertions cfg-attribute.

My thinking was that it would be more useful to get the active environment at run-time. There may be any number of different profiles configured (https://rocket.rs/v0.5-rc/guide/configuration/#profiles) but getting that value seems more complicated.

Copy link
Author

Choose a reason for hiding this comment

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

Okay, I think I understand what you mean - I'll look into it!

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.

None yet

2 participants