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 airbrake project key environment variable #147

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

agbpatro
Copy link
Collaborator

@agbpatro agbpatro commented Apr 5, 2024

Description

Allow setting AIRBRAKE_API_KEY environment variable so that key can be avoided being stored in plaintext. If the env variable is set, it will override whatever is stored in Airbrake.ProjectId field of config file.

Type of change

Please select all options that apply to this change:

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Bug fix (non-breaking change which fixes an issue)
  • Documentation update

Checklist:

Confirm you have completed the following steps:

  • My code follows the style of this project.
  • I have performed a self-review of my code.
  • I have made corresponding updates to the documentation.
  • I have added/updated unit tests to cover my changes.
  • I have added/updated integration tests to cover my changes.

Copy link
Collaborator

@vmallet vmallet left a comment

Choose a reason for hiding this comment

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

Also, could you add the PR comment straight into the commit message? That way it will stay with the repo forever.

(Suggestions to edit in the body:

Allow setting the airbrake api key via the AIRBRAKE_API_KEY environment 
variable. If this variable is set, it will override the `project_key` 
field in the airbrake section of config file.

README.md Outdated
@@ -172,6 +172,8 @@ Data is encapsulated into protobuf messages of different types. Protos can be re
```sh
make generate-protos
```
## Airbrake
Fleet telemetry allows you to monitor error using [airbrake](https://www.airbrake.io/error-monitoring). The integration test runs fleet telemetry with [errbit](https://github.com/errbit/errbit), which is airbrake compliant self-hosted error catcher. You can set project key for airbrake using either the config file or via environment variable `AIRBRAKE_PROJECT_KEY`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • ... to monitor errors using ...
  • ... which is an airbrake ...
  • ... You can set a project key
  • ... or via an environment

return githubairbrake.NewNotifierWithOptions(&githubairbrake.NotifierOptions{
projectKey, ok := os.LookupEnv("AIRBRAKE_PROJECT_KEY")
if !ok {
projectKey = c.Airbrake.ProjectKey
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add some logging here, it's always hard to debug these things without visibility.
Something like:

  • using airbrake project key from AIRBRAKE_PROJECT_KEY env variable (<- use const)
  • using airbrake project key from config file

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated with some modifications

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds good. If we had the actual env variable name in the log that'd be helpful for debugging ("ahhhh I have a typo, I was setting AIRBRAKE_API_KEY, silly me!) but at least now we know which path it's taking.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated!

@agbpatro agbpatro force-pushed the allow_support_for_airbrake_api_key_in_env branch from dcc6d82 to bed0088 Compare April 5, 2024 21:32
@agbpatro agbpatro requested a review from vmallet April 5, 2024 21:33
return githubairbrake.NewNotifierWithOptions(&githubairbrake.NotifierOptions{
projectKey, ok := os.LookupEnv("AIRBRAKE_PROJECT_KEY")
if !ok {
projectKey = c.Airbrake.ProjectKey
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds good. If we had the actual env variable name in the log that'd be helpful for debugging ("ahhhh I have a typo, I was setting AIRBRAKE_API_KEY, silly me!) but at least now we know which path it's taking.

@agbpatro agbpatro force-pushed the allow_support_for_airbrake_api_key_in_env branch from bed0088 to 4a1806a Compare April 5, 2024 21:49
Allow setting the airbrake api key via the AIRBRAKE_API_KEY environment
variable. If this variable is set, it will override the
field in the airbrake section of config file.
@agbpatro agbpatro force-pushed the allow_support_for_airbrake_api_key_in_env branch from 4a1806a to 5fa9b98 Compare April 5, 2024 21:50
@agbpatro agbpatro merged commit 5b12655 into main Apr 5, 2024
5 checks passed
@agbpatro agbpatro deleted the allow_support_for_airbrake_api_key_in_env branch April 5, 2024 21:57
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

3 participants