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

add some optional fields to error notice context #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

blackghost1987
Copy link
Collaborator

These optional context field are required by my project, so I added them to the config

@@ -6,6 +6,10 @@ pub struct Config {
pub workers: i32,
pub proxy: String,
pub app_version: String,
pub environment: Option<String>,
pub component: Option<String>,
Copy link
Owner

Choose a reason for hiding this comment

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

This should be configurable on notice level (same with os, hostname). Ideally, os & hostname are figured automatically.

@blackghost1987
Copy link
Collaborator Author

Yeah I made "hostname" to be automatically set in my app with the "nix" crate, but it required some conditional compilation as it's not working on windows. I can add it though in another merge request, if you think it's useful. Maybe we can even find a way to do it on windows properly as well.
I'm not sure about the "os", but maybe we can get it from the cfg! macro with "target_os". I'm not sure it can return a string though, it looks like it's returning a boolean only, so maybe requires some "if ... else if ... else if" magic.

What do you mean about making the "environment" and the "component" configurable on the notice level? Is there any way to pass arguments for one specific notice now? If not how should we implement it? Maybe until such a feature is not ready, we can add these as "defaults" for the future arguments, just to get it working for now.

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