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

[COST-4715] sentry -> glitchtip cleanup p1 #5010

Merged
merged 1 commit into from
Apr 1, 2024
Merged

Conversation

maskarb
Copy link
Member

@maskarb maskarb commented Apr 1, 2024

Jira Ticket

COST-4715

Description

  • replace API_SENTRY_DSN_KEY, CELERY_SENTRY_DSN_KEY, LISTENER_SENTRY_DSN_KEY, SOURCE_SENTRY_DSN_KEY with GLITCHTIP_KEY_NAME
  • Add new variables (KOKU_ENABLE_SENTRY, KOKU_SENTRY_DSN, and KOKU_SENTRY_ENVIRONMENT) which will consolidate several different SENTRY env variables.

This PR is written so that the old variables are still used. Once we switch stage/prod over to the new variables, we can remove them entirely.

Release Notes

  • proposed release note
* [[COST-4715](https://issues.redhat.com/browse/COST-4715)] Create new environment variables to consolidate the Glitchtip configuration

@maskarb maskarb added the hot-fix-smoke-tests pr_check label to run minimal smoke tests for fast moving bug-fix label Apr 1, 2024
@maskarb
Copy link
Member Author

maskarb commented Apr 1, 2024

/retest

Copy link

codecov bot commented Apr 1, 2024

Codecov Report

Merging #5010 (4a12de0) into main (738ea71) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #5010   +/-   ##
=====================================
  Coverage   94.1%   94.1%           
=====================================
  Files        377     377           
  Lines      31233   31233           
  Branches    3708    3708           
=====================================
  Hits       29380   29380           
  Misses      1181    1181           
  Partials     672     672           

Copy link
Contributor

@djnakabaale djnakabaale left a comment

Choose a reason for hiding this comment

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

This looks good to me. Here are some nitpick question:

traces_sampler=traces_sampler,
)
LOG.info("Sentry setup.")
# TODO: remove the following in favor of the above `if`
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you plan to remove this TODO in a followup pr?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, these cannot be removed until the other envs are switched to use the new variables.

@@ -34,6 +42,7 @@ def traces_sampler(sampling_context):
traces_sampler=traces_sampler,
)
LOG.info("Sentry setup.")
# TODO: remove the following in favor of the above `if`
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you plan to remove this TODO in a followup pr?

Copy link
Contributor

@djnakabaale djnakabaale left a comment

Choose a reason for hiding this comment

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

ltgm! 👍🏾

@maskarb maskarb merged commit fb19279 into main Apr 1, 2024
11 checks passed
@maskarb maskarb deleted the COST-4715-cleanup branch April 1, 2024 19:56
samdoran pushed a commit that referenced this pull request Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hot-fix-smoke-tests pr_check label to run minimal smoke tests for fast moving bug-fix smokes-required
Projects
None yet
2 participants