-
-
Notifications
You must be signed in to change notification settings - Fork 520
feat(monitor_check_ins): improve cron monitor slug readability with better formatting and hashing #2750
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
Open
amkisko
wants to merge
6
commits into
getsentry:master
Choose a base branch
from
amkisko:patch/cron-monitor-slug-formatting
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
feat(monitor_check_ins): improve cron monitor slug readability with better formatting and hashing #2750
amkisko
wants to merge
6
commits into
getsentry:master
from
amkisko:patch/cron-monitor-slug-formatting
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added a new SLUG_HASH_LENGTH constant and updated the slug generation logic to truncate long slugs from the beginning and append a hash for uniqueness. Updated tests to reflect the new slug format.
…tion Introduced a new MAX_NAME_LENGTH constant set to 128 to enforce length validation for names in the MonitorCheckIns module.
Updated the slug length calculation to correctly account for the additional character when generating slugs, ensuring proper truncation and uniqueness in the MonitorCheckIns module.
| @sentry_monitor_slug ||= begin | ||
| slug = name.gsub("::", "-").downcase | ||
| slug[-MAX_SLUG_LENGTH..-1] || slug | ||
| slug = name.gsub("::", "-").gsub(/([A-Z]+)([A-Z][a-z])/, '\1_\2').gsub(/([a-z\d])([A-Z])/, '\1_\2').downcase |
Check failure
Code scanning / CodeQL
Polynomial regular expression used on uncontrolled data High
This that depends on a may run slow on strings with many repetitions of 'A'.
regular expression
Error loading related location
Loading library input
Error loading related location
Loading
Author
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated group ([A-Z]+) from unlimited match to max 1000 characters.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR improves the readability of cron monitor slugs in the Sentry web UI by implementing better name formatting and intelligent truncation with hashing for long class names.
Attention: this PR introduces breaking change as slugs will be a bit different than before. According to rspec cases it affects only sentry-sidekiq.
Problem
Currently, cron monitor slugs generated from Ruby class names are hard to read in the Sentry web UI:
HappyWorkerWithCronbecomehappyworkerwithcron(no word separation)VeryLongOuterModule::VeryVeryVeryVeryLongInnerModule::Jobare blindly truncated without leaving any trace to parent classSolution
parameterizeandunderscoremethodsChanges
sentry_monitor_slugmethod with better CamelCase to snake_case conversionMAX_NAME_LENGTHandSLUG_HASH_LENGTHfor better maintainabilityExamples
HappyWorkerWithCron→happy_worker_with_cron(was:happyworkerwithcron)VeryLongOuterModule::VeryVeryVeryVeryLongInnerModule::Job→675905e0c9_very_very_very_long_inner_module-job(was:ongoutermodule-veryveryveryverylonginnermodule-job)fixes: RUBY-15
fixes: #2594