From b8163cf1598fef21356b586442a586084b10bbaa Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Thu, 16 Oct 2025 13:29:27 +0300 Subject: [PATCH 01/24] Introduce sentry-good_job integration with essential files and configurations - Added .gitignore to exclude unnecessary files and directories. - Created .rspec and .rubocop.yml for testing and code style configurations. - Introduced CHANGELOG.md for tracking changes and features. - Set up Gemfile and gemspec for dependency management. - Implemented core integration logic in lib/sentry-good_job.rb and related modules. - Added example application and setup scripts for demonstration. - Included comprehensive test suite for validation of functionality. This commit establishes the foundation for the sentry-good_job gem, enabling error tracking and monitoring for ActiveJob workers using Good Job. --- sentry-good_job/.gitignore | 78 +++++ sentry-good_job/.rspec | 3 + sentry-good_job/.rspec_status | 87 ++++++ sentry-good_job/.rubocop.yml | 19 ++ sentry-good_job/CHANGELOG.md | 33 +++ sentry-good_job/Gemfile | 16 + sentry-good_job/LICENSE.txt | 21 ++ sentry-good_job/Makefile | 22 ++ sentry-good_job/README.md | 182 ++++++++++++ sentry-good_job/Rakefile | 8 + sentry-good_job/bin/console | 14 + sentry-good_job/bin/setup | 19 ++ sentry-good_job/example/Gemfile | 8 + sentry-good_job/example/app.rb | 50 ++++ sentry-good_job/lib/sentry-good_job.rb | 56 ++++ sentry-good_job/lib/sentry/good_job.rb | 55 ++++ .../lib/sentry/good_job/configuration.rb | 53 ++++ .../lib/sentry/good_job/cron_monitoring.rb | 149 ++++++++++ .../lib/sentry/good_job/error_handler.rb | 57 ++++ .../lib/sentry/good_job/job_monitor.rb | 166 +++++++++++ sentry-good_job/lib/sentry/good_job/logger.rb | 40 +++ .../lib/sentry/good_job/version.rb | 7 + sentry-good_job/sentry-good_job.gemspec | 35 +++ .../sentry/good_job/configuration_spec.rb | 70 +++++ .../sentry/good_job/cron_monitoring_spec.rb | 278 ++++++++++++++++++ .../sentry/good_job/error_handler_spec.rb | 199 +++++++++++++ .../spec/sentry/good_job/job_monitor_spec.rb | 120 ++++++++ .../spec/sentry/good_job/logger_spec.rb | 161 ++++++++++ sentry-good_job/spec/sentry/good_job_spec.rb | 99 +++++++ sentry-good_job/spec/spec_helper.rb | 185 ++++++++++++ 30 files changed, 2290 insertions(+) create mode 100644 sentry-good_job/.gitignore create mode 100644 sentry-good_job/.rspec create mode 100644 sentry-good_job/.rspec_status create mode 100644 sentry-good_job/.rubocop.yml create mode 100644 sentry-good_job/CHANGELOG.md create mode 100644 sentry-good_job/Gemfile create mode 100644 sentry-good_job/LICENSE.txt create mode 100644 sentry-good_job/Makefile create mode 100644 sentry-good_job/README.md create mode 100644 sentry-good_job/Rakefile create mode 100755 sentry-good_job/bin/console create mode 100755 sentry-good_job/bin/setup create mode 100644 sentry-good_job/example/Gemfile create mode 100644 sentry-good_job/example/app.rb create mode 100644 sentry-good_job/lib/sentry-good_job.rb create mode 100644 sentry-good_job/lib/sentry/good_job.rb create mode 100644 sentry-good_job/lib/sentry/good_job/configuration.rb create mode 100644 sentry-good_job/lib/sentry/good_job/cron_monitoring.rb create mode 100644 sentry-good_job/lib/sentry/good_job/error_handler.rb create mode 100644 sentry-good_job/lib/sentry/good_job/job_monitor.rb create mode 100644 sentry-good_job/lib/sentry/good_job/logger.rb create mode 100644 sentry-good_job/lib/sentry/good_job/version.rb create mode 100644 sentry-good_job/sentry-good_job.gemspec create mode 100644 sentry-good_job/spec/sentry/good_job/configuration_spec.rb create mode 100644 sentry-good_job/spec/sentry/good_job/cron_monitoring_spec.rb create mode 100644 sentry-good_job/spec/sentry/good_job/error_handler_spec.rb create mode 100644 sentry-good_job/spec/sentry/good_job/job_monitor_spec.rb create mode 100644 sentry-good_job/spec/sentry/good_job/logger_spec.rb create mode 100644 sentry-good_job/spec/sentry/good_job_spec.rb create mode 100644 sentry-good_job/spec/spec_helper.rb diff --git a/sentry-good_job/.gitignore b/sentry-good_job/.gitignore new file mode 100644 index 000000000..2071284f1 --- /dev/null +++ b/sentry-good_job/.gitignore @@ -0,0 +1,78 @@ +/.bundle/ +/.yardoc +/_yardoc/ +/coverage/ +/doc/ +/pkg/ +/spec/reports/ +/tmp/ +*.gem +*.rbc +/.config +/coverage/ +/InstalledFiles +/pkg/ +/spec/reports/ +/spec/examples.txt +/test/tmp/ +/test/version_tmp/ +/tmp/ + +# Environment variables +.env +.env.local +.env.*.local + +# Logs +*.log + +# Runtime data +pids +*.pid +*.seed +*.pid.lock + +# Coverage directory used by tools like istanbul +coverage/ + +# nyc test coverage +.nyc_output + +# Dependency directories +node_modules/ + +# Optional npm cache directory +.npm + +# Optional REPL history +.node_repl_history + +# Output of 'npm pack' +*.tgz + +# Yarn Integrity file +.yarn-integrity + +# dotenv environment variables file +.env + +# next.js build output +.next + +# Nuxt.js build output +.nuxt + +# vuepress build output +.vuepress/dist + +# Serverless directories +.serverless + +# FuseBox cache +.fusebox/ + +# DynamoDB Local files +.dynamodb/ + +# TernJS port file +.tern-port diff --git a/sentry-good_job/.rspec b/sentry-good_job/.rspec new file mode 100644 index 000000000..4e33a322b --- /dev/null +++ b/sentry-good_job/.rspec @@ -0,0 +1,3 @@ +--require spec_helper +--color +--format documentation diff --git a/sentry-good_job/.rspec_status b/sentry-good_job/.rspec_status new file mode 100644 index 000000000..154009e0a --- /dev/null +++ b/sentry-good_job/.rspec_status @@ -0,0 +1,87 @@ +example_id | status | run_time | +----------------------------------------------------------- | ------ | --------------- | +./spec/sentry/good_job/configuration_spec.rb[1:1:1] | passed | 0.00139 seconds | +./spec/sentry/good_job/configuration_spec.rb[1:2:1] | passed | 0.00334 seconds | +./spec/sentry/good_job/configuration_spec.rb[1:3:1] | passed | 0.0003 seconds | +./spec/sentry/good_job/configuration_spec.rb[1:3:2] | passed | 0.00013 seconds | +./spec/sentry/good_job/configuration_spec.rb[1:3:3] | passed | 0.00012 seconds | +./spec/sentry/good_job/configuration_spec.rb[1:3:4] | passed | 0.00012 seconds | +./spec/sentry/good_job/configuration_spec.rb[1:3:5] | passed | 0.00012 seconds | +./spec/sentry/good_job/configuration_spec.rb[1:3:6] | passed | 0.00012 seconds | +./spec/sentry/good_job/configuration_spec.rb[1:3:7] | passed | 0.00028 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:1:1] | passed | 0.00013 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:1:2] | passed | 0.00012 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:1:3] | passed | 0.00075 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:1:4] | passed | 0.00036 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:1:5] | passed | 0.00576 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:2:1] | passed | 0.0002 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:2:2] | passed | 0.00014 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:2:3] | passed | 0.00013 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:3:1] | passed | 0.00014 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:3:2] | passed | 0.00015 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:3:3] | passed | 0.00013 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:3:4] | passed | 0.00012 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:3:5] | passed | 0.00012 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:1:1:1] | passed | 0.00036 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:1:2:1] | passed | 0.00034 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:1:3:1] | passed | 0.00039 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:1:4:1] | passed | 0.00047 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:1:4:2] | passed | 0.00043 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:2:1:1] | passed | 0.00031 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:2:2:1] | passed | 0.0003 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:2:3:1] | passed | 0.00028 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:2:4:1] | passed | 0.00066 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:2:4:2] | passed | 0.00083 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:2:4:3] | passed | 0.00393 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:3:1] | passed | 0.00047 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:3:2] | passed | 0.0004 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:3:3] | passed | 0.00045 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:3:4] | passed | 0.00084 seconds | +./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:3:5] | passed | 0.00155 seconds | +./spec/sentry/good_job/error_handler_spec.rb[1:1:1:1] | passed | 0.00148 seconds | +./spec/sentry/good_job/error_handler_spec.rb[1:1:2:1] | passed | 0.00218 seconds | +./spec/sentry/good_job/error_handler_spec.rb[1:1:2:2:1] | passed | 0.00302 seconds | +./spec/sentry/good_job/error_handler_spec.rb[1:1:2:3:1:1:1] | passed | 0.0015 seconds | +./spec/sentry/good_job/error_handler_spec.rb[1:1:2:3:1:2:1] | passed | 0.00143 seconds | +./spec/sentry/good_job/error_handler_spec.rb[1:1:2:3:2:1] | passed | 0.0006 seconds | +./spec/sentry/good_job/error_handler_spec.rb[1:1:2:4:1:1] | passed | 0.00035 seconds | +./spec/sentry/good_job/error_handler_spec.rb[1:1:2:4:2:1] | passed | 0.00036 seconds | +./spec/sentry/good_job/error_handler_spec.rb[1:2:1] | passed | 0.0006 seconds | +./spec/sentry/good_job/error_handler_spec.rb[1:2:2] | passed | 0.00019 seconds | +./spec/sentry/good_job/error_handler_spec.rb[1:2:3] | passed | 0.00016 seconds | +./spec/sentry/good_job/error_handler_spec.rb[1:3:1] | passed | 0.00027 seconds | +./spec/sentry/good_job/error_handler_spec.rb[1:3:2:1] | passed | 0.00019 seconds | +./spec/sentry/good_job/error_handler_spec.rb[1:3:3:1] | passed | 0.00065 seconds | +./spec/sentry/good_job/job_monitor_spec.rb[1:1:1:1] | passed | 0.00023 seconds | +./spec/sentry/good_job/job_monitor_spec.rb[1:1:2:1] | passed | 0.00025 seconds | +./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:1] | passed | 0.00035 seconds | +./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:2] | passed | 0.00026 seconds | +./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:3] | passed | 0.00025 seconds | +./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:4] | passed | 0.00034 seconds | +./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:5] | passed | 0.0009 seconds | +./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:6] | passed | 0.00026 seconds | +./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:7] | passed | 0.00022 seconds | +./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:8] | passed | 0.00022 seconds | +./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:9] | passed | 0.00025 seconds | +./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:10] | passed | 0.00029 seconds | +./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:11:1] | passed | 0.00031 seconds | +./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:12:1] | passed | 0.00027 seconds | +./spec/sentry/good_job/logger_spec.rb[1:1:1:1] | passed | 0.00013 seconds | +./spec/sentry/good_job/logger_spec.rb[1:1:2:1] | passed | 0.00018 seconds | +./spec/sentry/good_job/logger_spec.rb[1:1:3:1] | passed | 0.00017 seconds | +./spec/sentry/good_job/logger_spec.rb[1:2:1:1] | passed | 0.00013 seconds | +./spec/sentry/good_job/logger_spec.rb[1:2:2:1:1] | passed | 0.00022 seconds | +./spec/sentry/good_job/logger_spec.rb[1:2:2:2:1] | passed | 0.00014 seconds | +./spec/sentry/good_job/logger_spec.rb[1:3:1:1] | passed | 0.00021 seconds | +./spec/sentry/good_job/logger_spec.rb[1:3:2:1] | passed | 0.00016 seconds | +./spec/sentry/good_job/logger_spec.rb[1:4:1:1] | passed | 0.00021 seconds | +./spec/sentry/good_job/logger_spec.rb[1:4:2:1] | passed | 0.00015 seconds | +./spec/sentry/good_job/logger_spec.rb[1:5:1:1] | passed | 0.00022 seconds | +./spec/sentry/good_job/logger_spec.rb[1:5:2:1] | passed | 0.00016 seconds | +./spec/sentry/good_job_spec.rb[1:1] | passed | 0.00012 seconds | +./spec/sentry/good_job_spec.rb[1:2] | passed | 0.00011 seconds | +./spec/sentry/good_job_spec.rb[1:3:1] | passed | 0.00032 seconds | +./spec/sentry/good_job_spec.rb[1:3:2:1] | passed | 0.00031 seconds | +./spec/sentry/good_job_spec.rb[1:3:3:1] | passed | 0.0003 seconds | +./spec/sentry/good_job_spec.rb[1:4:1] | passed | 0.00017 seconds | +./spec/sentry/good_job_spec.rb[1:5:1] | passed | 0.00027 seconds | diff --git a/sentry-good_job/.rubocop.yml b/sentry-good_job/.rubocop.yml new file mode 100644 index 000000000..fc3c84d79 --- /dev/null +++ b/sentry-good_job/.rubocop.yml @@ -0,0 +1,19 @@ +inherit_gem: + rubocop-rails-omakase: rubocop.yml + +Layout/SpaceInsideArrayLiteralBrackets: + Enabled: false + +Layout/EmptyLineAfterMagicComment: + Enabled: true + +Style/FrozenStringLiteralComment: + Enabled: true + +Style/RedundantFreeze: + Enabled: true + +AllCops: + Exclude: + - "tmp/**/*" + - "examples/**/*" diff --git a/sentry-good_job/CHANGELOG.md b/sentry-good_job/CHANGELOG.md new file mode 100644 index 000000000..58ae53efa --- /dev/null +++ b/sentry-good_job/CHANGELOG.md @@ -0,0 +1,33 @@ +# Changelog + +Individual gem's changelog has been deprecated. Please check the [project changelog](https://github.com/getsentry/sentry-ruby/blob/master/CHANGELOG.md). + +## 5.28.0 + +### Features + +- Initial release of sentry-good_job integration +- Automatic error capture for ActiveJob workers using Good Job +- Performance monitoring for job execution +- Automatic cron monitoring setup for scheduled jobs +- Context preservation and trace propagation +- Configurable error reporting options +- Rails integration with automatic setup + +### Configuration Options + +- `report_after_job_retries`: Only report errors after all retry attempts are exhausted +- `report_only_dead_jobs`: Only report errors for jobs that cannot be retried +- `propagate_traces`: Propagate trace headers for distributed tracing +- `include_job_arguments`: Include job arguments in error context +- `auto_setup_cron_monitoring`: Automatically set up cron monitoring for scheduled jobs +- `logging_enabled`: Enable logging for the Good Job integration +- `logger`: Custom logger to use + +### Integration Features + +- Seamless integration with Rails applications +- Automatic setup when Good Job integration is enabled +- Support for both manual and automatic cron monitoring +- Respects ActiveJob retry configuration +- Comprehensive error context and performance metrics diff --git a/sentry-good_job/Gemfile b/sentry-good_job/Gemfile new file mode 100644 index 000000000..02091cca3 --- /dev/null +++ b/sentry-good_job/Gemfile @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +source "https://rubygems.org" + +# Specify your gem's dependencies in sentry-good_job.gemspec +gemspec + +gem "rake", "~> 13.0" + +group :development, :test do + gem "rspec", "~> 3.0" + gem "rubocop", "~> 1.0" + gem "rubocop-rails-omakase", "~> 1.0" + gem "simplecov", "~> 0.22" + gem "simplecov-cobertura", "~> 2.0" +end diff --git a/sentry-good_job/LICENSE.txt b/sentry-good_job/LICENSE.txt new file mode 100644 index 000000000..908c02ece --- /dev/null +++ b/sentry-good_job/LICENSE.txt @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2024 Sentry + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/sentry-good_job/Makefile b/sentry-good_job/Makefile new file mode 100644 index 000000000..8f0435497 --- /dev/null +++ b/sentry-good_job/Makefile @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +.PHONY: test +test: + bundle exec rspec + +.PHONY: lint +lint: + bundle exec rubocop + +.PHONY: install +install: + bundle install + +.PHONY: console +console: + bundle exec bin/console + +.PHONY: setup +setup: + bundle install + bundle exec bin/setup diff --git a/sentry-good_job/README.md b/sentry-good_job/README.md new file mode 100644 index 000000000..26e411526 --- /dev/null +++ b/sentry-good_job/README.md @@ -0,0 +1,182 @@ +

+ + + +
+

+ +# sentry-good_job, the Good Job integration for Sentry's Ruby client + +--- + +[![Gem Version](https://img.shields.io/gem/v/sentry-good_job.svg)](https://rubygems.org/gems/sentry-good_job) +![Build Status](https://github.com/getsentry/sentry-ruby/actions/workflows/sentry_good_job_test.yml/badge.svg) +[![Coverage Status](https://img.shields.io/codecov/c/github/getsentry/sentry-ruby/master?logo=codecov)](https://codecov.io/gh/getsentry/sentry-ruby/branch/master) +[![Gem](https://img.shields.io/gem/dt/sentry-good_job.svg)](https://rubygems.org/gems/sentry-good_job/) +[![SemVer](https://api.dependabot.com/badges/compatibility_score?dependency-name=sentry-good_job&package-manager=bundler&version-scheme=semver)](https://dependabot.com/compatibility-score.html?dependency-name=sentry-good_job&package-manager=bundler&version-scheme=semver) + +[Documentation](https://docs.sentry.io/platforms/ruby/guides/good_job/) | [Bug Tracker](https://github.com/getsentry/sentry-ruby/issues) | [Forum](https://forum.sentry.io/) | IRC: irc.freenode.net, #sentry + +The official Ruby-language client and integration layer for the [Sentry](https://github.com/getsentry/sentry) error reporting API. + +## Getting Started + +### Install + +```ruby +gem "sentry-ruby" +gem "sentry-good_job" +``` + +Then you're all set! `sentry-good_job` will automatically capture exceptions from your ActiveJob workers when using Good Job as the backend! + +## Features + +- **Automatic Error Capture**: Captures exceptions from ActiveJob workers using Good Job +- **Performance Monitoring**: Tracks job execution times and performance metrics +- **Cron Monitoring**: Automatic setup for scheduled jobs with cron monitoring +- **Context Preservation**: Maintains user context and trace propagation across job executions +- **Configurable Reporting**: Control when errors are reported (after retries, only dead jobs, etc.) +- **Rails Integration**: Seamless integration with Rails applications + +## Configuration + +The integration can be configured through Sentry's configuration: + +```ruby +Sentry.init do |config| + config.dsn = 'your-dsn-here' + + # Good Job specific configuration + config.good_job.report_after_job_retries = false + config.good_job.report_only_dead_jobs = false + config.good_job.propagate_traces = true + config.good_job.include_job_arguments = false + config.good_job.auto_setup_cron_monitoring = true + config.good_job.logging_enabled = false +end +``` + +### Configuration Options + +- `report_after_job_retries` (default: `false`): Only report errors after all retry attempts are exhausted +- `report_only_dead_jobs` (default: `false`): Only report errors for jobs that cannot be retried +- `propagate_traces` (default: `true`): Propagate trace headers for distributed tracing +- `include_job_arguments` (default: `false`): Include job arguments in error context (be careful with sensitive data) +- `auto_setup_cron_monitoring` (default: `true`): Automatically set up cron monitoring for scheduled jobs +- `logging_enabled` (default: `false`): Enable logging for the Good Job integration +- `logger` (default: `nil`): Custom logger to use (defaults to Rails.logger when available) + +## Usage + +### Basic Setup + +The integration works automatically once installed. It will: + +1. Capture exceptions from ActiveJob workers +2. Set up performance monitoring for job execution +3. Automatically configure cron monitoring for scheduled jobs +4. Preserve user context and trace propagation + +### Manual Job Monitoring + +You can manually set up monitoring for specific job classes: + +```ruby +class MyJob < ApplicationJob + # The integration will automatically set up monitoring +end +``` + +### Cron Monitoring + +For scheduled jobs, cron monitoring is automatically set up based on your Good Job configuration: + +```ruby +# config/application.rb +config.good_job.cron = { + 'my_scheduled_job' => { + class: 'MyScheduledJob', + cron: '0 * * * *' # Every hour + } +} +``` + +You can also manually set up cron monitoring: + +```ruby +class MyScheduledJob < ApplicationJob + sentry_cron_monitor "0 * * * *", timezone: "UTC" +end +``` + +### Custom Error Handling + +The integration respects ActiveJob's retry configuration and will only report errors based on your settings: + +```ruby +class MyJob < ApplicationJob + retry_on StandardError, wait: :exponentially_longer, attempts: 3 + + def perform + # This will only be reported to Sentry after 3 attempts if report_after_job_retries is true + raise "Something went wrong" + end +end +``` + +### Debugging and Detailed Logging + +For debugging purposes, you can enable detailed logging to see what the integration is doing: + +```ruby +Sentry.init do |config| + config.dsn = 'your-dsn-here' + + # Enable detailed logging for debugging + config.good_job.logging_enabled = true + config.good_job.logger = Logger.new($stdout) +end +``` + +This will output detailed information about: +- Job execution start and completion +- Error capture and reporting decisions +- Cron monitoring setup +- Performance metrics collection +- Trace propagation + +## Performance Monitoring + +When performance monitoring is enabled, the integration will track: + +- Job execution time +- Queue latency +- Retry counts +- Job context and metadata + +## Error Context + +The integration automatically adds relevant context to error reports: + +- Job class name +- Job ID +- Queue name +- Execution count +- Enqueued and scheduled timestamps +- Job arguments (if enabled) + +## Compatibility + +- Ruby 2.4+ +- Rails 5.2+ +- Good Job 3.0+ +- Sentry Ruby SDK 5.28.0+ + +## Contributing + +We welcome contributions! Please see our [contributing guidelines](https://github.com/getsentry/sentry-ruby/blob/master/CONTRIBUTING.md) for details. + +## License + +This project is licensed under the MIT License. See the [LICENSE](LICENSE.txt) file for details. diff --git a/sentry-good_job/Rakefile b/sentry-good_job/Rakefile new file mode 100644 index 000000000..b6ae73410 --- /dev/null +++ b/sentry-good_job/Rakefile @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +require "bundler/gem_tasks" +require "rspec/core/rake_task" + +RSpec::Core::RakeTask.new(:spec) + +task default: :spec diff --git a/sentry-good_job/bin/console b/sentry-good_job/bin/console new file mode 100755 index 000000000..05657330f --- /dev/null +++ b/sentry-good_job/bin/console @@ -0,0 +1,14 @@ +#!/usr/bin/env ruby + +require "bundler/setup" +require "sentry-good_job" + +# You can add fixtures and/or initialization code here to make experimenting +# with your gem easier. You can also use a different console, if you like. + +# (If you use this, don't forget to add pry to your Gemfile!) +# require "pry" +# Pry.start + +require "irb" +IRB.start(__FILE__) diff --git a/sentry-good_job/bin/setup b/sentry-good_job/bin/setup new file mode 100755 index 000000000..bffaa8904 --- /dev/null +++ b/sentry-good_job/bin/setup @@ -0,0 +1,19 @@ +#!/usr/bin/env ruby +require "fileutils" + +# path to your application root. +APP_ROOT = File.expand_path("..", __dir__) + +def system!(*args) + system(*args) || abort("\n== Command #{args} failed ==") +end + +FileUtils.chdir APP_ROOT do + # This script is a way to set up or update your development environment automatically. + # This script is idempotent, so that you can run it at any time and get an expectable outcome. + # Add necessary setup steps to this file. + + puts "== Installing dependencies ==" + system! "gem install bundler --conservative" + system("bundle check") || system!("bundle install") +end diff --git a/sentry-good_job/example/Gemfile b/sentry-good_job/example/Gemfile new file mode 100644 index 000000000..c28cdcbcb --- /dev/null +++ b/sentry-good_job/example/Gemfile @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +source "https://rubygems.org" + +gem "sentry-ruby" +gem "sentry-good_job" +gem "good_job" +gem "activejob" diff --git a/sentry-good_job/example/app.rb b/sentry-good_job/example/app.rb new file mode 100644 index 000000000..4eab9e980 --- /dev/null +++ b/sentry-good_job/example/app.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require "bundler/setup" +require "sentry-good_job" +require "active_job" +require "good_job" + +# Configure Sentry +Sentry.init do |config| + config.dsn = ENV["SENTRY_DSN"] || "http://12345:67890@sentry.localdomain/sentry/42" + config.environment = "development" + config.logger = Logger.new(STDOUT) + + # Good Job specific configuration + config.good_job.report_after_job_retries = false + config.good_job.include_job_arguments = true + config.good_job.logging_enabled = true +end + +# Example job classes +class HappyJob < ActiveJob::Base + def perform(message) + puts "Happy job executed with message: #{message}" + Sentry.add_breadcrumb(message: "Happy job completed successfully") + end +end + +class SadJob < ActiveJob::Base + def perform(message) + puts "Sad job executed with message: #{message}" + raise "Something went wrong in the sad job!" + end +end + +class ScheduledJob < ActiveJob::Base + def perform + puts "Scheduled job executed at #{Time.now}" + end +end + +# Example usage +puts "Sentry Good Job Integration Example" +puts "==================================" + +# Enqueue some jobs +HappyJob.perform_later("Hello from happy job!") +SadJob.perform_later("Hello from sad job!") + +puts "\nJobs enqueued. Check your Sentry dashboard for error reports." +puts "The sad job will generate an error that should be captured by Sentry." diff --git a/sentry-good_job/lib/sentry-good_job.rb b/sentry-good_job/lib/sentry-good_job.rb new file mode 100644 index 000000000..48e44798e --- /dev/null +++ b/sentry-good_job/lib/sentry-good_job.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require "good_job" +require "sentry-ruby" +require "sentry/integrable" +require "sentry/good_job/version" +require "sentry/good_job/configuration" +require "sentry/good_job/error_handler" +require "sentry/good_job/logger" +require "sentry/good_job/job_monitor" +require "sentry/good_job/cron_monitoring" + +module Sentry + module GoodJob + extend Sentry::Integrable + + register_integration name: "good_job", version: Sentry::GoodJob::VERSION + + if defined?(::Rails::Railtie) + class Railtie < ::Rails::Railtie + config.after_initialize do + next unless Sentry.initialized? && defined?(::Sentry::Rails) + + # Skip ActiveJob error reporting when using Good Job as the backend + # since we handle it ourselves + Sentry.configuration.rails.skippable_job_adapters << "ActiveJob::QueueAdapters::GoodJobAdapter" + + # Automatic setup for Good Job when the integration is enabled + if Sentry.configuration.enabled_patches.include?(:good_job) + Sentry::GoodJob.setup_good_job_integration + end + end + end + end + + def self.setup_good_job_integration + # Sentry Rails integration already handles ActiveJob exceptions automatically + # No need for custom error handling + + # Set up unified job monitoring for ApplicationJob + Sentry::GoodJob::JobMonitor.setup_for_job_class(ApplicationJob) + + # Set up cron monitoring for all scheduled jobs (automatically configured from Good Job config) + if Sentry.configuration.good_job.auto_setup_cron_monitoring + Sentry::GoodJob::CronMonitoring::Integration.setup_monitoring_for_scheduled_jobs + end + + Sentry::GoodJob::Logger.info "Sentry Good Job integration initialized automatically" + end + + # Delegate capture_exception so internal components can be tested in isolation + def self.capture_exception(exception, **options) + ::Sentry.capture_exception(exception, **options) + end + end +end diff --git a/sentry-good_job/lib/sentry/good_job.rb b/sentry-good_job/lib/sentry/good_job.rb new file mode 100644 index 000000000..3269b7f36 --- /dev/null +++ b/sentry-good_job/lib/sentry/good_job.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require "sentry-ruby" +require "sentry/integrable" +require "sentry/good_job/version" +require "sentry/good_job/configuration" +require "sentry/good_job/error_handler" +require "sentry/good_job/logger" +require "sentry/good_job/job_monitor" +require "sentry/good_job/cron_monitoring" + +module Sentry + module GoodJob + extend Sentry::Integrable + + register_integration name: "good_job", version: Sentry::GoodJob::VERSION + + if defined?(::Rails::Railtie) + class Railtie < ::Rails::Railtie + config.after_initialize do + next unless Sentry.initialized? && defined?(::Sentry::Rails) + + # Skip ActiveJob error reporting when using Good Job as the backend + # since we handle it ourselves + Sentry.configuration.rails.skippable_job_adapters << "ActiveJob::QueueAdapters::GoodJobAdapter" + + # Automatic setup for Good Job when the integration is enabled + if Sentry.configuration.enabled_patches.include?(:good_job) + Sentry::GoodJob.setup_good_job_integration + end + end + end + end + + def self.setup_good_job_integration + # Sentry Rails integration already handles ActiveJob exceptions automatically + # No need for custom error handling + + # Set up unified job monitoring for ApplicationJob + Sentry::GoodJob::JobMonitor.setup_for_job_class(ApplicationJob) + + # Set up cron monitoring for all scheduled jobs (automatically configured from Good Job config) + if Sentry.configuration.good_job.auto_setup_cron_monitoring + Sentry::GoodJob::CronMonitoring::Integration.setup_monitoring_for_scheduled_jobs + end + + Sentry::GoodJob::Logger.info "Sentry Good Job integration initialized automatically" + end + + # Delegate capture_exception so internal components can be tested in isolation + def self.capture_exception(exception, **options) + ::Sentry.capture_exception(exception, **options) + end + end +end diff --git a/sentry-good_job/lib/sentry/good_job/configuration.rb b/sentry-good_job/lib/sentry/good_job/configuration.rb new file mode 100644 index 000000000..2a8287480 --- /dev/null +++ b/sentry-good_job/lib/sentry/good_job/configuration.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module Sentry + class Configuration + attr_reader :good_job + + add_post_initialization_callback do + @good_job = Sentry::GoodJob::Configuration.new + @excluded_exceptions = @excluded_exceptions.concat(Sentry::GoodJob::IGNORE_DEFAULT) + end + end + + module GoodJob + IGNORE_DEFAULT = [ + "ActiveJob::DeserializationError", + "ActiveJob::SerializationError" + ] + + class Configuration + # Set this option to true if you want Sentry to only capture the last job + # retry if it fails. + attr_accessor :report_after_job_retries + + # Only report jobs that have retry_on_attempts set (i.e., jobs that can be retried) + attr_accessor :report_only_dead_jobs + + # Whether we should inject headers while enqueuing the job in order to have a connected trace + attr_accessor :propagate_traces + + # Whether to include job arguments in error context (be careful with sensitive data) + attr_accessor :include_job_arguments + + # Whether to automatically set up cron monitoring for all scheduled jobs + attr_accessor :auto_setup_cron_monitoring + + # When false, suppresses all Sentry GoodJob integration logs + attr_accessor :logging_enabled + + # Custom logger to use for Sentry GoodJob integration (defaults to Rails.logger) + attr_accessor :logger + + def initialize + @report_after_job_retries = false + @report_only_dead_jobs = false + @propagate_traces = true + @include_job_arguments = false + @auto_setup_cron_monitoring = true + @logging_enabled = false + @logger = nil + end + end + end +end diff --git a/sentry-good_job/lib/sentry/good_job/cron_monitoring.rb b/sentry-good_job/lib/sentry/good_job/cron_monitoring.rb new file mode 100644 index 000000000..8cee06eb2 --- /dev/null +++ b/sentry-good_job/lib/sentry/good_job/cron_monitoring.rb @@ -0,0 +1,149 @@ +# frozen_string_literal: true + +# Sentry Cron Monitoring for Good Job +# This module provides comprehensive cron monitoring for Good Job scheduled tasks +# Following Good Job's extension patterns and Sentry's integration guidelines +module Sentry + module GoodJob + module CronMonitoring + # Utility methods for cron parsing and configuration + # These methods handle the conversion between Good Job cron expressions and Sentry monitor configs + module Helpers + # Parse cron expression and create Sentry monitor config + def self.monitor_config_from_cron(cron_expression, timezone: nil) + return nil unless cron_expression.present? + + # Parse cron expression using fugit (same as Good Job) + parsed_cron = Fugit.parse_cron(cron_expression) + return nil unless parsed_cron + + # Convert to Sentry monitor config + if timezone.present? + ::Sentry::Cron::MonitorConfig.from_crontab(cron_expression, timezone: timezone) + else + ::Sentry::Cron::MonitorConfig.from_crontab(cron_expression) + end + rescue => e + Sentry::GoodJob::Logger.warn "Failed to parse cron expression '#{cron_expression}': #{e.message}" + nil + end + + # Generate monitor slug from job name + def self.monitor_slug(job_name) + job_name.to_s.underscore.gsub(/_job$/, "") + end + + # Parse cron expression and extract timezone + def self.parse_cron_with_timezone(cron_expression) + return [cron_expression, nil] unless cron_expression.present? + + parts = cron_expression.strip.split(" ") + return [cron_expression, nil] unless parts.length > 5 + + # Last part might be timezone + timezone = parts.last + # Basic timezone validation (matches common timezone formats including Europe/Stockholm, America/New_York, etc.) + if timezone.match?(/^[A-Za-z_]+\/[A-Za-z_]+$/) || timezone.match?(/^[A-Za-z_]+$/) + cron_without_timezone = cron_expression.gsub(/\s+#{Regexp.escape(timezone)}$/, "") + [cron_without_timezone, timezone] + else + [cron_expression, nil] + end + end + end + + # Main integration class that handles all cron monitoring setup + # This class follows Good Job's integration patterns and Sentry's extension guidelines + class Integration + # Set up monitoring for all scheduled jobs from Good Job configuration + def self.setup_monitoring_for_scheduled_jobs + return unless ::Sentry.initialized? + return unless ::Sentry.configuration.good_job.auto_setup_cron_monitoring + + cron_config = ::Rails.application.config.good_job.cron + return unless cron_config.present? + + cron_config.each do |job_name, job_config| + setup_monitoring_for_job(job_name, job_config) + end + + Sentry::GoodJob::Logger.info "Sentry cron monitoring setup for #{cron_config.keys.size} scheduled jobs" + end + + # Set up monitoring for a specific job + def self.setup_monitoring_for_job(job_name, job_config) + job_class_name = job_config[:class] + cron_expression = job_config[:cron] + + return unless job_class_name && cron_expression + + # Get the job class + job_class = begin + job_class_name.constantize + rescue NameError => e + Sentry::GoodJob::Logger.warn "Could not find job class '#{job_class_name}' for Sentry cron monitoring: #{e.message}" + return + end + + # Set up job monitoring if not already set up + Sentry::GoodJob::JobMonitor.setup_for_job_class(job_class) + + # Parse cron expression and create monitor config + cron_without_tz, timezone = Sentry::GoodJob::CronMonitoring::Helpers.parse_cron_with_timezone(cron_expression) + monitor_config = Sentry::GoodJob::CronMonitoring::Helpers.monitor_config_from_cron(cron_without_tz, timezone: timezone) + + if monitor_config + # Configure Sentry cron monitoring - use job_name as slug for consistency + monitor_slug = Sentry::GoodJob::CronMonitoring::Helpers.monitor_slug(job_name) + + # only patch if not explicitly included in job by user + unless job_class.ancestors.include?(Sentry::Cron::MonitorCheckIns) + job_class.include(Sentry::Cron::MonitorCheckIns) + end + + job_class.sentry_monitor_check_ins( + slug: monitor_slug, + monitor_config: monitor_config + ) + + Sentry::GoodJob::Logger.info "Added Sentry cron monitoring for #{job_class_name} (#{monitor_slug})" + else + Sentry::GoodJob::Logger.warn "Could not create monitor config for #{job_class_name} with cron '#{cron_expression}'" + end + end + + # Manually add cron monitoring to a specific job + def self.add_monitoring_to_job(job_class, slug: nil, cron_expression: nil, timezone: nil) + return unless ::Sentry.initialized? + + # Set up job monitoring if not already set up + Sentry::GoodJob::JobMonitor.setup_for_job_class(job_class) + + # Create monitor config + monitor_config = if cron_expression + Sentry::GoodJob::CronMonitoring::Helpers.monitor_config_from_cron(cron_expression, timezone: timezone) + else + # Default to hourly monitoring if no cron expression provided + ::Sentry::Cron::MonitorConfig.from_crontab("0 * * * *") + end + + if monitor_config + monitor_slug = slug || Sentry::GoodJob::CronMonitoring::Helpers.monitor_slug(job_class.name) + + # only patch if not explicitly included in job by user + unless job_class.ancestors.include?(Sentry::Cron::MonitorCheckIns) + job_class.include(Sentry::Cron::MonitorCheckIns) + end + + job_class.sentry_monitor_check_ins( + slug: monitor_slug, + monitor_config: monitor_config + ) + + Sentry::GoodJob::Logger.info "Added Sentry cron monitoring for #{job_class.name} (#{monitor_slug})" + end + end + end + end + end +end diff --git a/sentry-good_job/lib/sentry/good_job/error_handler.rb b/sentry-good_job/lib/sentry/good_job/error_handler.rb new file mode 100644 index 000000000..cf6040c8c --- /dev/null +++ b/sentry-good_job/lib/sentry/good_job/error_handler.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module Sentry + module GoodJob + class ErrorHandler + # Default retry attempts for ActiveJob (matches ActiveJob::Base.retry_on default) + DEFAULT_RETRY_ATTEMPTS = 5 + + # @param ex [Exception] the exception / error that occurred + # @param job [ActiveJob::Base] the job instance that failed + def call(ex, job) + return unless Sentry.initialized? + + # Skip reporting if configured to only report after all retries are exhausted + if Sentry.configuration.good_job.report_after_job_retries && retryable?(job) + retry_count = job.executions + max_retries = job.class.retry_on_attempts || DEFAULT_RETRY_ATTEMPTS + return if retry_count < max_retries + end + + # Skip reporting if configured to only report dead jobs and this job can be retried + return if Sentry.configuration.good_job.report_only_dead_jobs && retryable?(job) + + # Report to Sentry via GoodJob wrapper for testability + # Context is already set by the job concern + Sentry::GoodJob.capture_exception( + ex, + contexts: { good_job: job_context(job) }, + hint: { background: true } + ) + end + + private + + def retryable?(job) + job.class.retry_on_attempts.present? && job.class.retry_on_attempts > 0 + end + + def job_context(job) + context = { + job_class: job.class.name, + job_id: job.job_id, + queue_name: job.queue_name, + executions: job.executions, + enqueued_at: job.enqueued_at, + scheduled_at: job.scheduled_at + } + + if Sentry.configuration.good_job.include_job_arguments + context[:arguments] = job.arguments.map(&:inspect) + end + + context + end + end + end +end diff --git a/sentry-good_job/lib/sentry/good_job/job_monitor.rb b/sentry-good_job/lib/sentry/good_job/job_monitor.rb new file mode 100644 index 000000000..66c8791b9 --- /dev/null +++ b/sentry-good_job/lib/sentry/good_job/job_monitor.rb @@ -0,0 +1,166 @@ +# frozen_string_literal: true + +# Unified job monitoring for Sentry GoodJob integration +# Combines context setting, tracing, and cron monitoring in a single class +module Sentry + module GoodJob + class JobMonitor + def self.setup_for_job_class(job_class) + return unless defined?(::Rails) && ::Sentry.initialized? + + # Include Sentry::Cron::MonitorCheckIns module for cron monitoring + # only patch if not explicitly included in job by user + unless job_class.ancestors.include?(Sentry::Cron::MonitorCheckIns) + job_class.include(Sentry::Cron::MonitorCheckIns) + end + + # Add Sentry context attributes + job_class.attr_accessor :_sentry + + # Set up around_enqueue hook (only if not already set up) + unless job_class.respond_to?(:sentry_enqueue_hook_setup) + job_class.define_singleton_method(:sentry_enqueue_hook_setup) { true } + + job_class.around_enqueue do |job, block| + next block.call unless ::Sentry.initialized? + + ::Sentry.with_child_span(op: "queue.publish", description: job.class.to_s) do |span| + _sentry_set_span_data(span, id: job.job_id, queue: job.queue_name) + block.call + end + end + end + + # Set up around_perform hook with unified functionality (only if not already set up) + unless job_class.respond_to?(:sentry_perform_hook_setup) + job_class.define_singleton_method(:sentry_perform_hook_setup) { true } + + job_class.around_perform do |job, block| + next block.call unless ::Sentry.initialized? + + # Set up Sentry context + ::Sentry.clone_hub_to_current_thread + scope = ::Sentry.get_current_scope + if (user = job._sentry&.dig("user")) + scope.set_user(user) + end + scope.set_tags(queue: job.queue_name, job_id: job.job_id) + scope.set_contexts(active_job: _sentry_job_context(job)) + scope.set_transaction_name(job.class.name, source: :task) + transaction = _sentry_start_transaction(scope, job._sentry&.dig("trace_propagation_headers")) + + if transaction + scope.set_span(transaction) + + latency = ((Time.now.utc - job.enqueued_at) * 1000).to_i if job.enqueued_at + retry_count = job.executions.is_a?(Integer) ? job.executions - 1 : 0 + + _sentry_set_span_data( + transaction, + id: job.job_id, + queue: job.queue_name, + latency: latency, + retry_count: retry_count + ) + end + + begin + block.call + _sentry_finish_transaction(transaction, 200) + rescue => error + _sentry_finish_transaction(transaction, 500) + raise + end + end + end + + # Add convenience method for cron monitoring configuration + job_class.define_singleton_method(:sentry_cron_monitor) do |cron_expression, timezone: nil, slug: nil| + return unless ::Sentry.initialized? + + # Create monitor config + monitor_config = Sentry::GoodJob::CronMonitoring::Helpers.monitor_config_from_cron(cron_expression, timezone: timezone) + return unless monitor_config + + # Use provided slug or generate from class name + monitor_slug = slug || Sentry::GoodJob::CronMonitoring::Helpers.monitor_slug(name) + + sentry_monitor_check_ins(slug: monitor_slug, monitor_config: monitor_config) + end + + # Add instance methods for Sentry context + job_class.define_method(:enqueue) do |options = {}| + self._sentry ||= {} + + user = ::Sentry.get_current_scope&.user + self._sentry["user"] = user if user.present? + + self._sentry["trace_propagation_headers"] = ::Sentry.get_trace_propagation_headers + + super(options) + end + + job_class.define_method(:serialize) do + begin + super().tap do |job_data| + if _sentry + job_data["_sentry"] = _sentry.to_json + end + end + rescue JSON::GeneratorError, TypeError + # Swallow JSON serialization errors. Better to lose Sentry context than fail to serialize the job. + super() + end + end + + job_class.define_method(:deserialize) do |job_data| + super(job_data) + + begin + self._sentry = JSON.parse(job_data["_sentry"]) if job_data["_sentry"] + rescue JSON::ParserError + # Swallow JSON parsing errors. Better to lose Sentry context than fail to deserialize the job. + end + end + + # Add private helper methods + job_class.define_method(:_sentry_set_span_data) do |span, id:, queue:, latency: nil, retry_count: nil| + if span + span.set_data("messaging.message.id", id) + span.set_data("messaging.destination.name", queue) + span.set_data("messaging.message.receive.latency", latency) if latency + span.set_data("messaging.message.retry.count", retry_count) if retry_count + end + end + + job_class.define_method(:_sentry_job_context) do |job| + job.serialize.symbolize_keys.except(:arguments, :_sentry) + end + + job_class.define_method(:_sentry_start_transaction) do |scope, env| + options = { + name: scope.transaction_name, + source: scope.transaction_source, + op: "queue.process", + origin: "auto.queue.active_job" + } + + transaction = ::Sentry.continue_trace(env, **options) + ::Sentry.start_transaction(transaction: transaction, **options) + end + + job_class.define_method(:_sentry_finish_transaction) do |transaction, status| + return unless transaction + + transaction.set_http_status(status) + transaction.finish + end + + # Make helper methods private + job_class.class_eval do + private :_sentry_set_span_data, :_sentry_job_context, :_sentry_start_transaction, :_sentry_finish_transaction + end + end + end + end +end diff --git a/sentry-good_job/lib/sentry/good_job/logger.rb b/sentry-good_job/lib/sentry/good_job/logger.rb new file mode 100644 index 000000000..1bb97516c --- /dev/null +++ b/sentry-good_job/lib/sentry/good_job/logger.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +# Centralized logging utility for Sentry Good Job integration +module Sentry + module GoodJob + module Logger + def self.enabled? + ::Sentry.configuration.good_job.logging_enabled && logger.present? + end + + def self.logger + if ::Sentry.configuration.good_job.logger + ::Sentry.configuration.good_job.logger + elsif defined?(::Rails) && ::Rails.respond_to?(:logger) + ::Rails.logger + else + nil + end + end + + def self.info(message) + return unless enabled? + + logger.info(message) + end + + def self.warn(message) + return unless enabled? + + logger.warn(message) + end + + def self.error(message) + return unless enabled? + + logger.error(message) + end + end + end +end diff --git a/sentry-good_job/lib/sentry/good_job/version.rb b/sentry-good_job/lib/sentry/good_job/version.rb new file mode 100644 index 000000000..04f60d81c --- /dev/null +++ b/sentry-good_job/lib/sentry/good_job/version.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module Sentry + module GoodJob + VERSION = "5.28.0" + end +end diff --git a/sentry-good_job/sentry-good_job.gemspec b/sentry-good_job/sentry-good_job.gemspec new file mode 100644 index 000000000..86b7432bf --- /dev/null +++ b/sentry-good_job/sentry-good_job.gemspec @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require_relative "lib/sentry/good_job/version" + +Gem::Specification.new do |spec| + spec.name = "sentry-good_job" + spec.version = Sentry::GoodJob::VERSION + spec.authors = ["Sentry Team"] + spec.description = spec.summary = "A gem that provides Good Job integration for the Sentry error logger" + spec.email = "accounts@sentry.io" + spec.license = 'MIT' + + spec.platform = Gem::Platform::RUBY + spec.required_ruby_version = '>= 2.4' + spec.extra_rdoc_files = ["README.md", "LICENSE.txt"] + spec.files = `git ls-files | grep -Ev '^(spec|benchmarks|examples|\.rubocop\.yml)'`.split("\n") + + github_root_uri = 'https://github.com/getsentry/sentry-ruby' + spec.homepage = "#{github_root_uri}/tree/#{spec.version}/#{spec.name}" + + spec.metadata = { + "homepage_uri" => spec.homepage, + "source_code_uri" => spec.homepage, + "changelog_uri" => "#{github_root_uri}/blob/#{spec.version}/CHANGELOG.md", + "bug_tracker_uri" => "#{github_root_uri}/issues", + "documentation_uri" => "http://www.rubydoc.info/gems/#{spec.name}/#{spec.version}" + } + + spec.bindir = "exe" + spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } + spec.require_paths = ["lib"] + + spec.add_dependency "sentry-ruby", "~> 5.28.0" + spec.add_dependency "good_job", ">= 3.0" +end diff --git a/sentry-good_job/spec/sentry/good_job/configuration_spec.rb b/sentry-good_job/spec/sentry/good_job/configuration_spec.rb new file mode 100644 index 000000000..1a97b3226 --- /dev/null +++ b/sentry-good_job/spec/sentry/good_job/configuration_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Sentry::GoodJob::Configuration do + before do + perform_basic_setup + end + + let(:config) { Sentry.configuration.good_job } + + describe "default values" do + it "sets default values correctly" do + expect(config.report_after_job_retries).to eq(false) + expect(config.report_only_dead_jobs).to eq(false) + expect(config.propagate_traces).to eq(true) + expect(config.include_job_arguments).to eq(false) + expect(config.auto_setup_cron_monitoring).to eq(true) + expect(config.logging_enabled).to eq(false) + expect(config.logger).to be_nil + end + end + + describe "IGNORE_DEFAULT" do + it "includes expected exceptions" do + expect(Sentry::GoodJob::IGNORE_DEFAULT).to include( + "ActiveJob::DeserializationError", + "ActiveJob::SerializationError" + ) + end + end + + describe "configuration attributes" do + it "allows setting report_after_job_retries" do + config.report_after_job_retries = true + expect(config.report_after_job_retries).to eq(true) + end + + it "allows setting report_only_dead_jobs" do + config.report_only_dead_jobs = true + expect(config.report_only_dead_jobs).to eq(true) + end + + it "allows setting propagate_traces" do + config.propagate_traces = false + expect(config.propagate_traces).to eq(false) + end + + it "allows setting include_job_arguments" do + config.include_job_arguments = true + expect(config.include_job_arguments).to eq(true) + end + + it "allows setting auto_setup_cron_monitoring" do + config.auto_setup_cron_monitoring = false + expect(config.auto_setup_cron_monitoring).to eq(false) + end + + it "allows setting logging_enabled" do + config.logging_enabled = true + expect(config.logging_enabled).to eq(true) + end + + it "allows setting logger" do + logger = double("Logger") + config.logger = logger + expect(config.logger).to eq(logger) + end + end +end diff --git a/sentry-good_job/spec/sentry/good_job/cron_monitoring_spec.rb b/sentry-good_job/spec/sentry/good_job/cron_monitoring_spec.rb new file mode 100644 index 000000000..ec04459ea --- /dev/null +++ b/sentry-good_job/spec/sentry/good_job/cron_monitoring_spec.rb @@ -0,0 +1,278 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Sentry::GoodJob::CronMonitoring do + before do + perform_basic_setup + end + + describe "Helpers" do + describe ".monitor_config_from_cron" do + it "returns nil for empty cron expression" do + expect(described_class::Helpers.monitor_config_from_cron("")).to be_nil + end + + it "returns nil for nil cron expression" do + expect(described_class::Helpers.monitor_config_from_cron(nil)).to be_nil + end + + it "creates monitor config for valid cron expression" do + config = described_class::Helpers.monitor_config_from_cron("0 * * * *") + expect(config).to be_a(Sentry::Cron::MonitorConfig) + end + + it "creates monitor config with timezone" do + config = described_class::Helpers.monitor_config_from_cron("0 * * * *", timezone: "UTC") + expect(config).to be_a(Sentry::Cron::MonitorConfig) + end + + it "handles parsing errors gracefully" do + allow(Fugit).to receive(:parse_cron).and_raise(StandardError.new("Invalid cron")) + allow(Sentry::GoodJob::Logger).to receive(:warn) + + result = described_class::Helpers.monitor_config_from_cron("invalid") + + expect(result).to be_nil + expect(Sentry::GoodJob::Logger).to have_received(:warn) + end + end + + describe ".monitor_slug" do + it "converts job name to slug" do + expect(described_class::Helpers.monitor_slug("TestJob")).to eq("test") + end + + it "removes _job suffix" do + expect(described_class::Helpers.monitor_slug("TestJob")).to eq("test") + end + + it "handles snake_case names" do + expect(described_class::Helpers.monitor_slug("test_job")).to eq("test") + end + end + + describe ".parse_cron_with_timezone" do + it "returns cron and nil timezone for simple cron" do + cron, timezone = described_class::Helpers.parse_cron_with_timezone("0 * * * *") + expect(cron).to eq("0 * * * *") + expect(timezone).to be_nil + end + + it "extracts timezone from cron with timezone" do + cron, timezone = described_class::Helpers.parse_cron_with_timezone("0 * * * * UTC") + expect(cron).to eq("0 * * * *") + expect(timezone).to eq("UTC") + end + + it "extracts complex timezone from cron" do + cron, timezone = described_class::Helpers.parse_cron_with_timezone("0 * * * * Europe/Stockholm") + expect(cron).to eq("0 * * * *") + expect(timezone).to eq("Europe/Stockholm") + end + + it "handles invalid timezone format" do + cron, timezone = described_class::Helpers.parse_cron_with_timezone("0 * * * * invalid-timezone") + expect(cron).to eq("0 * * * * invalid-timezone") + expect(timezone).to be_nil + end + + it "returns original cron for short expressions" do + cron, timezone = described_class::Helpers.parse_cron_with_timezone("0 * * *") + expect(cron).to eq("0 * * *") + expect(timezone).to be_nil + end + end + end + + describe "Integration" do + let(:rails_app) { double("RailsApplication") } + let(:rails_config) { double("RailsConfig") } + let(:good_job_config) { double("GoodJobConfig") } + + before do + stub_const("Rails", double("Rails")) + allow(Rails).to receive(:application).and_return(rails_app) + allow(rails_app).to receive(:config).and_return(rails_config) + allow(rails_config).to receive(:good_job).and_return(good_job_config) + end + + describe ".setup_monitoring_for_scheduled_jobs" do + context "when Sentry is not initialized" do + before do + allow(Sentry).to receive(:initialized?).and_return(false) + end + + it "does not set up monitoring" do + expect(described_class::Integration).not_to receive(:setup_monitoring_for_job) + described_class::Integration.setup_monitoring_for_scheduled_jobs + end + end + + context "when auto_setup_cron_monitoring is disabled" do + before do + allow(Sentry).to receive(:initialized?).and_return(true) + Sentry.configuration.good_job.auto_setup_cron_monitoring = false + end + + it "does not set up monitoring" do + expect(described_class::Integration).not_to receive(:setup_monitoring_for_job) + described_class::Integration.setup_monitoring_for_scheduled_jobs + end + end + + context "when cron config is not present" do + before do + allow(Sentry).to receive(:initialized?).and_return(true) + Sentry.configuration.good_job.auto_setup_cron_monitoring = true + allow(good_job_config).to receive(:cron).and_return(nil) + end + + it "does not set up monitoring" do + expect(described_class::Integration).not_to receive(:setup_monitoring_for_job) + described_class::Integration.setup_monitoring_for_scheduled_jobs + end + end + + context "when cron config is present" do + let(:cron_config) do + { + "test_job" => { class: "TestJob", cron: "0 * * * *" }, + "another_job" => { class: "AnotherJob", cron: "0 0 * * *" } + } + end + + before do + allow(Sentry).to receive(:initialized?).and_return(true) + Sentry.configuration.good_job.auto_setup_cron_monitoring = true + allow(good_job_config).to receive(:cron).and_return(cron_config) + end + + it "sets up monitoring for each job" do + expect(described_class::Integration).to receive(:setup_monitoring_for_job).with("test_job", cron_config["test_job"]) + expect(described_class::Integration).to receive(:setup_monitoring_for_job).with("another_job", cron_config["another_job"]) + + described_class::Integration.setup_monitoring_for_scheduled_jobs + end + + it "logs the setup completion" do + allow(described_class::Integration).to receive(:setup_monitoring_for_job) + allow(Sentry::GoodJob::Logger).to receive(:info) + + described_class::Integration.setup_monitoring_for_scheduled_jobs + + expect(Sentry::GoodJob::Logger).to have_received(:info).with("Sentry cron monitoring setup for 2 scheduled jobs") + end + end + end + + describe ".setup_monitoring_for_job" do + let(:job_class) { Class.new(ActiveJob::Base) } + + before do + allow(Sentry).to receive(:initialized?).and_return(true) + stub_const("TestJob", job_class) + end + + context "when job class is missing" do + let(:job_config) { { class: "NonExistentJob", cron: "0 * * * *" } } + + it "logs a warning and returns" do + allow(Sentry::GoodJob::Logger).to receive(:warn) + + described_class::Integration.setup_monitoring_for_job("test_job", job_config) + + expect(Sentry::GoodJob::Logger).to have_received(:warn).with(/Could not find job class/) + end + end + + context "when job config is missing class" do + let(:job_config) { { cron: "0 * * * *" } } + + it "does not set up monitoring" do + expect(Sentry::GoodJob::JobMonitor).not_to receive(:setup_for_job_class) + described_class::Integration.setup_monitoring_for_job("test_job", job_config) + end + end + + context "when job config is missing cron" do + let(:job_config) { { class: "TestJob" } } + + it "does not set up monitoring" do + expect(Sentry::GoodJob::JobMonitor).not_to receive(:setup_for_job_class) + described_class::Integration.setup_monitoring_for_job("test_job", job_config) + end + end + + context "when job config is complete" do + let(:job_config) { { class: "TestJob", cron: "0 * * * *" } } + + it "sets up job monitoring" do + expect(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class).with(job_class) + described_class::Integration.setup_monitoring_for_job("test_job", job_config) + end + + it "sets up cron monitoring" do + allow(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class) + expect(job_class).to receive(:sentry_monitor_check_ins) + + described_class::Integration.setup_monitoring_for_job("test_job", job_config) + end + + it "logs the setup completion" do + allow(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class) + allow(job_class).to receive(:sentry_monitor_check_ins) + allow(Sentry::GoodJob::Logger).to receive(:info) + + described_class::Integration.setup_monitoring_for_job("test_job", job_config) + + expect(Sentry::GoodJob::Logger).to have_received(:info).with(/Added Sentry cron monitoring for TestJob/) + end + end + end + + describe ".add_monitoring_to_job" do + let(:job_class) { Class.new(ActiveJob::Base) } + + before do + allow(Sentry).to receive(:initialized?).and_return(true) + end + + it "sets up job monitoring" do + expect(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class).with(job_class) + described_class::Integration.add_monitoring_to_job(job_class) + end + + it "sets up cron monitoring with default config" do + allow(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class) + expect(job_class).to receive(:sentry_monitor_check_ins) + + described_class::Integration.add_monitoring_to_job(job_class) + end + + it "uses provided slug" do + allow(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class) + expect(job_class).to receive(:sentry_monitor_check_ins).with(hash_including(slug: "custom_slug")) + + described_class::Integration.add_monitoring_to_job(job_class, slug: "custom_slug") + end + + it "uses provided cron expression" do + allow(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class) + expect(job_class).to receive(:sentry_monitor_check_ins) + + described_class::Integration.add_monitoring_to_job(job_class, cron_expression: "0 0 * * *") + end + + it "logs the setup completion" do + allow(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class) + allow(job_class).to receive(:sentry_monitor_check_ins) + allow(Sentry::GoodJob::Logger).to receive(:info) + + described_class::Integration.add_monitoring_to_job(job_class) + + expect(Sentry::GoodJob::Logger).to have_received(:info).with(/Added Sentry cron monitoring/) + end + end + end +end diff --git a/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb b/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb new file mode 100644 index 000000000..16f9c793e --- /dev/null +++ b/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb @@ -0,0 +1,199 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Sentry::GoodJob::ErrorHandler do + before do + perform_basic_setup + end + + let(:handler) { described_class.new } + let(:transport) { Sentry.get_current_client.transport } + + describe "#call" do + let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 1, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } + let(:job_class) { double("JobClass", retry_on_attempts: nil, name: "TestJob") } + let(:exception) { build_exception } + + context "when Sentry is not initialized" do + before do + allow(Sentry).to receive(:initialized?).and_return(false) + end + + it "does not capture the exception" do + expect(Sentry::GoodJob).not_to receive(:capture_exception) + handler.call(exception, job) + end + end + + context "when Sentry is initialized" do + before do + allow(Sentry).to receive(:initialized?).and_return(true) + end + + it "captures the exception with correct context" do + expect(Sentry::GoodJob).to receive(:capture_exception).with( + exception, + contexts: { good_job: hash_including( + job_class: "TestJob", + job_id: "123", + queue_name: "default", + executions: 1 + ) }, + hint: { background: true } + ) + + handler.call(exception, job) + end + + context "when include_job_arguments is enabled" do + before do + Sentry.configuration.good_job.include_job_arguments = true + end + + it "includes job arguments in context" do + expect(Sentry::GoodJob).to receive(:capture_exception).with( + exception, + contexts: { good_job: hash_including(arguments: []) }, + hint: { background: true } + ) + + handler.call(exception, job) + end + end + + context "when report_after_job_retries is enabled" do + before do + Sentry.configuration.good_job.report_after_job_retries = true + end + + context "and job is retryable" do + let(:job_class) { double("JobClass", retry_on_attempts: 3, name: "TestJob") } + + context "and retry count is less than max retries" do + let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 2, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } + + it "does not capture the exception" do + expect(Sentry::GoodJob).not_to receive(:capture_exception) + handler.call(exception, job) + end + end + + context "and retry count equals max retries" do + let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 3, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } + + it "captures the exception" do + expect(Sentry::GoodJob).to receive(:capture_exception) + handler.call(exception, job) + end + end + end + + context "and job is not retryable" do + let(:job_class) { double("JobClass", retry_on_attempts: nil, name: "TestJob") } + + it "captures the exception" do + expect(Sentry::GoodJob).to receive(:capture_exception) + handler.call(exception, job) + end + end + end + + context "when report_only_dead_jobs is enabled" do + before do + Sentry.configuration.good_job.report_only_dead_jobs = true + end + + context "and job is retryable" do + let(:job_class) { double("JobClass", retry_on_attempts: 3, name: "TestJob") } + + it "does not capture the exception" do + expect(Sentry::GoodJob).not_to receive(:capture_exception) + handler.call(exception, job) + end + end + + context "and job is not retryable" do + let(:job_class) { double("JobClass", retry_on_attempts: nil, name: "TestJob") } + + it "captures the exception" do + expect(Sentry::GoodJob).to receive(:capture_exception) + handler.call(exception, job) + end + end + end + end + end + + describe "#retryable?" do + it "returns true when job has retry_on_attempts" do + job_class = double("JobClass", retry_on_attempts: 3) + job = double("Job", class: job_class) + + expect(handler.send(:retryable?, job)).to be true + end + + it "returns false when job has no retry_on_attempts" do + job_class = double("JobClass", retry_on_attempts: nil) + job = double("Job", class: job_class) + + expect(handler.send(:retryable?, job)).to be false + end + + it "returns false when job has retry_on_attempts of 0" do + job_class = double("JobClass", retry_on_attempts: 0) + job = double("Job", class: job_class) + + expect(handler.send(:retryable?, job)).to be false + end + end + + describe "#job_context" do + let(:job) do + double("Job", + class: double("JobClass", name: "TestJob"), + job_id: "123", + queue_name: "default", + executions: 1, + enqueued_at: Time.now, + scheduled_at: nil, + arguments: ["arg1", "arg2"] + ) + end + + it "returns basic job context" do + context = handler.send(:job_context, job) + + expect(context).to include( + job_class: "TestJob", + job_id: "123", + queue_name: "default", + executions: 1 + ) + end + + context "when include_job_arguments is enabled" do + before do + Sentry.configuration.good_job.include_job_arguments = true + end + + it "includes arguments in context" do + context = handler.send(:job_context, job) + + expect(context[:arguments]).to eq(['"arg1"', '"arg2"']) + end + end + + context "when include_job_arguments is disabled" do + before do + Sentry.configuration.good_job.include_job_arguments = false + end + + it "does not include arguments in context" do + context = handler.send(:job_context, job) + + expect(context).not_to have_key(:arguments) + end + end + end +end diff --git a/sentry-good_job/spec/sentry/good_job/job_monitor_spec.rb b/sentry-good_job/spec/sentry/good_job/job_monitor_spec.rb new file mode 100644 index 000000000..aa64fcf56 --- /dev/null +++ b/sentry-good_job/spec/sentry/good_job/job_monitor_spec.rb @@ -0,0 +1,120 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Sentry::GoodJob::JobMonitor do + before do + perform_basic_setup + end + + let(:job_class) { Class.new(ActiveJob::Base) } + + describe ".setup_for_job_class" do + context "when Rails is not available" do + before do + hide_const("Rails") + end + + it "does not set up monitoring" do + expect(job_class).not_to receive(:include) + described_class.setup_for_job_class(job_class) + end + end + + context "when Sentry is not initialized" do + before do + stub_const("Rails", double("Rails")) + allow(Sentry).to receive(:initialized?).and_return(false) + end + + it "does not set up monitoring" do + expect(job_class).not_to receive(:include) + described_class.setup_for_job_class(job_class) + end + end + + context "when Rails and Sentry are available" do + before do + stub_const("Rails", double("Rails")) + allow(Sentry).to receive(:initialized?).and_return(true) + end + + it "includes Sentry::Cron::MonitorCheckIns" do + expect(job_class).to receive(:include).with(Sentry::Cron::MonitorCheckIns) + described_class.setup_for_job_class(job_class) + end + + it "adds _sentry attribute accessor" do + expect(job_class).to receive(:attr_accessor).with(:_sentry) + described_class.setup_for_job_class(job_class) + end + + it "sets up around_enqueue hook" do + expect(job_class).to receive(:around_enqueue) + described_class.setup_for_job_class(job_class) + end + + it "sets up around_perform hook" do + expect(job_class).to receive(:around_perform) + described_class.setup_for_job_class(job_class) + end + + it "adds sentry_cron_monitor class method" do + described_class.setup_for_job_class(job_class) + expect(job_class).to respond_to(:sentry_cron_monitor) + end + + it "adds enqueue instance method" do + described_class.setup_for_job_class(job_class) + expect(job_class.instance_methods).to include(:enqueue) + end + + it "adds serialize instance method" do + described_class.setup_for_job_class(job_class) + expect(job_class.instance_methods).to include(:serialize) + end + + it "adds deserialize instance method" do + described_class.setup_for_job_class(job_class) + expect(job_class.instance_methods).to include(:deserialize) + end + + it "adds private helper methods" do + described_class.setup_for_job_class(job_class) + expect(job_class.private_instance_methods).to include(:_sentry_set_span_data) + expect(job_class.private_instance_methods).to include(:_sentry_job_context) + expect(job_class.private_instance_methods).to include(:_sentry_start_transaction) + expect(job_class.private_instance_methods).to include(:_sentry_finish_transaction) + end + + it "makes helper methods private" do + expect(job_class).to receive(:class_eval).at_least(:once) + described_class.setup_for_job_class(job_class) + end + + context "when job class already has Sentry::Cron::MonitorCheckIns included" do + before do + job_class.include(Sentry::Cron::MonitorCheckIns) + end + + it "does not include it again" do + expect(job_class).not_to receive(:include).with(Sentry::Cron::MonitorCheckIns) + described_class.setup_for_job_class(job_class) + end + end + + context "when hooks are already set up" do + before do + job_class.define_singleton_method(:sentry_enqueue_hook_setup) { true } + job_class.define_singleton_method(:sentry_perform_hook_setup) { true } + end + + it "does not set up hooks again" do + expect(job_class).not_to receive(:around_enqueue) + expect(job_class).not_to receive(:around_perform) + described_class.setup_for_job_class(job_class) + end + end + end + end +end diff --git a/sentry-good_job/spec/sentry/good_job/logger_spec.rb b/sentry-good_job/spec/sentry/good_job/logger_spec.rb new file mode 100644 index 000000000..f7ea67710 --- /dev/null +++ b/sentry-good_job/spec/sentry/good_job/logger_spec.rb @@ -0,0 +1,161 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Sentry::GoodJob::Logger do + before do + perform_basic_setup + end + + let(:mock_logger) { double("Logger") } + + describe ".enabled?" do + context "when logging is disabled" do + before do + Sentry.configuration.good_job.logging_enabled = false + end + + it "returns false" do + expect(described_class.enabled?).to be false + end + end + + context "when logging is enabled but no logger is available" do + before do + Sentry.configuration.good_job.logging_enabled = true + Sentry.configuration.good_job.logger = nil + allow(described_class).to receive(:logger).and_return(nil) + end + + it "returns false" do + expect(described_class.enabled?).to be false + end + end + + context "when logging is enabled and logger is available" do + before do + Sentry.configuration.good_job.logging_enabled = true + allow(described_class).to receive(:logger).and_return(mock_logger) + end + + it "returns true" do + expect(described_class.enabled?).to be true + end + end + end + + describe ".logger" do + context "when custom logger is configured" do + before do + Sentry.configuration.good_job.logger = mock_logger + end + + it "returns the custom logger" do + expect(described_class.logger).to eq(mock_logger) + end + end + + context "when no custom logger is configured" do + before do + Sentry.configuration.good_job.logger = nil + end + + context "and Rails is available" do + before do + stub_const("Rails", double("Rails")) + allow(Rails).to receive(:respond_to?).with(:logger).and_return(true) + allow(Rails).to receive(:logger).and_return(mock_logger) + end + + it "returns Rails.logger" do + expect(described_class.logger).to eq(mock_logger) + end + end + + context "and Rails is not available" do + before do + hide_const("Rails") + end + + it "returns nil" do + expect(described_class.logger).to be_nil + end + end + end + end + + describe ".info" do + context "when logging is enabled" do + before do + Sentry.configuration.good_job.logging_enabled = true + allow(described_class).to receive(:logger).and_return(mock_logger) + end + + it "logs the message" do + expect(mock_logger).to receive(:info).with("test message") + described_class.info("test message") + end + end + + context "when logging is disabled" do + before do + Sentry.configuration.good_job.logging_enabled = false + end + + it "does not log the message" do + expect(mock_logger).not_to receive(:info) + described_class.info("test message") + end + end + end + + describe ".warn" do + context "when logging is enabled" do + before do + Sentry.configuration.good_job.logging_enabled = true + allow(described_class).to receive(:logger).and_return(mock_logger) + end + + it "logs the warning" do + expect(mock_logger).to receive(:warn).with("test warning") + described_class.warn("test warning") + end + end + + context "when logging is disabled" do + before do + Sentry.configuration.good_job.logging_enabled = false + end + + it "does not log the warning" do + expect(mock_logger).not_to receive(:warn) + described_class.warn("test warning") + end + end + end + + describe ".error" do + context "when logging is enabled" do + before do + Sentry.configuration.good_job.logging_enabled = true + allow(described_class).to receive(:logger).and_return(mock_logger) + end + + it "logs the error" do + expect(mock_logger).to receive(:error).with("test error") + described_class.error("test error") + end + end + + context "when logging is disabled" do + before do + Sentry.configuration.good_job.logging_enabled = false + end + + it "does not log the error" do + expect(mock_logger).not_to receive(:error) + described_class.error("test error") + end + end + end +end diff --git a/sentry-good_job/spec/sentry/good_job_spec.rb b/sentry-good_job/spec/sentry/good_job_spec.rb new file mode 100644 index 000000000..139ebd50a --- /dev/null +++ b/sentry-good_job/spec/sentry/good_job_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Sentry::GoodJob do + before do + perform_basic_setup + end + + let(:transport) do + Sentry.get_current_client.transport + end + + it "registers the integration" do + expect(Sentry.integrations).to have_key("good_job") + end + + it "has the correct version" do + expect(described_class::VERSION).to eq("5.28.0") + end + + describe "setup_good_job_integration" do + before do + # Mock ApplicationJob + stub_const("ApplicationJob", Class.new(ActiveJob::Base)) + + # Mock Rails application configuration + rails_app = double("Rails::Application") + rails_config = double("Rails::Configuration") + good_job_config = double("GoodJob::Configuration") + + allow(::Rails).to receive(:application).and_return(rails_app) + allow(rails_app).to receive(:config).and_return(rails_config) + allow(rails_config).to receive(:good_job).and_return(good_job_config) + allow(good_job_config).to receive(:cron).and_return({}) + end + + it "sets up job monitoring for ApplicationJob" do + expect(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class).with(ApplicationJob) + + described_class.setup_good_job_integration + end + + context "when auto_setup_cron_monitoring is enabled" do + before do + Sentry.configuration.good_job.auto_setup_cron_monitoring = true + end + + it "sets up cron monitoring" do + expect(Sentry::GoodJob::CronMonitoring::Integration).to receive(:setup_monitoring_for_scheduled_jobs) + + described_class.setup_good_job_integration + end + end + + context "when auto_setup_cron_monitoring is disabled" do + before do + Sentry.configuration.good_job.auto_setup_cron_monitoring = false + end + + it "does not set up cron monitoring" do + expect(Sentry::GoodJob::CronMonitoring::Integration).not_to receive(:setup_monitoring_for_scheduled_jobs) + + described_class.setup_good_job_integration + end + end + end + + describe "capture_exception" do + it "delegates to Sentry.capture_exception" do + exception = build_exception + options = { hint: { background: true } } + + expect(Sentry).to receive(:capture_exception).with(exception, **options) + + described_class.capture_exception(exception, **options) + end + end + + describe "Rails integration" do + before do + # Mock Rails configuration + rails_config = double("Rails::Configuration") + allow(rails_config).to receive(:good_job).and_return(double("GoodJobConfig")) + + # Mock Sentry Rails configuration + sentry_rails_config = double("Sentry::Rails::Configuration") + allow(sentry_rails_config).to receive(:skippable_job_adapters).and_return([]) + + allow(Sentry.configuration).to receive(:rails).and_return(sentry_rails_config) + end + + it "adds GoodJobAdapter to skippable_job_adapters" do + # This test verifies that the integration would add the adapter to the skippable list + # In a real Rails environment, this would be done by the Railtie + expect(Sentry.configuration.rails.skippable_job_adapters).to be_an(Array) + end + end +end diff --git a/sentry-good_job/spec/spec_helper.rb b/sentry-good_job/spec/spec_helper.rb new file mode 100644 index 000000000..7cdb9f948 --- /dev/null +++ b/sentry-good_job/spec/spec_helper.rb @@ -0,0 +1,185 @@ +# frozen_string_literal: true + +require "bundler/setup" +begin + require "debug/prelude" +rescue LoadError +end + +require "active_job" +require "good_job" + +require "sentry-ruby" +require "sentry/test_helper" + +# Fixing crash: +# activesupport-6.1.7.10/lib/active_support/logger_thread_safe_level.rb:16:in +# . `': uninitialized constant ActiveSupport::LoggerThreadSafeLevel::Logger (NameError) +require "logger" + +require 'simplecov' + +SimpleCov.start do + project_name "sentry-good_job" + root File.join(__FILE__, "../../../") + coverage_dir File.join(__FILE__, "../../coverage") +end + +if ENV["CI"] + require 'simplecov-cobertura' + SimpleCov.formatter = SimpleCov::Formatter::CoberturaFormatter +end + +require "sentry-good_job" + +DUMMY_DSN = 'http://12345:67890@sentry.localdomain/sentry/42' + +RSpec.configure do |config| + # Enable flags like --only-failures and --next-failure + config.example_status_persistence_file_path = ".rspec_status" + + # Disable RSpec exposing methods globally on `Module` and `main` + config.disable_monkey_patching! + + config.expect_with :rspec do |c| + c.syntax = :expect + end + + config.before :suite do + puts "\n" + puts "*" * 100 + puts "Running with Good Job #{GoodJob::VERSION}" + puts "*" * 100 + puts "\n" + end + + config.before :each do + # Make sure we reset the env in case something leaks in + ENV.delete('SENTRY_DSN') + ENV.delete('SENTRY_CURRENT_ENV') + ENV.delete('SENTRY_ENVIRONMENT') + ENV.delete('SENTRY_RELEASE') + ENV.delete('RACK_ENV') + end + + config.include(Sentry::TestHelper) + + config.after :each do + reset_sentry_globals! + end +end + +def build_exception + 1 / 0 +rescue ZeroDivisionError => e + e +end + +def build_exception_with_cause(cause = "exception a") + begin + raise cause + rescue + raise "exception b" + end +rescue RuntimeError => e + e +end + +def build_exception_with_two_causes + begin + begin + raise "exception a" + rescue + raise "exception b" + end + rescue + raise "exception c" + end +rescue RuntimeError => e + e +end + +class HappyJob < ActiveJob::Base + def perform + crumb = Sentry::Breadcrumb.new(message: "I'm happy!") + Sentry.add_breadcrumb(crumb) + Sentry.set_tags mood: 'happy' + end +end + +class SadJob < ActiveJob::Base + def perform + crumb = Sentry::Breadcrumb.new(message: "I'm sad!") + Sentry.add_breadcrumb(crumb) + Sentry.set_tags mood: 'sad' + + raise "I'm sad!" + end +end + +class VerySadJob < ActiveJob::Base + def perform + crumb = Sentry::Breadcrumb.new(message: "I'm very sad!") + Sentry.add_breadcrumb(crumb) + Sentry.set_tags mood: 'very sad' + + raise "I'm very sad!" + end +end + +class ReportingJob < ActiveJob::Base + def perform + Sentry.capture_message("I have something to say!") + end +end + +class HappyJobWithCron < HappyJob + include Sentry::Cron::MonitorCheckIns + sentry_monitor_check_ins +end + +class SadJobWithCron < SadJob + include Sentry::Cron::MonitorCheckIns + sentry_monitor_check_ins slug: "failed_job", monitor_config: Sentry::Cron::MonitorConfig.from_crontab("5 * * * *") +end + +class WorkloadJob < ActiveJob::Base + def perform + # Create some CPU work that should show up in the profile + calculate_fibonacci(25) + sleep_and_sort + generate_strings + end + + private + + def calculate_fibonacci(n) + return n if n <= 1 + calculate_fibonacci(n - 1) + calculate_fibonacci(n - 2) + end + + def sleep_and_sort + # Mix of CPU and IO work + sleep(0.01) + array = (1..1000).to_a.shuffle + array.sort + end + + def generate_strings + # Memory and CPU work + 100.times do |i| + "test string #{i}" * 100 + Math.sqrt(i * 1000) + end + end +end + +def perform_basic_setup + Sentry.init do |config| + config.dsn = DUMMY_DSN + config.sdk_logger = ::Logger.new(nil) + config.background_worker_threads = 0 + config.transport.transport_class = Sentry::DummyTransport + yield config if block_given? + end +end From 75c18f9994e77aa6fb7e52daf0ecfef418ee7f0d Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Thu, 16 Oct 2025 14:34:25 +0300 Subject: [PATCH 02/24] Update .gitignore and remove .rspec_status file - Added .rspec_status to .gitignore to prevent tracking of RSpec status files. - Deleted the existing .rspec_status file to clean up unnecessary artifacts from the repository. --- sentry-good_job/.gitignore | 3 ++ sentry-good_job/.rspec_status | 87 ----------------------------------- 2 files changed, 3 insertions(+), 87 deletions(-) delete mode 100644 sentry-good_job/.rspec_status diff --git a/sentry-good_job/.gitignore b/sentry-good_job/.gitignore index 2071284f1..7bbe4fbc0 100644 --- a/sentry-good_job/.gitignore +++ b/sentry-good_job/.gitignore @@ -76,3 +76,6 @@ node_modules/ # TernJS port file .tern-port + +# RSpec status file +.rspec_status diff --git a/sentry-good_job/.rspec_status b/sentry-good_job/.rspec_status deleted file mode 100644 index 154009e0a..000000000 --- a/sentry-good_job/.rspec_status +++ /dev/null @@ -1,87 +0,0 @@ -example_id | status | run_time | ------------------------------------------------------------ | ------ | --------------- | -./spec/sentry/good_job/configuration_spec.rb[1:1:1] | passed | 0.00139 seconds | -./spec/sentry/good_job/configuration_spec.rb[1:2:1] | passed | 0.00334 seconds | -./spec/sentry/good_job/configuration_spec.rb[1:3:1] | passed | 0.0003 seconds | -./spec/sentry/good_job/configuration_spec.rb[1:3:2] | passed | 0.00013 seconds | -./spec/sentry/good_job/configuration_spec.rb[1:3:3] | passed | 0.00012 seconds | -./spec/sentry/good_job/configuration_spec.rb[1:3:4] | passed | 0.00012 seconds | -./spec/sentry/good_job/configuration_spec.rb[1:3:5] | passed | 0.00012 seconds | -./spec/sentry/good_job/configuration_spec.rb[1:3:6] | passed | 0.00012 seconds | -./spec/sentry/good_job/configuration_spec.rb[1:3:7] | passed | 0.00028 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:1:1] | passed | 0.00013 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:1:2] | passed | 0.00012 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:1:3] | passed | 0.00075 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:1:4] | passed | 0.00036 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:1:5] | passed | 0.00576 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:2:1] | passed | 0.0002 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:2:2] | passed | 0.00014 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:2:3] | passed | 0.00013 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:3:1] | passed | 0.00014 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:3:2] | passed | 0.00015 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:3:3] | passed | 0.00013 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:3:4] | passed | 0.00012 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:1:3:5] | passed | 0.00012 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:1:1:1] | passed | 0.00036 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:1:2:1] | passed | 0.00034 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:1:3:1] | passed | 0.00039 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:1:4:1] | passed | 0.00047 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:1:4:2] | passed | 0.00043 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:2:1:1] | passed | 0.00031 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:2:2:1] | passed | 0.0003 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:2:3:1] | passed | 0.00028 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:2:4:1] | passed | 0.00066 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:2:4:2] | passed | 0.00083 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:2:4:3] | passed | 0.00393 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:3:1] | passed | 0.00047 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:3:2] | passed | 0.0004 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:3:3] | passed | 0.00045 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:3:4] | passed | 0.00084 seconds | -./spec/sentry/good_job/cron_monitoring_spec.rb[1:2:3:5] | passed | 0.00155 seconds | -./spec/sentry/good_job/error_handler_spec.rb[1:1:1:1] | passed | 0.00148 seconds | -./spec/sentry/good_job/error_handler_spec.rb[1:1:2:1] | passed | 0.00218 seconds | -./spec/sentry/good_job/error_handler_spec.rb[1:1:2:2:1] | passed | 0.00302 seconds | -./spec/sentry/good_job/error_handler_spec.rb[1:1:2:3:1:1:1] | passed | 0.0015 seconds | -./spec/sentry/good_job/error_handler_spec.rb[1:1:2:3:1:2:1] | passed | 0.00143 seconds | -./spec/sentry/good_job/error_handler_spec.rb[1:1:2:3:2:1] | passed | 0.0006 seconds | -./spec/sentry/good_job/error_handler_spec.rb[1:1:2:4:1:1] | passed | 0.00035 seconds | -./spec/sentry/good_job/error_handler_spec.rb[1:1:2:4:2:1] | passed | 0.00036 seconds | -./spec/sentry/good_job/error_handler_spec.rb[1:2:1] | passed | 0.0006 seconds | -./spec/sentry/good_job/error_handler_spec.rb[1:2:2] | passed | 0.00019 seconds | -./spec/sentry/good_job/error_handler_spec.rb[1:2:3] | passed | 0.00016 seconds | -./spec/sentry/good_job/error_handler_spec.rb[1:3:1] | passed | 0.00027 seconds | -./spec/sentry/good_job/error_handler_spec.rb[1:3:2:1] | passed | 0.00019 seconds | -./spec/sentry/good_job/error_handler_spec.rb[1:3:3:1] | passed | 0.00065 seconds | -./spec/sentry/good_job/job_monitor_spec.rb[1:1:1:1] | passed | 0.00023 seconds | -./spec/sentry/good_job/job_monitor_spec.rb[1:1:2:1] | passed | 0.00025 seconds | -./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:1] | passed | 0.00035 seconds | -./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:2] | passed | 0.00026 seconds | -./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:3] | passed | 0.00025 seconds | -./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:4] | passed | 0.00034 seconds | -./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:5] | passed | 0.0009 seconds | -./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:6] | passed | 0.00026 seconds | -./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:7] | passed | 0.00022 seconds | -./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:8] | passed | 0.00022 seconds | -./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:9] | passed | 0.00025 seconds | -./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:10] | passed | 0.00029 seconds | -./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:11:1] | passed | 0.00031 seconds | -./spec/sentry/good_job/job_monitor_spec.rb[1:1:3:12:1] | passed | 0.00027 seconds | -./spec/sentry/good_job/logger_spec.rb[1:1:1:1] | passed | 0.00013 seconds | -./spec/sentry/good_job/logger_spec.rb[1:1:2:1] | passed | 0.00018 seconds | -./spec/sentry/good_job/logger_spec.rb[1:1:3:1] | passed | 0.00017 seconds | -./spec/sentry/good_job/logger_spec.rb[1:2:1:1] | passed | 0.00013 seconds | -./spec/sentry/good_job/logger_spec.rb[1:2:2:1:1] | passed | 0.00022 seconds | -./spec/sentry/good_job/logger_spec.rb[1:2:2:2:1] | passed | 0.00014 seconds | -./spec/sentry/good_job/logger_spec.rb[1:3:1:1] | passed | 0.00021 seconds | -./spec/sentry/good_job/logger_spec.rb[1:3:2:1] | passed | 0.00016 seconds | -./spec/sentry/good_job/logger_spec.rb[1:4:1:1] | passed | 0.00021 seconds | -./spec/sentry/good_job/logger_spec.rb[1:4:2:1] | passed | 0.00015 seconds | -./spec/sentry/good_job/logger_spec.rb[1:5:1:1] | passed | 0.00022 seconds | -./spec/sentry/good_job/logger_spec.rb[1:5:2:1] | passed | 0.00016 seconds | -./spec/sentry/good_job_spec.rb[1:1] | passed | 0.00012 seconds | -./spec/sentry/good_job_spec.rb[1:2] | passed | 0.00011 seconds | -./spec/sentry/good_job_spec.rb[1:3:1] | passed | 0.00032 seconds | -./spec/sentry/good_job_spec.rb[1:3:2:1] | passed | 0.00031 seconds | -./spec/sentry/good_job_spec.rb[1:3:3:1] | passed | 0.0003 seconds | -./spec/sentry/good_job_spec.rb[1:4:1] | passed | 0.00017 seconds | -./spec/sentry/good_job_spec.rb[1:5:1] | passed | 0.00027 seconds | From 0c31fa102f648d378b9a22b4e7801444da4e5c30 Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Thu, 16 Oct 2025 14:35:40 +0300 Subject: [PATCH 03/24] Refactor job monitoring setup in sentry-good_job integration - Updated README.md to clarify the manual setup process for job monitoring in job classes and initializers. - Removed automatic setup of job monitoring for ApplicationJob from the integration logic in lib/sentry-good_job.rb. - Adjusted tests to reflect the change, ensuring that job monitoring is not automatically set up and can be manually configured instead. - Maintained backward compatibility by keeping the main entry point in lib/sentry/good_job.rb. --- sentry-good_job/README.md | 20 ++++++- sentry-good_job/lib/sentry-good_job.rb | 3 -- sentry-good_job/lib/sentry/good_job.rb | 55 +------------------- sentry-good_job/spec/sentry/good_job_spec.rb | 15 ++++-- 4 files changed, 30 insertions(+), 63 deletions(-) diff --git a/sentry-good_job/README.md b/sentry-good_job/README.md index 26e411526..614d727fb 100644 --- a/sentry-good_job/README.md +++ b/sentry-good_job/README.md @@ -80,11 +80,27 @@ The integration works automatically once installed. It will: ### Manual Job Monitoring -You can manually set up monitoring for specific job classes: +You need to manually set up monitoring for your job classes. This can be done in your job class or in an initializer: ```ruby class MyJob < ApplicationJob - # The integration will automatically set up monitoring + # Set up monitoring for this job class + Sentry::GoodJob::JobMonitor.setup_for_job_class(self) +end +``` + +Or set up monitoring for all your job classes in an initializer: + +```ruby +# config/initializers/sentry_good_job.rb +Rails.application.config.after_initialize do + # Set up monitoring for ApplicationJob if it exists + if defined?(ApplicationJob) + Sentry::GoodJob::JobMonitor.setup_for_job_class(ApplicationJob) + end + + # Or set up monitoring for specific job classes + # Sentry::GoodJob::JobMonitor.setup_for_job_class(MyJob) end ``` diff --git a/sentry-good_job/lib/sentry-good_job.rb b/sentry-good_job/lib/sentry-good_job.rb index 48e44798e..0e918e87d 100644 --- a/sentry-good_job/lib/sentry-good_job.rb +++ b/sentry-good_job/lib/sentry-good_job.rb @@ -37,9 +37,6 @@ def self.setup_good_job_integration # Sentry Rails integration already handles ActiveJob exceptions automatically # No need for custom error handling - # Set up unified job monitoring for ApplicationJob - Sentry::GoodJob::JobMonitor.setup_for_job_class(ApplicationJob) - # Set up cron monitoring for all scheduled jobs (automatically configured from Good Job config) if Sentry.configuration.good_job.auto_setup_cron_monitoring Sentry::GoodJob::CronMonitoring::Integration.setup_monitoring_for_scheduled_jobs diff --git a/sentry-good_job/lib/sentry/good_job.rb b/sentry-good_job/lib/sentry/good_job.rb index 3269b7f36..b1bbae0b3 100644 --- a/sentry-good_job/lib/sentry/good_job.rb +++ b/sentry-good_job/lib/sentry/good_job.rb @@ -1,55 +1,4 @@ # frozen_string_literal: true -require "sentry-ruby" -require "sentry/integrable" -require "sentry/good_job/version" -require "sentry/good_job/configuration" -require "sentry/good_job/error_handler" -require "sentry/good_job/logger" -require "sentry/good_job/job_monitor" -require "sentry/good_job/cron_monitoring" - -module Sentry - module GoodJob - extend Sentry::Integrable - - register_integration name: "good_job", version: Sentry::GoodJob::VERSION - - if defined?(::Rails::Railtie) - class Railtie < ::Rails::Railtie - config.after_initialize do - next unless Sentry.initialized? && defined?(::Sentry::Rails) - - # Skip ActiveJob error reporting when using Good Job as the backend - # since we handle it ourselves - Sentry.configuration.rails.skippable_job_adapters << "ActiveJob::QueueAdapters::GoodJobAdapter" - - # Automatic setup for Good Job when the integration is enabled - if Sentry.configuration.enabled_patches.include?(:good_job) - Sentry::GoodJob.setup_good_job_integration - end - end - end - end - - def self.setup_good_job_integration - # Sentry Rails integration already handles ActiveJob exceptions automatically - # No need for custom error handling - - # Set up unified job monitoring for ApplicationJob - Sentry::GoodJob::JobMonitor.setup_for_job_class(ApplicationJob) - - # Set up cron monitoring for all scheduled jobs (automatically configured from Good Job config) - if Sentry.configuration.good_job.auto_setup_cron_monitoring - Sentry::GoodJob::CronMonitoring::Integration.setup_monitoring_for_scheduled_jobs - end - - Sentry::GoodJob::Logger.info "Sentry Good Job integration initialized automatically" - end - - # Delegate capture_exception so internal components can be tested in isolation - def self.capture_exception(exception, **options) - ::Sentry.capture_exception(exception, **options) - end - end -end +# This file is kept for backward compatibility and simply requires the main entry point +require "sentry-good_job" diff --git a/sentry-good_job/spec/sentry/good_job_spec.rb b/sentry-good_job/spec/sentry/good_job_spec.rb index 139ebd50a..7a8d0d366 100644 --- a/sentry-good_job/spec/sentry/good_job_spec.rb +++ b/sentry-good_job/spec/sentry/good_job_spec.rb @@ -21,9 +21,6 @@ describe "setup_good_job_integration" do before do - # Mock ApplicationJob - stub_const("ApplicationJob", Class.new(ActiveJob::Base)) - # Mock Rails application configuration rails_app = double("Rails::Application") rails_config = double("Rails::Configuration") @@ -35,12 +32,20 @@ allow(good_job_config).to receive(:cron).and_return({}) end - it "sets up job monitoring for ApplicationJob" do - expect(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class).with(ApplicationJob) + it "does not automatically set up job monitoring for any specific job class" do + expect(Sentry::GoodJob::JobMonitor).not_to receive(:setup_for_job_class) described_class.setup_good_job_integration end + it "allows manual setup of job monitoring" do + job_class = Class.new(ActiveJob::Base) + + expect(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class).with(job_class) + + Sentry::GoodJob::JobMonitor.setup_for_job_class(job_class) + end + context "when auto_setup_cron_monitoring is enabled" do before do Sentry.configuration.good_job.auto_setup_cron_monitoring = true From fe90fb51e1f06d737da46128cf63077523aca5ac Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Thu, 16 Oct 2025 14:49:13 +0300 Subject: [PATCH 04/24] Refactor error handling in sentry-good_job integration - Simplified retry logic in the error handler by using a default retry attempt value instead of relying on job class configuration. - Updated the `retryable?` method to determine retryability based on job execution count rather than the presence of retry configuration. - Adjusted related tests to reflect changes in job class definitions and execution counts for better clarity and accuracy. --- .../lib/sentry/good_job/error_handler.rb | 8 ++++-- .../sentry/good_job/error_handler_spec.rb | 28 ++++++++----------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/sentry-good_job/lib/sentry/good_job/error_handler.rb b/sentry-good_job/lib/sentry/good_job/error_handler.rb index cf6040c8c..920bc607d 100644 --- a/sentry-good_job/lib/sentry/good_job/error_handler.rb +++ b/sentry-good_job/lib/sentry/good_job/error_handler.rb @@ -14,7 +14,8 @@ def call(ex, job) # Skip reporting if configured to only report after all retries are exhausted if Sentry.configuration.good_job.report_after_job_retries && retryable?(job) retry_count = job.executions - max_retries = job.class.retry_on_attempts || DEFAULT_RETRY_ATTEMPTS + # Use default retry attempts since we can't reliably access the configured value + max_retries = DEFAULT_RETRY_ATTEMPTS return if retry_count < max_retries end @@ -33,7 +34,10 @@ def call(ex, job) private def retryable?(job) - job.class.retry_on_attempts.present? && job.class.retry_on_attempts > 0 + # Since we can't reliably access retry configuration, we'll use a simpler approach: + # If the job has been executed more than once, it's likely retryable + # This is a conservative approach that works with the actual ActiveJob API + job.executions > 1 end def job_context(job) diff --git a/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb b/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb index 16f9c793e..41eaa4244 100644 --- a/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb +++ b/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb @@ -12,7 +12,7 @@ describe "#call" do let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 1, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } - let(:job_class) { double("JobClass", retry_on_attempts: nil, name: "TestJob") } + let(:job_class) { double("JobClass", name: "TestJob") } let(:exception) { build_exception } context "when Sentry is not initialized" do @@ -68,10 +68,9 @@ end context "and job is retryable" do - let(:job_class) { double("JobClass", retry_on_attempts: 3, name: "TestJob") } + let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 2, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } context "and retry count is less than max retries" do - let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 2, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } it "does not capture the exception" do expect(Sentry::GoodJob).not_to receive(:capture_exception) @@ -80,7 +79,7 @@ end context "and retry count equals max retries" do - let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 3, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } + let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 5, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } it "captures the exception" do expect(Sentry::GoodJob).to receive(:capture_exception) @@ -90,7 +89,7 @@ end context "and job is not retryable" do - let(:job_class) { double("JobClass", retry_on_attempts: nil, name: "TestJob") } + let(:job_class) { double("JobClass", name: "TestJob") } it "captures the exception" do expect(Sentry::GoodJob).to receive(:capture_exception) @@ -105,7 +104,7 @@ end context "and job is retryable" do - let(:job_class) { double("JobClass", retry_on_attempts: 3, name: "TestJob") } + let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 2, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } it "does not capture the exception" do expect(Sentry::GoodJob).not_to receive(:capture_exception) @@ -114,7 +113,7 @@ end context "and job is not retryable" do - let(:job_class) { double("JobClass", retry_on_attempts: nil, name: "TestJob") } + let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 1, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } it "captures the exception" do expect(Sentry::GoodJob).to receive(:capture_exception) @@ -126,23 +125,20 @@ end describe "#retryable?" do - it "returns true when job has retry_on_attempts" do - job_class = double("JobClass", retry_on_attempts: 3) - job = double("Job", class: job_class) + it "returns true when job has been executed more than once" do + job = double("Job", executions: 2) expect(handler.send(:retryable?, job)).to be true end - it "returns false when job has no retry_on_attempts" do - job_class = double("JobClass", retry_on_attempts: nil) - job = double("Job", class: job_class) + it "returns false when job has been executed only once" do + job = double("Job", executions: 1) expect(handler.send(:retryable?, job)).to be false end - it "returns false when job has retry_on_attempts of 0" do - job_class = double("JobClass", retry_on_attempts: 0) - job = double("Job", class: job_class) + it "returns false when job has not been executed" do + job = double("Job", executions: 0) expect(handler.send(:retryable?, job)).to be false end From 1dcd6bb1571950902bf8220b00ea446ed22a8e85 Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Thu, 16 Oct 2025 15:02:09 +0300 Subject: [PATCH 05/24] Enhance cron monitoring and error handling in sentry-good_job integration - Improved cron expression parsing to handle various timezone formats, including multi-level IANA timezones and UTC offsets. - Updated error handling logic to better determine when to report job failures based on retry counts, introducing a new method to check if retries have been exhausted. - Added comprehensive tests for new timezone handling and retry exhaustion logic to ensure robustness and accuracy. --- .../lib/sentry/good_job/cron_monitoring.rb | 19 ++++++--- .../lib/sentry/good_job/error_handler.rb | 38 +++++++++++++----- .../sentry/good_job/cron_monitoring_spec.rb | 40 ++++++++++++++++++- .../sentry/good_job/error_handler_spec.rb | 25 ++++++++++-- 4 files changed, 102 insertions(+), 20 deletions(-) diff --git a/sentry-good_job/lib/sentry/good_job/cron_monitoring.rb b/sentry-good_job/lib/sentry/good_job/cron_monitoring.rb index 8cee06eb2..d5685330d 100644 --- a/sentry-good_job/lib/sentry/good_job/cron_monitoring.rb +++ b/sentry-good_job/lib/sentry/good_job/cron_monitoring.rb @@ -11,14 +11,14 @@ module CronMonitoring module Helpers # Parse cron expression and create Sentry monitor config def self.monitor_config_from_cron(cron_expression, timezone: nil) - return nil unless cron_expression.present? + return nil unless cron_expression && !cron_expression.strip.empty? # Parse cron expression using fugit (same as Good Job) parsed_cron = Fugit.parse_cron(cron_expression) return nil unless parsed_cron # Convert to Sentry monitor config - if timezone.present? + if timezone && !timezone.strip.empty? ::Sentry::Cron::MonitorConfig.from_crontab(cron_expression, timezone: timezone) else ::Sentry::Cron::MonitorConfig.from_crontab(cron_expression) @@ -35,15 +35,24 @@ def self.monitor_slug(job_name) # Parse cron expression and extract timezone def self.parse_cron_with_timezone(cron_expression) - return [cron_expression, nil] unless cron_expression.present? + return [cron_expression, nil] unless cron_expression && !cron_expression.strip.empty? parts = cron_expression.strip.split(" ") return [cron_expression, nil] unless parts.length > 5 # Last part might be timezone timezone = parts.last - # Basic timezone validation (matches common timezone formats including Europe/Stockholm, America/New_York, etc.) - if timezone.match?(/^[A-Za-z_]+\/[A-Za-z_]+$/) || timezone.match?(/^[A-Za-z_]+$/) + # Comprehensive timezone validation that handles: + # - Standard timezone names (UTC, GMT) + # - IANA timezone identifiers (America/New_York, Europe/Stockholm) + # - Multi-level IANA timezones (America/Argentina/Buenos_Aires) + # - UTC offsets (UTC+2, UTC-5, GMT+1, GMT-8) + # - Numeric timezones (GMT-5, UTC+2) + if timezone.match?(/^[A-Za-z_]+$/) || # Simple timezone names (UTC, GMT, EST, etc.) + timezone.match?(/^[A-Za-z_]+\/[A-Za-z_]+$/) || # Single slash timezones (Europe/Stockholm) + timezone.match?(/^[A-Za-z_]+\/[A-Za-z_]+\/[A-Za-z_]+$/) || # Multi-slash timezones (America/Argentina/Buenos_Aires) + timezone.match?(/^[A-Za-z_]+[+-]\d+$/) || # UTC/GMT offsets (UTC+2, GMT-5) + timezone.match?(/^[A-Za-z_]+\/[A-Za-z_]+[+-]\d+$/) # IANA with offset (Europe/Stockholm+1) cron_without_timezone = cron_expression.gsub(/\s+#{Regexp.escape(timezone)}$/, "") [cron_without_timezone, timezone] else diff --git a/sentry-good_job/lib/sentry/good_job/error_handler.rb b/sentry-good_job/lib/sentry/good_job/error_handler.rb index 920bc607d..35183b04e 100644 --- a/sentry-good_job/lib/sentry/good_job/error_handler.rb +++ b/sentry-good_job/lib/sentry/good_job/error_handler.rb @@ -12,15 +12,22 @@ def call(ex, job) return unless Sentry.initialized? # Skip reporting if configured to only report after all retries are exhausted - if Sentry.configuration.good_job.report_after_job_retries && retryable?(job) - retry_count = job.executions - # Use default retry attempts since we can't reliably access the configured value - max_retries = DEFAULT_RETRY_ATTEMPTS - return if retry_count < max_retries + if Sentry.configuration.good_job.report_after_job_retries + if retryable?(job) + # For retryable jobs, only report after max retries are reached + return unless has_exhausted_retries?(job) + end + # For non-retryable jobs, report immediately (they're dead on first failure) end - # Skip reporting if configured to only report dead jobs and this job can be retried - return if Sentry.configuration.good_job.report_only_dead_jobs && retryable?(job) + # Skip reporting if configured to only report dead jobs + if Sentry.configuration.good_job.report_only_dead_jobs + if retryable?(job) + # For retryable jobs, never report (they're not dead yet) + return + end + # For non-retryable jobs, report immediately (they're dead on first failure) + end # Report to Sentry via GoodJob wrapper for testability # Context is already set by the job concern @@ -34,12 +41,23 @@ def call(ex, job) private def retryable?(job) - # Since we can't reliably access retry configuration, we'll use a simpler approach: - # If the job has been executed more than once, it's likely retryable - # This is a conservative approach that works with the actual ActiveJob API + # Determine if a job is likely retryable based on execution patterns + # This is a heuristic approach since we can't reliably access retry configuration + + # If the job has been executed multiple times, it's likely retryable + # This covers the common case where jobs are configured to retry job.executions > 1 end + def has_exhausted_retries?(job) + # Determine if a job has likely exhausted its retries + # This is a heuristic based on execution count and reasonable retry limits + + # If a job has been executed many times (more than typical retry limits), + # it's likely exhausted its retries and is now dead + job.executions > DEFAULT_RETRY_ATTEMPTS + end + def job_context(job) context = { job_class: job.class.name, diff --git a/sentry-good_job/spec/sentry/good_job/cron_monitoring_spec.rb b/sentry-good_job/spec/sentry/good_job/cron_monitoring_spec.rb index ec04459ea..413a03f02 100644 --- a/sentry-good_job/spec/sentry/good_job/cron_monitoring_spec.rb +++ b/sentry-good_job/spec/sentry/good_job/cron_monitoring_spec.rb @@ -72,8 +72,8 @@ end it "handles invalid timezone format" do - cron, timezone = described_class::Helpers.parse_cron_with_timezone("0 * * * * invalid-timezone") - expect(cron).to eq("0 * * * * invalid-timezone") + cron, timezone = described_class::Helpers.parse_cron_with_timezone("0 * * * * invalid@timezone") + expect(cron).to eq("0 * * * * invalid@timezone") expect(timezone).to be_nil end @@ -82,6 +82,42 @@ expect(cron).to eq("0 * * *") expect(timezone).to be_nil end + + it "handles multi-slash timezones" do + cron, timezone = described_class::Helpers.parse_cron_with_timezone("0 * * * * America/Argentina/Buenos_Aires") + expect(cron).to eq("0 * * * *") + expect(timezone).to eq("America/Argentina/Buenos_Aires") + end + + it "handles GMT offsets" do + cron, timezone = described_class::Helpers.parse_cron_with_timezone("0 * * * * GMT-5") + expect(cron).to eq("0 * * * *") + expect(timezone).to eq("GMT-5") + end + + it "handles UTC offsets" do + cron, timezone = described_class::Helpers.parse_cron_with_timezone("0 * * * * UTC+2") + expect(cron).to eq("0 * * * *") + expect(timezone).to eq("UTC+2") + end + + it "handles timezones with underscores" do + cron, timezone = described_class::Helpers.parse_cron_with_timezone("0 * * * * America/New_York") + expect(cron).to eq("0 * * * *") + expect(timezone).to eq("America/New_York") + end + + it "handles timezones with positive offsets" do + cron, timezone = described_class::Helpers.parse_cron_with_timezone("0 * * * * GMT+1") + expect(cron).to eq("0 * * * *") + expect(timezone).to eq("GMT+1") + end + + it "handles timezones with negative offsets" do + cron, timezone = described_class::Helpers.parse_cron_with_timezone("0 * * * * UTC-8") + expect(cron).to eq("0 * * * *") + expect(timezone).to eq("UTC-8") + end end end diff --git a/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb b/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb index 41eaa4244..a255f984a 100644 --- a/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb +++ b/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb @@ -71,15 +71,14 @@ let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 2, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } context "and retry count is less than max retries" do - it "does not capture the exception" do expect(Sentry::GoodJob).not_to receive(:capture_exception) handler.call(exception, job) end end - context "and retry count equals max retries" do - let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 5, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } + context "and retry count exceeds max retries" do + let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 6, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } it "captures the exception" do expect(Sentry::GoodJob).to receive(:capture_exception) @@ -144,6 +143,26 @@ end end + describe "#has_exhausted_retries?" do + it "returns true when job has been executed more than default retry attempts" do + job = double("Job", executions: 6) + + expect(handler.send(:has_exhausted_retries?, job)).to be true + end + + it "returns false when job has been executed less than default retry attempts" do + job = double("Job", executions: 3) + + expect(handler.send(:has_exhausted_retries?, job)).to be false + end + + it "returns false when job has been executed exactly default retry attempts" do + job = double("Job", executions: 5) + + expect(handler.send(:has_exhausted_retries?, job)).to be false + end + end + describe "#job_context" do let(:job) do double("Job", From 01fff5a1595e46d6b1ccc2523a64656d235ba327 Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Fri, 17 Oct 2025 08:27:41 +0300 Subject: [PATCH 06/24] Update configuration option terminology in sentry-good_job integration - Changed `report_only_dead_jobs` to `report_only_discarded_jobs` for clarity, indicating that it now reports errors for jobs that have been discarded (failed and cannot be retried). - Updated related documentation in CHANGELOG.md and README.md to reflect the new terminology. - Adjusted configuration and error handling logic to ensure consistency with the updated option name. - Modified tests to validate the new configuration behavior. --- sentry-good_job/CHANGELOG.md | 2 +- sentry-good_job/README.md | 4 ++-- sentry-good_job/lib/sentry/good_job/configuration.rb | 6 +++--- sentry-good_job/lib/sentry/good_job/error_handler.rb | 4 ++-- .../spec/sentry/good_job/configuration_spec.rb | 8 ++++---- .../spec/sentry/good_job/error_handler_spec.rb | 4 ++-- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/sentry-good_job/CHANGELOG.md b/sentry-good_job/CHANGELOG.md index 58ae53efa..f16b1c047 100644 --- a/sentry-good_job/CHANGELOG.md +++ b/sentry-good_job/CHANGELOG.md @@ -17,7 +17,7 @@ Individual gem's changelog has been deprecated. Please check the [project change ### Configuration Options - `report_after_job_retries`: Only report errors after all retry attempts are exhausted -- `report_only_dead_jobs`: Only report errors for jobs that cannot be retried +- `report_only_discarded_jobs`: Only report errors for jobs that have been discarded (failed and cannot be retried) - `propagate_traces`: Propagate trace headers for distributed tracing - `include_job_arguments`: Include job arguments in error context - `auto_setup_cron_monitoring`: Automatically set up cron monitoring for scheduled jobs diff --git a/sentry-good_job/README.md b/sentry-good_job/README.md index 614d727fb..ca22a8db6 100644 --- a/sentry-good_job/README.md +++ b/sentry-good_job/README.md @@ -49,7 +49,7 @@ Sentry.init do |config| # Good Job specific configuration config.good_job.report_after_job_retries = false - config.good_job.report_only_dead_jobs = false + config.good_job.report_only_discarded_jobs = false config.good_job.propagate_traces = true config.good_job.include_job_arguments = false config.good_job.auto_setup_cron_monitoring = true @@ -60,7 +60,7 @@ end ### Configuration Options - `report_after_job_retries` (default: `false`): Only report errors after all retry attempts are exhausted -- `report_only_dead_jobs` (default: `false`): Only report errors for jobs that cannot be retried +- `report_only_discarded_jobs` (default: `false`): Only report errors for jobs that have been discarded (failed and cannot be retried) - `propagate_traces` (default: `true`): Propagate trace headers for distributed tracing - `include_job_arguments` (default: `false`): Include job arguments in error context (be careful with sensitive data) - `auto_setup_cron_monitoring` (default: `true`): Automatically set up cron monitoring for scheduled jobs diff --git a/sentry-good_job/lib/sentry/good_job/configuration.rb b/sentry-good_job/lib/sentry/good_job/configuration.rb index 2a8287480..02f5dacdb 100644 --- a/sentry-good_job/lib/sentry/good_job/configuration.rb +++ b/sentry-good_job/lib/sentry/good_job/configuration.rb @@ -21,8 +21,8 @@ class Configuration # retry if it fails. attr_accessor :report_after_job_retries - # Only report jobs that have retry_on_attempts set (i.e., jobs that can be retried) - attr_accessor :report_only_dead_jobs + # Only report jobs that have been discarded (failed and cannot be retried) + attr_accessor :report_only_discarded_jobs # Whether we should inject headers while enqueuing the job in order to have a connected trace attr_accessor :propagate_traces @@ -41,7 +41,7 @@ class Configuration def initialize @report_after_job_retries = false - @report_only_dead_jobs = false + @report_only_discarded_jobs = false @propagate_traces = true @include_job_arguments = false @auto_setup_cron_monitoring = true diff --git a/sentry-good_job/lib/sentry/good_job/error_handler.rb b/sentry-good_job/lib/sentry/good_job/error_handler.rb index 35183b04e..b0fe9facf 100644 --- a/sentry-good_job/lib/sentry/good_job/error_handler.rb +++ b/sentry-good_job/lib/sentry/good_job/error_handler.rb @@ -20,8 +20,8 @@ def call(ex, job) # For non-retryable jobs, report immediately (they're dead on first failure) end - # Skip reporting if configured to only report dead jobs - if Sentry.configuration.good_job.report_only_dead_jobs + # Skip reporting if configured to only report discarded jobs + if Sentry.configuration.good_job.report_only_discarded_jobs if retryable?(job) # For retryable jobs, never report (they're not dead yet) return diff --git a/sentry-good_job/spec/sentry/good_job/configuration_spec.rb b/sentry-good_job/spec/sentry/good_job/configuration_spec.rb index 1a97b3226..6deddc66a 100644 --- a/sentry-good_job/spec/sentry/good_job/configuration_spec.rb +++ b/sentry-good_job/spec/sentry/good_job/configuration_spec.rb @@ -12,7 +12,7 @@ describe "default values" do it "sets default values correctly" do expect(config.report_after_job_retries).to eq(false) - expect(config.report_only_dead_jobs).to eq(false) + expect(config.report_only_discarded_jobs).to eq(false) expect(config.propagate_traces).to eq(true) expect(config.include_job_arguments).to eq(false) expect(config.auto_setup_cron_monitoring).to eq(true) @@ -36,9 +36,9 @@ expect(config.report_after_job_retries).to eq(true) end - it "allows setting report_only_dead_jobs" do - config.report_only_dead_jobs = true - expect(config.report_only_dead_jobs).to eq(true) + it "allows setting report_only_discarded_jobs" do + config.report_only_discarded_jobs = true + expect(config.report_only_discarded_jobs).to eq(true) end it "allows setting propagate_traces" do diff --git a/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb b/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb index a255f984a..e00d16d22 100644 --- a/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb +++ b/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb @@ -97,9 +97,9 @@ end end - context "when report_only_dead_jobs is enabled" do + context "when report_only_discarded_jobs is enabled" do before do - Sentry.configuration.good_job.report_only_dead_jobs = true + Sentry.configuration.good_job.report_only_discarded_jobs = true end context "and job is retryable" do From 8f673922d31cfad43221ffeae74a35e426f61f58 Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Fri, 17 Oct 2025 09:47:05 +0300 Subject: [PATCH 07/24] Refactor sentry-good_job integration to leverage sentry-rails for ActiveJob functionality - Removed deprecated configuration options related to job error reporting and tracing, now handled by sentry-rails. - Introduced GoodJob-specific context and tag enhancements for better integration with ActiveJob. - Updated documentation in CHANGELOG.md and README.md to reflect changes in configuration and functionality. - Implemented cron monitoring setup for scheduled jobs, improving the integration's capabilities. - Adjusted tests to validate the new behavior and ensure compatibility with the updated integration logic. --- sentry-good_job/CHANGELOG.md | 13 +- sentry-good_job/README.md | 70 +++--- sentry-good_job/example/app.rb | 6 +- sentry-good_job/lib/sentry-good_job.rb | 18 +- .../sentry/good_job/active_job_extensions.rb | 135 +++++++++++ .../lib/sentry/good_job/configuration.rb | 24 +- .../lib/sentry/good_job/context_helpers.rb | 89 ++++++++ .../{cron_monitoring.rb => cron_helpers.rb} | 92 +++++--- .../lib/sentry/good_job/error_handler.rb | 79 ------- .../lib/sentry/good_job/job_monitor.rb | 166 -------------- .../good_job/active_job_extensions_spec.rb | 142 ++++++++++++ .../sentry/good_job/configuration_spec.rb | 38 +--- .../sentry/good_job/context_helpers_spec.rb | 114 ++++++++++ ...onitoring_spec.rb => cron_helpers_spec.rb} | 45 ++-- .../sentry/good_job/error_handler_spec.rb | 214 ------------------ .../spec/sentry/good_job/job_monitor_spec.rb | 120 ---------- sentry-good_job/spec/sentry/good_job_spec.rb | 23 +- sentry-rails/lib/sentry/rails/active_job.rb | 108 ++++++++- 18 files changed, 734 insertions(+), 762 deletions(-) create mode 100644 sentry-good_job/lib/sentry/good_job/active_job_extensions.rb create mode 100644 sentry-good_job/lib/sentry/good_job/context_helpers.rb rename sentry-good_job/lib/sentry/good_job/{cron_monitoring.rb => cron_helpers.rb} (60%) delete mode 100644 sentry-good_job/lib/sentry/good_job/error_handler.rb delete mode 100644 sentry-good_job/lib/sentry/good_job/job_monitor.rb create mode 100644 sentry-good_job/spec/sentry/good_job/active_job_extensions_spec.rb create mode 100644 sentry-good_job/spec/sentry/good_job/context_helpers_spec.rb rename sentry-good_job/spec/sentry/good_job/{cron_monitoring_spec.rb => cron_helpers_spec.rb} (86%) delete mode 100644 sentry-good_job/spec/sentry/good_job/error_handler_spec.rb delete mode 100644 sentry-good_job/spec/sentry/good_job/job_monitor_spec.rb diff --git a/sentry-good_job/CHANGELOG.md b/sentry-good_job/CHANGELOG.md index f16b1c047..700fb23d0 100644 --- a/sentry-good_job/CHANGELOG.md +++ b/sentry-good_job/CHANGELOG.md @@ -16,14 +16,17 @@ Individual gem's changelog has been deprecated. Please check the [project change ### Configuration Options -- `report_after_job_retries`: Only report errors after all retry attempts are exhausted -- `report_only_discarded_jobs`: Only report errors for jobs that have been discarded (failed and cannot be retried) -- `propagate_traces`: Propagate trace headers for distributed tracing -- `include_job_arguments`: Include job arguments in error context -- `auto_setup_cron_monitoring`: Automatically set up cron monitoring for scheduled jobs +#### Good Job Specific Options +- `enable_cron_monitors`: Enable cron monitoring for scheduled jobs - `logging_enabled`: Enable logging for the Good Job integration - `logger`: Custom logger to use +#### ActiveJob Options (handled by sentry-rails) +- `config.rails.active_job_report_on_retry_error`: Only report errors after all retry attempts are exhausted +- `config.send_default_pii`: Include job arguments in error context + +**Note**: The Good Job integration now leverages sentry-rails for core ActiveJob functionality, including trace propagation, user context preservation, and error reporting. + ### Integration Features - Seamless integration with Rails applications diff --git a/sentry-good_job/README.md b/sentry-good_job/README.md index ca22a8db6..dbb7961ed 100644 --- a/sentry-good_job/README.md +++ b/sentry-good_job/README.md @@ -48,25 +48,30 @@ Sentry.init do |config| config.dsn = 'your-dsn-here' # Good Job specific configuration - config.good_job.report_after_job_retries = false - config.good_job.report_only_discarded_jobs = false - config.good_job.propagate_traces = true - config.good_job.include_job_arguments = false - config.good_job.auto_setup_cron_monitoring = true + config.good_job.enable_cron_monitors = true config.good_job.logging_enabled = false + + # ActiveJob configuration (handled by sentry-rails) + config.rails.active_job_report_on_retry_error = false + config.send_default_pii = false # Controls job arguments inclusion end ``` ### Configuration Options -- `report_after_job_retries` (default: `false`): Only report errors after all retry attempts are exhausted -- `report_only_discarded_jobs` (default: `false`): Only report errors for jobs that have been discarded (failed and cannot be retried) -- `propagate_traces` (default: `true`): Propagate trace headers for distributed tracing -- `include_job_arguments` (default: `false`): Include job arguments in error context (be careful with sensitive data) -- `auto_setup_cron_monitoring` (default: `true`): Automatically set up cron monitoring for scheduled jobs +#### Good Job Specific Options + +- `enable_cron_monitors` (default: `true`): Enable cron monitoring for scheduled jobs - `logging_enabled` (default: `false`): Enable logging for the Good Job integration - `logger` (default: `nil`): Custom logger to use (defaults to Rails.logger when available) +#### ActiveJob Options (handled by sentry-rails) + +- `config.rails.active_job_report_on_retry_error` (default: `false`): Only report errors after all retry attempts are exhausted +- `config.send_default_pii` (default: `false`): Include job arguments in error context (be careful with sensitive data) + +**Note**: The Good Job integration now leverages sentry-rails for core ActiveJob functionality, including trace propagation, user context preservation, and error reporting. This provides better integration and reduces duplication. + ## Usage ### Basic Setup @@ -78,31 +83,15 @@ The integration works automatically once installed. It will: 3. Automatically configure cron monitoring for scheduled jobs 4. Preserve user context and trace propagation -### Manual Job Monitoring +### Automatic Setup -You need to manually set up monitoring for your job classes. This can be done in your job class or in an initializer: - -```ruby -class MyJob < ApplicationJob - # Set up monitoring for this job class - Sentry::GoodJob::JobMonitor.setup_for_job_class(self) -end -``` - -Or set up monitoring for all your job classes in an initializer: +The integration works automatically once installed. It will: -```ruby -# config/initializers/sentry_good_job.rb -Rails.application.config.after_initialize do - # Set up monitoring for ApplicationJob if it exists - if defined?(ApplicationJob) - Sentry::GoodJob::JobMonitor.setup_for_job_class(ApplicationJob) - end - - # Or set up monitoring for specific job classes - # Sentry::GoodJob::JobMonitor.setup_for_job_class(MyJob) -end -``` +1. **Capture exceptions** from ActiveJob workers using sentry-rails +2. **Set up performance monitoring** for job execution with enhanced GoodJob-specific metrics +3. **Automatically configure cron monitoring** for scheduled jobs +4. **Preserve user context and trace propagation** across job executions +5. **Add GoodJob-specific context** including queue name, executions, priority, and latency ### Cron Monitoring @@ -135,7 +124,7 @@ class MyJob < ApplicationJob retry_on StandardError, wait: :exponentially_longer, attempts: 3 def perform - # This will only be reported to Sentry after 3 attempts if report_after_job_retries is true + # This will only be reported to Sentry after 3 attempts if active_job_report_on_retry_error is true raise "Something went wrong" end end @@ -160,16 +149,17 @@ This will output detailed information about: - Error capture and reporting decisions - Cron monitoring setup - Performance metrics collection -- Trace propagation +- GoodJob-specific context enhancement ## Performance Monitoring When performance monitoring is enabled, the integration will track: - Job execution time -- Queue latency +- Queue latency (GoodJob-specific) - Retry counts - Job context and metadata +- GoodJob-specific metrics (queue name, executions, priority) ## Error Context @@ -177,10 +167,12 @@ The integration automatically adds relevant context to error reports: - Job class name - Job ID -- Queue name -- Execution count +- Queue name (GoodJob-specific) +- Execution count (GoodJob-specific) +- Priority (GoodJob-specific) - Enqueued and scheduled timestamps -- Job arguments (if enabled) +- Job arguments (if enabled via send_default_pii) +- Latency metrics (GoodJob-specific) ## Compatibility diff --git a/sentry-good_job/example/app.rb b/sentry-good_job/example/app.rb index 4eab9e980..8e096d7e6 100644 --- a/sentry-good_job/example/app.rb +++ b/sentry-good_job/example/app.rb @@ -12,9 +12,11 @@ config.logger = Logger.new(STDOUT) # Good Job specific configuration - config.good_job.report_after_job_retries = false - config.good_job.include_job_arguments = true config.good_job.logging_enabled = true + + # ActiveJob configuration (handled by sentry-rails) + config.rails.active_job_report_on_retry_error = false + config.send_default_pii = true # Include job arguments end # Example job classes diff --git a/sentry-good_job/lib/sentry-good_job.rb b/sentry-good_job/lib/sentry-good_job.rb index 0e918e87d..c8108a3e8 100644 --- a/sentry-good_job/lib/sentry-good_job.rb +++ b/sentry-good_job/lib/sentry-good_job.rb @@ -5,10 +5,10 @@ require "sentry/integrable" require "sentry/good_job/version" require "sentry/good_job/configuration" -require "sentry/good_job/error_handler" require "sentry/good_job/logger" -require "sentry/good_job/job_monitor" -require "sentry/good_job/cron_monitoring" +require "sentry/good_job/context_helpers" +require "sentry/good_job/active_job_extensions" +require "sentry/good_job/cron_helpers" module Sentry module GoodJob @@ -21,10 +21,6 @@ class Railtie < ::Rails::Railtie config.after_initialize do next unless Sentry.initialized? && defined?(::Sentry::Rails) - # Skip ActiveJob error reporting when using Good Job as the backend - # since we handle it ourselves - Sentry.configuration.rails.skippable_job_adapters << "ActiveJob::QueueAdapters::GoodJobAdapter" - # Automatic setup for Good Job when the integration is enabled if Sentry.configuration.enabled_patches.include?(:good_job) Sentry::GoodJob.setup_good_job_integration @@ -34,12 +30,12 @@ class Railtie < ::Rails::Railtie end def self.setup_good_job_integration - # Sentry Rails integration already handles ActiveJob exceptions automatically - # No need for custom error handling + # Enhance sentry-rails ActiveJob integration with GoodJob-specific context + Sentry::GoodJob::ActiveJobExtensions.setup # Set up cron monitoring for all scheduled jobs (automatically configured from Good Job config) - if Sentry.configuration.good_job.auto_setup_cron_monitoring - Sentry::GoodJob::CronMonitoring::Integration.setup_monitoring_for_scheduled_jobs + if Sentry.configuration.good_job.enable_cron_monitors + Sentry::GoodJob::CronHelpers::Integration.setup_monitoring_for_scheduled_jobs end Sentry::GoodJob::Logger.info "Sentry Good Job integration initialized automatically" diff --git a/sentry-good_job/lib/sentry/good_job/active_job_extensions.rb b/sentry-good_job/lib/sentry/good_job/active_job_extensions.rb new file mode 100644 index 000000000..17c35d070 --- /dev/null +++ b/sentry-good_job/lib/sentry/good_job/active_job_extensions.rb @@ -0,0 +1,135 @@ +# frozen_string_literal: true + +# GoodJob-specific extensions to sentry-rails ActiveJob integration +# This module enhances sentry-rails ActiveJob with GoodJob-specific functionality: +# - GoodJob-specific context and tags +# - GoodJob-specific span data enhancements +module Sentry + module GoodJob + module ActiveJobExtensions + # Enhance sentry-rails ActiveJob context with GoodJob-specific data + def self.enhance_sentry_context(job, base_context) + return base_context unless job.respond_to?(:queue_name) && job.respond_to?(:executions) + + # Add GoodJob-specific context to the existing sentry-rails context + good_job_context = { + queue_name: job.queue_name, + executions: job.executions, + enqueued_at: job.enqueued_at, + priority: job.respond_to?(:priority) ? job.priority : nil + } + + # Merge with base context, preserving existing structure + base_context.merge(good_job: good_job_context) + end + + # Enhance sentry-rails ActiveJob tags with GoodJob-specific data + def self.enhance_sentry_tags(job, base_tags) + return base_tags unless job.respond_to?(:queue_name) && job.respond_to?(:executions) + + good_job_tags = { + queue_name: job.queue_name, + executions: job.executions + } + + # Add priority if available + if job.respond_to?(:priority) + good_job_tags[:priority] = job.priority + end + + base_tags.merge(good_job_tags) + end + + # Set up GoodJob-specific ActiveJob extensions + def self.setup + return unless defined?(::Rails) && ::Sentry.initialized? + + # Hook into sentry-rails ActiveJob integration + if defined?(::Sentry::Rails::ActiveJobExtensions::SentryReporter) + enhance_sentry_reporter + end + + # Set up GoodJob-specific ActiveJob extensions + setup_good_job_extensions + end + + private + + def self.enhance_sentry_reporter + # Enhance the sentry_context method in SentryReporter + ::Sentry::Rails::ActiveJobExtensions::SentryReporter.class_eval do + alias_method :original_sentry_context, :sentry_context + + def sentry_context(job) + base_context = original_sentry_context(job) + Sentry::GoodJob::ActiveJobExtensions.enhance_sentry_context(job, base_context) + end + end + end + + def self.setup_good_job_extensions + # Extend ActiveJob::Base with GoodJob-specific functionality + ActiveSupport.on_load(:active_job) do + # Add GoodJob-specific attributes and methods + include GoodJobExtensions + end + end + + # GoodJob-specific extensions for ActiveJob + module GoodJobExtensions + extend ActiveSupport::Concern + + included do + # Set up around_enqueue hook for GoodJob-specific enqueue span + around_enqueue do |job, block| + next block.call unless ::Sentry.initialized? + + # Create enqueue span with GoodJob-specific data + ::Sentry.with_child_span(op: "queue.publish", description: job.class.name) do |span| + set_span_data(span, job) + block.call + end + end + + # Set up around_perform hook for GoodJob-specific tags and span data + around_perform do |job, block| + next block.call unless ::Sentry.initialized? + + # Add GoodJob-specific tags to current scope + good_job_tags = { + queue_name: job.queue_name, + executions: job.executions + } + good_job_tags[:priority] = job.priority if job.respond_to?(:priority) + + Sentry.with_scope do |scope| + scope.set_tags(good_job_tags) + block.call + end + end + end + + private + + # Override set_span_data to add GoodJob-specific functionality + def set_span_data(span, job, retry_count: nil) + return unless span + + # Call the base implementation + super(span, job, retry_count: retry_count) + + # Add GoodJob-specific span data (latency) + latency = calculate_job_latency(job) + span.set_data("messaging.message.receive.latency", latency) if latency + end + + # Calculate job latency in milliseconds (GoodJob-specific) + def calculate_job_latency(job) + return nil unless job.enqueued_at + + ((Time.now.utc - job.enqueued_at) * 1000).to_i + end + end + end + end +end diff --git a/sentry-good_job/lib/sentry/good_job/configuration.rb b/sentry-good_job/lib/sentry/good_job/configuration.rb index 02f5dacdb..976bd9db2 100644 --- a/sentry-good_job/lib/sentry/good_job/configuration.rb +++ b/sentry-good_job/lib/sentry/good_job/configuration.rb @@ -17,21 +17,9 @@ module GoodJob ] class Configuration - # Set this option to true if you want Sentry to only capture the last job - # retry if it fails. - attr_accessor :report_after_job_retries - - # Only report jobs that have been discarded (failed and cannot be retried) - attr_accessor :report_only_discarded_jobs - - # Whether we should inject headers while enqueuing the job in order to have a connected trace - attr_accessor :propagate_traces - - # Whether to include job arguments in error context (be careful with sensitive data) - attr_accessor :include_job_arguments - - # Whether to automatically set up cron monitoring for all scheduled jobs - attr_accessor :auto_setup_cron_monitoring + # Whether to enable cron monitoring for all scheduled jobs + # This is GoodJob-specific functionality for monitoring scheduled tasks + attr_accessor :enable_cron_monitors # When false, suppresses all Sentry GoodJob integration logs attr_accessor :logging_enabled @@ -40,11 +28,7 @@ class Configuration attr_accessor :logger def initialize - @report_after_job_retries = false - @report_only_discarded_jobs = false - @propagate_traces = true - @include_job_arguments = false - @auto_setup_cron_monitoring = true + @enable_cron_monitors = true @logging_enabled = false @logger = nil end diff --git a/sentry-good_job/lib/sentry/good_job/context_helpers.rb b/sentry-good_job/lib/sentry/good_job/context_helpers.rb new file mode 100644 index 000000000..9bc1d7239 --- /dev/null +++ b/sentry-good_job/lib/sentry/good_job/context_helpers.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +# Helper methods for adding GoodJob-specific information to Sentry context +# This works WITH sentry-rails, not against it +module Sentry + module GoodJob + module ContextHelpers + # Add GoodJob-specific information to the existing Sentry Rails context + def self.add_context(job, base_context = {}) + return base_context unless job.respond_to?(:queue_name) && job.respond_to?(:executions) + + good_job_context = { + queue_name: job.queue_name, + executions: job.executions, + enqueued_at: job.enqueued_at, + priority: job.respond_to?(:priority) ? job.priority : nil + } + + # Note: Job arguments are handled by sentry-rails via send_default_pii configuration + # This is controlled by Sentry.configuration.send_default_pii, not GoodJob-specific config + + # Merge with base context + base_context.merge(good_job: good_job_context) + end + + # Add GoodJob-specific information to the existing Sentry Rails tags + def self.add_tags(job, base_tags = {}) + return base_tags unless job.respond_to?(:queue_name) && job.respond_to?(:executions) + + good_job_tags = { + queue_name: job.queue_name, + executions: job.executions + } + + # Add priority if available + if job.respond_to?(:priority) + good_job_tags[:priority] = job.priority + end + + base_tags.merge(good_job_tags) + end + + # Enhanced context that includes both ActiveJob and GoodJob-specific data + def self.enhanced_context(job) + # Start with sentry-rails ActiveJob context + active_job_context = { + active_job: job.class.name, + arguments: job.respond_to?(:arguments) ? job.arguments.map(&:inspect) : [], + scheduled_at: job.scheduled_at, + job_id: job.job_id, + provider_job_id: job.provider_job_id, + locale: job.locale + } + + # Add GoodJob-specific context + good_job_context = { + queue_name: job.queue_name, + executions: job.executions, + enqueued_at: job.enqueued_at, + priority: job.respond_to?(:priority) ? job.priority : nil + } + + { + active_job: active_job_context, + good_job: good_job_context + } + end + + # Enhanced tags that include both ActiveJob and GoodJob-specific data + def self.enhanced_tags(job) + base_tags = { + job_id: job.job_id, + provider_job_id: job.provider_job_id + } + + good_job_tags = { + queue_name: job.queue_name, + executions: job.executions + } + + if job.respond_to?(:priority) + good_job_tags[:priority] = job.priority + end + + base_tags.merge(good_job_tags) + end + end + end +end diff --git a/sentry-good_job/lib/sentry/good_job/cron_monitoring.rb b/sentry-good_job/lib/sentry/good_job/cron_helpers.rb similarity index 60% rename from sentry-good_job/lib/sentry/good_job/cron_monitoring.rb rename to sentry-good_job/lib/sentry/good_job/cron_helpers.rb index d5685330d..936528e1c 100644 --- a/sentry-good_job/lib/sentry/good_job/cron_monitoring.rb +++ b/sentry-good_job/lib/sentry/good_job/cron_helpers.rb @@ -1,11 +1,12 @@ # frozen_string_literal: true -# Sentry Cron Monitoring for Good Job -# This module provides comprehensive cron monitoring for Good Job scheduled tasks -# Following Good Job's extension patterns and Sentry's integration guidelines +# Sentry Cron Monitoring for Active Job +# This module provides comprehensive cron monitoring for Active Job scheduled tasks +# It works with any Active Job adapter, including GoodJob +# Following Active Job's extension patterns and Sentry's integration guidelines module Sentry module GoodJob - module CronMonitoring + module CronHelpers # Utility methods for cron parsing and configuration # These methods handle the conversion between Good Job cron expressions and Sentry monitor configs module Helpers @@ -67,57 +68,73 @@ class Integration # Set up monitoring for all scheduled jobs from Good Job configuration def self.setup_monitoring_for_scheduled_jobs return unless ::Sentry.initialized? - return unless ::Sentry.configuration.good_job.auto_setup_cron_monitoring + return unless ::Sentry.configuration.good_job.enable_cron_monitors cron_config = ::Rails.application.config.good_job.cron return unless cron_config.present? - cron_config.each do |job_name, job_config| - setup_monitoring_for_job(job_name, job_config) + cron_config.each do |cron_key, job_config| + setup_monitoring_for_job(cron_key, job_config) end Sentry::GoodJob::Logger.info "Sentry cron monitoring setup for #{cron_config.keys.size} scheduled jobs" end # Set up monitoring for a specific job - def self.setup_monitoring_for_job(job_name, job_config) + def self.setup_monitoring_for_job(cron_key, job_config) job_class_name = job_config[:class] cron_expression = job_config[:cron] return unless job_class_name && cron_expression - # Get the job class - job_class = begin - job_class_name.constantize - rescue NameError => e - Sentry::GoodJob::Logger.warn "Could not find job class '#{job_class_name}' for Sentry cron monitoring: #{e.message}" - return - end - - # Set up job monitoring if not already set up - Sentry::GoodJob::JobMonitor.setup_for_job_class(job_class) - - # Parse cron expression and create monitor config - cron_without_tz, timezone = Sentry::GoodJob::CronMonitoring::Helpers.parse_cron_with_timezone(cron_expression) - monitor_config = Sentry::GoodJob::CronMonitoring::Helpers.monitor_config_from_cron(cron_without_tz, timezone: timezone) - - if monitor_config - # Configure Sentry cron monitoring - use job_name as slug for consistency - monitor_slug = Sentry::GoodJob::CronMonitoring::Helpers.monitor_slug(job_name) + # Defer job class constantization to avoid boot-time issues + # The job class will be constantized when the job is actually executed + # This prevents issues during development boot and circular dependencies + + # Store the monitoring configuration for later use + # We'll set up the monitoring when the job class is first loaded + deferred_setup = lambda do + job_class = begin + job_class_name.constantize + rescue NameError => e + Sentry::GoodJob::Logger.warn "Could not find job class '#{job_class_name}' for Sentry cron monitoring: #{e.message}" + return + end + # Include Sentry::Cron::MonitorCheckIns module for cron monitoring # only patch if not explicitly included in job by user unless job_class.ancestors.include?(Sentry::Cron::MonitorCheckIns) job_class.include(Sentry::Cron::MonitorCheckIns) end - job_class.sentry_monitor_check_ins( - slug: monitor_slug, - monitor_config: monitor_config - ) + # Parse cron expression and create monitor config + cron_without_tz, timezone = Sentry::GoodJob::CronHelpers::Helpers.parse_cron_with_timezone(cron_expression) + monitor_config = Sentry::GoodJob::CronHelpers::Helpers.monitor_config_from_cron(cron_without_tz, timezone: timezone) + + if monitor_config + # Configure Sentry cron monitoring - use cron_key as slug for consistency + monitor_slug = Sentry::GoodJob::CronHelpers::Helpers.monitor_slug(cron_key) + + job_class.sentry_monitor_check_ins( + slug: monitor_slug, + monitor_config: monitor_config + ) + + Sentry::GoodJob::Logger.info "Added Sentry cron monitoring for #{job_class_name} (#{monitor_slug})" + else + Sentry::GoodJob::Logger.warn "Could not create monitor config for #{job_class_name} with cron '#{cron_expression}'" + end + end - Sentry::GoodJob::Logger.info "Added Sentry cron monitoring for #{job_class_name} (#{monitor_slug})" + # Set up monitoring when the job class is first loaded + # This defers constantization until the job is actually needed + if defined?(Rails) && Rails.application + Rails.application.config.after_initialize do + deferred_setup.call + end else - Sentry::GoodJob::Logger.warn "Could not create monitor config for #{job_class_name} with cron '#{cron_expression}'" + # Fallback for non-Rails environments + deferred_setup.call end end @@ -125,19 +142,22 @@ def self.setup_monitoring_for_job(job_name, job_config) def self.add_monitoring_to_job(job_class, slug: nil, cron_expression: nil, timezone: nil) return unless ::Sentry.initialized? - # Set up job monitoring if not already set up - Sentry::GoodJob::JobMonitor.setup_for_job_class(job_class) + # Include Sentry::Cron::MonitorCheckIns module for cron monitoring + # only patch if not explicitly included in job by user + unless job_class.ancestors.include?(Sentry::Cron::MonitorCheckIns) + job_class.include(Sentry::Cron::MonitorCheckIns) + end # Create monitor config monitor_config = if cron_expression - Sentry::GoodJob::CronMonitoring::Helpers.monitor_config_from_cron(cron_expression, timezone: timezone) + Sentry::GoodJob::CronHelpers::Helpers.monitor_config_from_cron(cron_expression, timezone: timezone) else # Default to hourly monitoring if no cron expression provided ::Sentry::Cron::MonitorConfig.from_crontab("0 * * * *") end if monitor_config - monitor_slug = slug || Sentry::GoodJob::CronMonitoring::Helpers.monitor_slug(job_class.name) + monitor_slug = slug || Sentry::GoodJob::CronHelpers::Helpers.monitor_slug(job_class.name) # only patch if not explicitly included in job by user unless job_class.ancestors.include?(Sentry::Cron::MonitorCheckIns) diff --git a/sentry-good_job/lib/sentry/good_job/error_handler.rb b/sentry-good_job/lib/sentry/good_job/error_handler.rb deleted file mode 100644 index b0fe9facf..000000000 --- a/sentry-good_job/lib/sentry/good_job/error_handler.rb +++ /dev/null @@ -1,79 +0,0 @@ -# frozen_string_literal: true - -module Sentry - module GoodJob - class ErrorHandler - # Default retry attempts for ActiveJob (matches ActiveJob::Base.retry_on default) - DEFAULT_RETRY_ATTEMPTS = 5 - - # @param ex [Exception] the exception / error that occurred - # @param job [ActiveJob::Base] the job instance that failed - def call(ex, job) - return unless Sentry.initialized? - - # Skip reporting if configured to only report after all retries are exhausted - if Sentry.configuration.good_job.report_after_job_retries - if retryable?(job) - # For retryable jobs, only report after max retries are reached - return unless has_exhausted_retries?(job) - end - # For non-retryable jobs, report immediately (they're dead on first failure) - end - - # Skip reporting if configured to only report discarded jobs - if Sentry.configuration.good_job.report_only_discarded_jobs - if retryable?(job) - # For retryable jobs, never report (they're not dead yet) - return - end - # For non-retryable jobs, report immediately (they're dead on first failure) - end - - # Report to Sentry via GoodJob wrapper for testability - # Context is already set by the job concern - Sentry::GoodJob.capture_exception( - ex, - contexts: { good_job: job_context(job) }, - hint: { background: true } - ) - end - - private - - def retryable?(job) - # Determine if a job is likely retryable based on execution patterns - # This is a heuristic approach since we can't reliably access retry configuration - - # If the job has been executed multiple times, it's likely retryable - # This covers the common case where jobs are configured to retry - job.executions > 1 - end - - def has_exhausted_retries?(job) - # Determine if a job has likely exhausted its retries - # This is a heuristic based on execution count and reasonable retry limits - - # If a job has been executed many times (more than typical retry limits), - # it's likely exhausted its retries and is now dead - job.executions > DEFAULT_RETRY_ATTEMPTS - end - - def job_context(job) - context = { - job_class: job.class.name, - job_id: job.job_id, - queue_name: job.queue_name, - executions: job.executions, - enqueued_at: job.enqueued_at, - scheduled_at: job.scheduled_at - } - - if Sentry.configuration.good_job.include_job_arguments - context[:arguments] = job.arguments.map(&:inspect) - end - - context - end - end - end -end diff --git a/sentry-good_job/lib/sentry/good_job/job_monitor.rb b/sentry-good_job/lib/sentry/good_job/job_monitor.rb deleted file mode 100644 index 66c8791b9..000000000 --- a/sentry-good_job/lib/sentry/good_job/job_monitor.rb +++ /dev/null @@ -1,166 +0,0 @@ -# frozen_string_literal: true - -# Unified job monitoring for Sentry GoodJob integration -# Combines context setting, tracing, and cron monitoring in a single class -module Sentry - module GoodJob - class JobMonitor - def self.setup_for_job_class(job_class) - return unless defined?(::Rails) && ::Sentry.initialized? - - # Include Sentry::Cron::MonitorCheckIns module for cron monitoring - # only patch if not explicitly included in job by user - unless job_class.ancestors.include?(Sentry::Cron::MonitorCheckIns) - job_class.include(Sentry::Cron::MonitorCheckIns) - end - - # Add Sentry context attributes - job_class.attr_accessor :_sentry - - # Set up around_enqueue hook (only if not already set up) - unless job_class.respond_to?(:sentry_enqueue_hook_setup) - job_class.define_singleton_method(:sentry_enqueue_hook_setup) { true } - - job_class.around_enqueue do |job, block| - next block.call unless ::Sentry.initialized? - - ::Sentry.with_child_span(op: "queue.publish", description: job.class.to_s) do |span| - _sentry_set_span_data(span, id: job.job_id, queue: job.queue_name) - block.call - end - end - end - - # Set up around_perform hook with unified functionality (only if not already set up) - unless job_class.respond_to?(:sentry_perform_hook_setup) - job_class.define_singleton_method(:sentry_perform_hook_setup) { true } - - job_class.around_perform do |job, block| - next block.call unless ::Sentry.initialized? - - # Set up Sentry context - ::Sentry.clone_hub_to_current_thread - scope = ::Sentry.get_current_scope - if (user = job._sentry&.dig("user")) - scope.set_user(user) - end - scope.set_tags(queue: job.queue_name, job_id: job.job_id) - scope.set_contexts(active_job: _sentry_job_context(job)) - scope.set_transaction_name(job.class.name, source: :task) - transaction = _sentry_start_transaction(scope, job._sentry&.dig("trace_propagation_headers")) - - if transaction - scope.set_span(transaction) - - latency = ((Time.now.utc - job.enqueued_at) * 1000).to_i if job.enqueued_at - retry_count = job.executions.is_a?(Integer) ? job.executions - 1 : 0 - - _sentry_set_span_data( - transaction, - id: job.job_id, - queue: job.queue_name, - latency: latency, - retry_count: retry_count - ) - end - - begin - block.call - _sentry_finish_transaction(transaction, 200) - rescue => error - _sentry_finish_transaction(transaction, 500) - raise - end - end - end - - # Add convenience method for cron monitoring configuration - job_class.define_singleton_method(:sentry_cron_monitor) do |cron_expression, timezone: nil, slug: nil| - return unless ::Sentry.initialized? - - # Create monitor config - monitor_config = Sentry::GoodJob::CronMonitoring::Helpers.monitor_config_from_cron(cron_expression, timezone: timezone) - return unless monitor_config - - # Use provided slug or generate from class name - monitor_slug = slug || Sentry::GoodJob::CronMonitoring::Helpers.monitor_slug(name) - - sentry_monitor_check_ins(slug: monitor_slug, monitor_config: monitor_config) - end - - # Add instance methods for Sentry context - job_class.define_method(:enqueue) do |options = {}| - self._sentry ||= {} - - user = ::Sentry.get_current_scope&.user - self._sentry["user"] = user if user.present? - - self._sentry["trace_propagation_headers"] = ::Sentry.get_trace_propagation_headers - - super(options) - end - - job_class.define_method(:serialize) do - begin - super().tap do |job_data| - if _sentry - job_data["_sentry"] = _sentry.to_json - end - end - rescue JSON::GeneratorError, TypeError - # Swallow JSON serialization errors. Better to lose Sentry context than fail to serialize the job. - super() - end - end - - job_class.define_method(:deserialize) do |job_data| - super(job_data) - - begin - self._sentry = JSON.parse(job_data["_sentry"]) if job_data["_sentry"] - rescue JSON::ParserError - # Swallow JSON parsing errors. Better to lose Sentry context than fail to deserialize the job. - end - end - - # Add private helper methods - job_class.define_method(:_sentry_set_span_data) do |span, id:, queue:, latency: nil, retry_count: nil| - if span - span.set_data("messaging.message.id", id) - span.set_data("messaging.destination.name", queue) - span.set_data("messaging.message.receive.latency", latency) if latency - span.set_data("messaging.message.retry.count", retry_count) if retry_count - end - end - - job_class.define_method(:_sentry_job_context) do |job| - job.serialize.symbolize_keys.except(:arguments, :_sentry) - end - - job_class.define_method(:_sentry_start_transaction) do |scope, env| - options = { - name: scope.transaction_name, - source: scope.transaction_source, - op: "queue.process", - origin: "auto.queue.active_job" - } - - transaction = ::Sentry.continue_trace(env, **options) - ::Sentry.start_transaction(transaction: transaction, **options) - end - - job_class.define_method(:_sentry_finish_transaction) do |transaction, status| - return unless transaction - - transaction.set_http_status(status) - transaction.finish - end - - # Make helper methods private - job_class.class_eval do - private :_sentry_set_span_data, :_sentry_job_context, :_sentry_start_transaction, :_sentry_finish_transaction - end - end - end - end -end diff --git a/sentry-good_job/spec/sentry/good_job/active_job_extensions_spec.rb b/sentry-good_job/spec/sentry/good_job/active_job_extensions_spec.rb new file mode 100644 index 000000000..8aacc3f9f --- /dev/null +++ b/sentry-good_job/spec/sentry/good_job/active_job_extensions_spec.rb @@ -0,0 +1,142 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Sentry::GoodJob::ActiveJobExtensions do + let(:job) do + double("Job", + class: double("JobClass", name: "TestJob"), + job_id: "123", + provider_job_id: "provider_123", + queue_name: "default", + executions: 2, + enqueued_at: Time.now, + scheduled_at: Time.now + 1.hour, + priority: 5, + arguments: ["arg1", "arg2"], + locale: "en" + ) + end + + let(:base_context) { { active_job: "TestJob" } } + let(:base_tags) { { job_id: "123" } } + + before do + perform_basic_setup + end + + describe ".enhance_sentry_context" do + it "enhances sentry context with GoodJob-specific data" do + result = described_class.enhance_sentry_context(job, base_context) + + expect(result[:good_job]).to include( + queue_name: "default", + executions: 2, + enqueued_at: job.enqueued_at, + priority: 5 + ) + end + + it "preserves base context" do + result = described_class.enhance_sentry_context(job, base_context) + + expect(result[:active_job]).to eq("TestJob") + end + + it "returns base context when job doesn't respond to required methods" do + incomplete_job = double("IncompleteJob", job_id: "123") + result = described_class.enhance_sentry_context(incomplete_job, base_context) + + expect(result).to eq(base_context) + end + end + + describe ".enhance_sentry_tags" do + it "enhances sentry tags with GoodJob-specific data" do + result = described_class.enhance_sentry_tags(job, base_tags) + + expect(result).to include( + job_id: "123", + queue_name: "default", + executions: 2, + priority: 5 + ) + end + + it "preserves base tags" do + result = described_class.enhance_sentry_tags(job, base_tags) + + expect(result[:job_id]).to eq("123") + end + + it "returns base tags when job doesn't respond to required methods" do + incomplete_job = double("IncompleteJob", job_id: "123") + result = described_class.enhance_sentry_tags(incomplete_job, base_tags) + + expect(result).to eq(base_tags) + end + end + + describe ".setup" do + it "sets up GoodJob-specific ActiveJob extensions" do + # Mock Rails and Sentry to ensure conditions are met + stub_const("Rails", double("Rails")) + allow(Sentry).to receive(:initialized?).and_return(true) + + # Mock SentryReporter to be defined + stub_const("Sentry::Rails::ActiveJobExtensions::SentryReporter", Class.new) + + # Mock ActiveSupport.on_load to test it's called + allow(ActiveSupport).to receive(:on_load).with(:active_job).and_yield + + # Mock the private methods to test they are called + allow(described_class).to receive(:enhance_sentry_reporter) + allow(described_class).to receive(:setup_good_job_extensions) + + described_class.setup + + expect(described_class).to have_received(:enhance_sentry_reporter) + expect(described_class).to have_received(:setup_good_job_extensions) + end + end + + describe "GoodJobExtensions" do + let(:job_class) do + Class.new(ActiveJob::Base) do + include Sentry::GoodJob::ActiveJobExtensions::GoodJobExtensions + + def self.name + "TestJob" + end + + def perform + "test" + end + end + end + + let(:job) { job_class.new } + + before do + perform_basic_setup + end + + describe "around_enqueue" do + it "creates child span for enqueue with GoodJob-specific data" do + expect(Sentry).to receive(:with_child_span).with(op: "queue.publish", description: "TestJob") + job.enqueue + end + end + + describe "around_perform" do + it "adds GoodJob-specific tags during perform" do + scope = double("Scope") + allow(scope).to receive(:set_tags) + allow(scope).to receive(:span).and_return(nil) + + expect(Sentry).to receive(:with_scope).and_yield(scope) + job.perform_now + end + end + end +end diff --git a/sentry-good_job/spec/sentry/good_job/configuration_spec.rb b/sentry-good_job/spec/sentry/good_job/configuration_spec.rb index 6deddc66a..a1bc0bfd0 100644 --- a/sentry-good_job/spec/sentry/good_job/configuration_spec.rb +++ b/sentry-good_job/spec/sentry/good_job/configuration_spec.rb @@ -11,11 +11,7 @@ describe "default values" do it "sets default values correctly" do - expect(config.report_after_job_retries).to eq(false) - expect(config.report_only_discarded_jobs).to eq(false) - expect(config.propagate_traces).to eq(true) - expect(config.include_job_arguments).to eq(false) - expect(config.auto_setup_cron_monitoring).to eq(true) + expect(config.enable_cron_monitors).to eq(true) expect(config.logging_enabled).to eq(false) expect(config.logger).to be_nil end @@ -31,29 +27,15 @@ end describe "configuration attributes" do - it "allows setting report_after_job_retries" do - config.report_after_job_retries = true - expect(config.report_after_job_retries).to eq(true) - end - - it "allows setting report_only_discarded_jobs" do - config.report_only_discarded_jobs = true - expect(config.report_only_discarded_jobs).to eq(true) - end - - it "allows setting propagate_traces" do - config.propagate_traces = false - expect(config.propagate_traces).to eq(false) - end - - it "allows setting include_job_arguments" do - config.include_job_arguments = true - expect(config.include_job_arguments).to eq(true) - end - - it "allows setting auto_setup_cron_monitoring" do - config.auto_setup_cron_monitoring = false - expect(config.auto_setup_cron_monitoring).to eq(false) + # Removed configuration options that are now handled by sentry-rails: + # - report_after_job_retries (use sentry-rails active_job_report_on_retry_error) + # - report_only_discarded_jobs (handled by ActiveJob retry/discard logic) + # - propagate_traces (handled by sentry-rails) + # - include_job_arguments (use sentry-rails send_default_pii) + + it "allows setting enable_cron_monitors" do + config.enable_cron_monitors = false + expect(config.enable_cron_monitors).to eq(false) end it "allows setting logging_enabled" do diff --git a/sentry-good_job/spec/sentry/good_job/context_helpers_spec.rb b/sentry-good_job/spec/sentry/good_job/context_helpers_spec.rb new file mode 100644 index 000000000..1023d6e35 --- /dev/null +++ b/sentry-good_job/spec/sentry/good_job/context_helpers_spec.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Sentry::GoodJob::ContextHelpers do + let(:job) do + double("Job", + class: double("JobClass", name: "TestJob"), + job_id: "123", + provider_job_id: "provider_123", + queue_name: "default", + executions: 2, + enqueued_at: Time.now, + scheduled_at: Time.now + 1.hour, + priority: 5, + arguments: ["arg1", "arg2"], + locale: "en" + ) + end + + let(:incomplete_job) do + double("IncompleteJob", + class: double("JobClass", name: "TestJob"), + job_id: "123" + ) + end + + describe ".add_context" do + let(:base_context) { { existing: "context" } } + + it "adds GoodJob-specific context" do + result = described_class.add_context(job, base_context) + + expect(result[:good_job]).to include( + queue_name: "default", + executions: 2, + enqueued_at: job.enqueued_at, + priority: 5 + ) + end + + it "preserves base context" do + result = described_class.add_context(job, base_context) + + expect(result[:existing]).to eq("context") + end + + it "when job doesn't respond to required methods" do + result = described_class.add_context(incomplete_job, base_context) + + expect(result).to eq(base_context) + end + end + + describe ".add_tags" do + let(:base_tags) { { existing: "tag" } } + + it "adds GoodJob-specific tags" do + result = described_class.add_tags(job, base_tags) + + expect(result).to include( + queue_name: "default", + executions: 2, + priority: 5 + ) + end + + it "preserves base tags" do + result = described_class.add_tags(job, base_tags) + + expect(result[:existing]).to eq("tag") + end + + it "when job doesn't respond to required methods" do + result = described_class.add_tags(incomplete_job, base_tags) + + expect(result).to eq(base_tags) + end + end + + describe ".enhanced_context" do + it "returns enhanced context with both ActiveJob and GoodJob data" do + result = described_class.enhanced_context(job) + + expect(result[:active_job]).to include( + active_job: "TestJob", + job_id: "123", + provider_job_id: "provider_123", + locale: "en" + ) + + expect(result[:good_job]).to include( + queue_name: "default", + executions: 2, + enqueued_at: job.enqueued_at, + priority: 5 + ) + end + end + + describe ".enhanced_tags" do + it "returns enhanced tags with both ActiveJob and GoodJob data" do + result = described_class.enhanced_tags(job) + + expect(result).to include( + job_id: "123", + provider_job_id: "provider_123", + queue_name: "default", + executions: 2, + priority: 5 + ) + end + end +end diff --git a/sentry-good_job/spec/sentry/good_job/cron_monitoring_spec.rb b/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb similarity index 86% rename from sentry-good_job/spec/sentry/good_job/cron_monitoring_spec.rb rename to sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb index 413a03f02..3efa35707 100644 --- a/sentry-good_job/spec/sentry/good_job/cron_monitoring_spec.rb +++ b/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" -RSpec.describe Sentry::GoodJob::CronMonitoring do +RSpec.describe Sentry::GoodJob::CronHelpers do before do perform_basic_setup end @@ -145,10 +145,10 @@ end end - context "when auto_setup_cron_monitoring is disabled" do + context "when enable_cron_monitors is disabled" do before do allow(Sentry).to receive(:initialized?).and_return(true) - Sentry.configuration.good_job.auto_setup_cron_monitoring = false + Sentry.configuration.good_job.enable_cron_monitors = false end it "does not set up monitoring" do @@ -160,7 +160,7 @@ context "when cron config is not present" do before do allow(Sentry).to receive(:initialized?).and_return(true) - Sentry.configuration.good_job.auto_setup_cron_monitoring = true + Sentry.configuration.good_job.enable_cron_monitors = true allow(good_job_config).to receive(:cron).and_return(nil) end @@ -180,7 +180,7 @@ before do allow(Sentry).to receive(:initialized?).and_return(true) - Sentry.configuration.good_job.auto_setup_cron_monitoring = true + Sentry.configuration.good_job.enable_cron_monitors = true allow(good_job_config).to receive(:cron).and_return(cron_config) end @@ -215,6 +215,8 @@ it "logs a warning and returns" do allow(Sentry::GoodJob::Logger).to receive(:warn) + # Mock Rails.application.config.after_initialize to execute immediately + allow(Rails.application.config).to receive(:after_initialize).and_yield described_class::Integration.setup_monitoring_for_job("test_job", job_config) @@ -226,7 +228,7 @@ let(:job_config) { { cron: "0 * * * *" } } it "does not set up monitoring" do - expect(Sentry::GoodJob::JobMonitor).not_to receive(:setup_for_job_class) + # No job monitoring setup needed since we removed JobMonitor described_class::Integration.setup_monitoring_for_job("test_job", job_config) end end @@ -235,7 +237,7 @@ let(:job_config) { { class: "TestJob" } } it "does not set up monitoring" do - expect(Sentry::GoodJob::JobMonitor).not_to receive(:setup_for_job_class) + # No job monitoring setup needed since we removed JobMonitor described_class::Integration.setup_monitoring_for_job("test_job", job_config) end end @@ -243,22 +245,26 @@ context "when job config is complete" do let(:job_config) { { class: "TestJob", cron: "0 * * * *" } } - it "sets up job monitoring" do - expect(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class).with(job_class) + it "sets up cron monitoring" do + expect(job_class).to receive(:include).with(Sentry::Cron::MonitorCheckIns).at_least(:once) + expect(job_class).to receive(:sentry_monitor_check_ins) + # Mock Rails.application.config.after_initialize to execute immediately + allow(Rails.application.config).to receive(:after_initialize).and_yield described_class::Integration.setup_monitoring_for_job("test_job", job_config) end - it "sets up cron monitoring" do - allow(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class) + it "sets up cron monitoring with proper configuration" do expect(job_class).to receive(:sentry_monitor_check_ins) - + # Mock Rails.application.config.after_initialize to execute immediately + allow(Rails.application.config).to receive(:after_initialize).and_yield described_class::Integration.setup_monitoring_for_job("test_job", job_config) end it "logs the setup completion" do - allow(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class) allow(job_class).to receive(:sentry_monitor_check_ins) allow(Sentry::GoodJob::Logger).to receive(:info) + # Mock Rails.application.config.after_initialize to execute immediately + allow(Rails.application.config).to receive(:after_initialize).and_yield described_class::Integration.setup_monitoring_for_job("test_job", job_config) @@ -274,34 +280,35 @@ allow(Sentry).to receive(:initialized?).and_return(true) end - it "sets up job monitoring" do - expect(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class).with(job_class) + it "sets up cron monitoring" do + expect(job_class).to receive(:include).with(Sentry::Cron::MonitorCheckIns).at_least(:once) + expect(job_class).to receive(:sentry_monitor_check_ins) described_class::Integration.add_monitoring_to_job(job_class) end it "sets up cron monitoring with default config" do - allow(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class) + # JobMonitor removed - no setup needed expect(job_class).to receive(:sentry_monitor_check_ins) described_class::Integration.add_monitoring_to_job(job_class) end it "uses provided slug" do - allow(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class) + # JobMonitor removed - no setup needed expect(job_class).to receive(:sentry_monitor_check_ins).with(hash_including(slug: "custom_slug")) described_class::Integration.add_monitoring_to_job(job_class, slug: "custom_slug") end it "uses provided cron expression" do - allow(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class) + # JobMonitor removed - no setup needed expect(job_class).to receive(:sentry_monitor_check_ins) described_class::Integration.add_monitoring_to_job(job_class, cron_expression: "0 0 * * *") end it "logs the setup completion" do - allow(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class) + # JobMonitor removed - no setup needed allow(job_class).to receive(:sentry_monitor_check_ins) allow(Sentry::GoodJob::Logger).to receive(:info) diff --git a/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb b/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb deleted file mode 100644 index e00d16d22..000000000 --- a/sentry-good_job/spec/sentry/good_job/error_handler_spec.rb +++ /dev/null @@ -1,214 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -RSpec.describe Sentry::GoodJob::ErrorHandler do - before do - perform_basic_setup - end - - let(:handler) { described_class.new } - let(:transport) { Sentry.get_current_client.transport } - - describe "#call" do - let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 1, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } - let(:job_class) { double("JobClass", name: "TestJob") } - let(:exception) { build_exception } - - context "when Sentry is not initialized" do - before do - allow(Sentry).to receive(:initialized?).and_return(false) - end - - it "does not capture the exception" do - expect(Sentry::GoodJob).not_to receive(:capture_exception) - handler.call(exception, job) - end - end - - context "when Sentry is initialized" do - before do - allow(Sentry).to receive(:initialized?).and_return(true) - end - - it "captures the exception with correct context" do - expect(Sentry::GoodJob).to receive(:capture_exception).with( - exception, - contexts: { good_job: hash_including( - job_class: "TestJob", - job_id: "123", - queue_name: "default", - executions: 1 - ) }, - hint: { background: true } - ) - - handler.call(exception, job) - end - - context "when include_job_arguments is enabled" do - before do - Sentry.configuration.good_job.include_job_arguments = true - end - - it "includes job arguments in context" do - expect(Sentry::GoodJob).to receive(:capture_exception).with( - exception, - contexts: { good_job: hash_including(arguments: []) }, - hint: { background: true } - ) - - handler.call(exception, job) - end - end - - context "when report_after_job_retries is enabled" do - before do - Sentry.configuration.good_job.report_after_job_retries = true - end - - context "and job is retryable" do - let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 2, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } - - context "and retry count is less than max retries" do - it "does not capture the exception" do - expect(Sentry::GoodJob).not_to receive(:capture_exception) - handler.call(exception, job) - end - end - - context "and retry count exceeds max retries" do - let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 6, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } - - it "captures the exception" do - expect(Sentry::GoodJob).to receive(:capture_exception) - handler.call(exception, job) - end - end - end - - context "and job is not retryable" do - let(:job_class) { double("JobClass", name: "TestJob") } - - it "captures the exception" do - expect(Sentry::GoodJob).to receive(:capture_exception) - handler.call(exception, job) - end - end - end - - context "when report_only_discarded_jobs is enabled" do - before do - Sentry.configuration.good_job.report_only_discarded_jobs = true - end - - context "and job is retryable" do - let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 2, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } - - it "does not capture the exception" do - expect(Sentry::GoodJob).not_to receive(:capture_exception) - handler.call(exception, job) - end - end - - context "and job is not retryable" do - let(:job) { double("Job", class: job_class, job_id: "123", queue_name: "default", executions: 1, enqueued_at: Time.now, scheduled_at: nil, arguments: []) } - - it "captures the exception" do - expect(Sentry::GoodJob).to receive(:capture_exception) - handler.call(exception, job) - end - end - end - end - end - - describe "#retryable?" do - it "returns true when job has been executed more than once" do - job = double("Job", executions: 2) - - expect(handler.send(:retryable?, job)).to be true - end - - it "returns false when job has been executed only once" do - job = double("Job", executions: 1) - - expect(handler.send(:retryable?, job)).to be false - end - - it "returns false when job has not been executed" do - job = double("Job", executions: 0) - - expect(handler.send(:retryable?, job)).to be false - end - end - - describe "#has_exhausted_retries?" do - it "returns true when job has been executed more than default retry attempts" do - job = double("Job", executions: 6) - - expect(handler.send(:has_exhausted_retries?, job)).to be true - end - - it "returns false when job has been executed less than default retry attempts" do - job = double("Job", executions: 3) - - expect(handler.send(:has_exhausted_retries?, job)).to be false - end - - it "returns false when job has been executed exactly default retry attempts" do - job = double("Job", executions: 5) - - expect(handler.send(:has_exhausted_retries?, job)).to be false - end - end - - describe "#job_context" do - let(:job) do - double("Job", - class: double("JobClass", name: "TestJob"), - job_id: "123", - queue_name: "default", - executions: 1, - enqueued_at: Time.now, - scheduled_at: nil, - arguments: ["arg1", "arg2"] - ) - end - - it "returns basic job context" do - context = handler.send(:job_context, job) - - expect(context).to include( - job_class: "TestJob", - job_id: "123", - queue_name: "default", - executions: 1 - ) - end - - context "when include_job_arguments is enabled" do - before do - Sentry.configuration.good_job.include_job_arguments = true - end - - it "includes arguments in context" do - context = handler.send(:job_context, job) - - expect(context[:arguments]).to eq(['"arg1"', '"arg2"']) - end - end - - context "when include_job_arguments is disabled" do - before do - Sentry.configuration.good_job.include_job_arguments = false - end - - it "does not include arguments in context" do - context = handler.send(:job_context, job) - - expect(context).not_to have_key(:arguments) - end - end - end -end diff --git a/sentry-good_job/spec/sentry/good_job/job_monitor_spec.rb b/sentry-good_job/spec/sentry/good_job/job_monitor_spec.rb deleted file mode 100644 index aa64fcf56..000000000 --- a/sentry-good_job/spec/sentry/good_job/job_monitor_spec.rb +++ /dev/null @@ -1,120 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -RSpec.describe Sentry::GoodJob::JobMonitor do - before do - perform_basic_setup - end - - let(:job_class) { Class.new(ActiveJob::Base) } - - describe ".setup_for_job_class" do - context "when Rails is not available" do - before do - hide_const("Rails") - end - - it "does not set up monitoring" do - expect(job_class).not_to receive(:include) - described_class.setup_for_job_class(job_class) - end - end - - context "when Sentry is not initialized" do - before do - stub_const("Rails", double("Rails")) - allow(Sentry).to receive(:initialized?).and_return(false) - end - - it "does not set up monitoring" do - expect(job_class).not_to receive(:include) - described_class.setup_for_job_class(job_class) - end - end - - context "when Rails and Sentry are available" do - before do - stub_const("Rails", double("Rails")) - allow(Sentry).to receive(:initialized?).and_return(true) - end - - it "includes Sentry::Cron::MonitorCheckIns" do - expect(job_class).to receive(:include).with(Sentry::Cron::MonitorCheckIns) - described_class.setup_for_job_class(job_class) - end - - it "adds _sentry attribute accessor" do - expect(job_class).to receive(:attr_accessor).with(:_sentry) - described_class.setup_for_job_class(job_class) - end - - it "sets up around_enqueue hook" do - expect(job_class).to receive(:around_enqueue) - described_class.setup_for_job_class(job_class) - end - - it "sets up around_perform hook" do - expect(job_class).to receive(:around_perform) - described_class.setup_for_job_class(job_class) - end - - it "adds sentry_cron_monitor class method" do - described_class.setup_for_job_class(job_class) - expect(job_class).to respond_to(:sentry_cron_monitor) - end - - it "adds enqueue instance method" do - described_class.setup_for_job_class(job_class) - expect(job_class.instance_methods).to include(:enqueue) - end - - it "adds serialize instance method" do - described_class.setup_for_job_class(job_class) - expect(job_class.instance_methods).to include(:serialize) - end - - it "adds deserialize instance method" do - described_class.setup_for_job_class(job_class) - expect(job_class.instance_methods).to include(:deserialize) - end - - it "adds private helper methods" do - described_class.setup_for_job_class(job_class) - expect(job_class.private_instance_methods).to include(:_sentry_set_span_data) - expect(job_class.private_instance_methods).to include(:_sentry_job_context) - expect(job_class.private_instance_methods).to include(:_sentry_start_transaction) - expect(job_class.private_instance_methods).to include(:_sentry_finish_transaction) - end - - it "makes helper methods private" do - expect(job_class).to receive(:class_eval).at_least(:once) - described_class.setup_for_job_class(job_class) - end - - context "when job class already has Sentry::Cron::MonitorCheckIns included" do - before do - job_class.include(Sentry::Cron::MonitorCheckIns) - end - - it "does not include it again" do - expect(job_class).not_to receive(:include).with(Sentry::Cron::MonitorCheckIns) - described_class.setup_for_job_class(job_class) - end - end - - context "when hooks are already set up" do - before do - job_class.define_singleton_method(:sentry_enqueue_hook_setup) { true } - job_class.define_singleton_method(:sentry_perform_hook_setup) { true } - end - - it "does not set up hooks again" do - expect(job_class).not_to receive(:around_enqueue) - expect(job_class).not_to receive(:around_perform) - described_class.setup_for_job_class(job_class) - end - end - end - end -end diff --git a/sentry-good_job/spec/sentry/good_job_spec.rb b/sentry-good_job/spec/sentry/good_job_spec.rb index 7a8d0d366..928280536 100644 --- a/sentry-good_job/spec/sentry/good_job_spec.rb +++ b/sentry-good_job/spec/sentry/good_job_spec.rb @@ -33,38 +33,37 @@ end it "does not automatically set up job monitoring for any specific job class" do - expect(Sentry::GoodJob::JobMonitor).not_to receive(:setup_for_job_class) + # The integration now only sets up cron monitoring, not custom job monitoring + expect(Sentry::GoodJob::CronHelpers::Integration).to receive(:setup_monitoring_for_scheduled_jobs) described_class.setup_good_job_integration end - it "allows manual setup of job monitoring" do - job_class = Class.new(ActiveJob::Base) + it "sets up cron monitoring when enabled" do + expect(Sentry::GoodJob::CronHelpers::Integration).to receive(:setup_monitoring_for_scheduled_jobs) - expect(Sentry::GoodJob::JobMonitor).to receive(:setup_for_job_class).with(job_class) - - Sentry::GoodJob::JobMonitor.setup_for_job_class(job_class) + described_class.setup_good_job_integration end - context "when auto_setup_cron_monitoring is enabled" do + context "when enable_cron_monitors is enabled" do before do - Sentry.configuration.good_job.auto_setup_cron_monitoring = true + Sentry.configuration.good_job.enable_cron_monitors = true end it "sets up cron monitoring" do - expect(Sentry::GoodJob::CronMonitoring::Integration).to receive(:setup_monitoring_for_scheduled_jobs) + expect(Sentry::GoodJob::CronHelpers::Integration).to receive(:setup_monitoring_for_scheduled_jobs) described_class.setup_good_job_integration end end - context "when auto_setup_cron_monitoring is disabled" do + context "when enable_cron_monitors is disabled" do before do - Sentry.configuration.good_job.auto_setup_cron_monitoring = false + Sentry.configuration.good_job.enable_cron_monitors = false end it "does not set up cron monitoring" do - expect(Sentry::GoodJob::CronMonitoring::Integration).not_to receive(:setup_monitoring_for_scheduled_jobs) + expect(Sentry::GoodJob::CronHelpers::Integration).not_to receive(:setup_monitoring_for_scheduled_jobs) described_class.setup_good_job_integration end diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index 2f5a134d0..ed74e981f 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -5,6 +5,9 @@ module Sentry module Rails module ActiveJobExtensions + # Add _sentry attribute for storing context and trace data + attr_accessor :_sentry + def perform_now if !Sentry.initialized? || already_supported_by_sentry_integration? super @@ -19,6 +22,76 @@ def already_supported_by_sentry_integration? Sentry.configuration.rails.skippable_job_adapters.include?(self.class.queue_adapter.class.to_s) end + # Enhanced enqueue method that captures context and trace data + def enqueue(options = {}) + self._sentry ||= {} + + # Capture user context + user = ::Sentry.get_current_scope&.user + self._sentry["user"] = user if user.present? + + # Capture trace propagation headers + self._sentry["trace_propagation_headers"] = ::Sentry.get_trace_propagation_headers + + super(options) + end + + # Enhanced serialize method that includes _sentry data + def serialize + super.tap do |job_data| + if _sentry + job_data["_sentry"] = _sentry.to_json + end + end + rescue + # Swallow errors. Better to lose Sentry context than fail to serialize the job. + super + end + + # Enhanced deserialize method that restores _sentry data + def deserialize(job_data) + super(job_data) + + begin + self._sentry = JSON.parse(job_data["_sentry"]) if job_data["_sentry"] + rescue + # Swallow errors. Better to lose Sentry context than fail to deserialize the job. + end + end + + # Set span data with messaging semantics + def set_span_data(span, job, retry_count: nil) + return unless span + + span.set_data("messaging.message.id", job.job_id) + span.set_data("messaging.destination.name", job.queue_name) if job.respond_to?(:queue_name) + span.set_data("messaging.message.retry.count", retry_count) if retry_count + end + + # Start transaction with trace propagation + def start_transaction(scope, trace_headers) + options = { + name: scope.transaction_name, + source: scope.transaction_source, + op: "queue.process", + origin: "auto.queue.active_job" + } + + transaction = ::Sentry.continue_trace(trace_headers, **options) + ::Sentry.start_transaction(transaction: transaction, **options) + end + + # Finish transaction with proper status + def finish_transaction(transaction, status) + return unless transaction + + transaction.set_http_status(status) + transaction.finish + end + + private + + class SentryReporter OP_NAME = "queue.active_job" SPAN_ORIGIN = "auto.queue.active_job" @@ -32,20 +105,33 @@ def record(job, &block) Sentry.with_scope do |scope| begin scope.set_transaction_name(job.class.name, source: :task) - transaction = - if job.is_a?(::Sentry::SendEventJob) - nil - else - Sentry.start_transaction( - name: scope.transaction_name, - source: scope.transaction_source, - op: OP_NAME, - origin: SPAN_ORIGIN - ) - end + + # Restore user context if available + if job._sentry && (user = job._sentry["user"]) + scope.set_user(user) + end + + # Set up transaction with trace propagation + transaction = nil + if job._sentry && job._sentry["trace_propagation_headers"] + transaction = job.start_transaction(scope, job._sentry["trace_propagation_headers"]) + else + transaction = Sentry.start_transaction( + name: scope.transaction_name, + source: scope.transaction_source, + op: OP_NAME, + origin: SPAN_ORIGIN + ) unless job.is_a?(::Sentry::SendEventJob) + end scope.set_span(transaction) if transaction + # Add enhanced span data + if transaction + retry_count = job.executions.is_a?(Integer) ? job.executions - 1 : 0 + job.set_span_data(transaction, job, retry_count: retry_count) + end + yield.tap do finish_sentry_transaction(transaction, 200) end From 07e9c7a470e425bc7d61ed068b7e2732e46c2e28 Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Fri, 17 Oct 2025 09:53:52 +0300 Subject: [PATCH 08/24] Fix enhance_sentry_reporter method to properly encapsulate patch --- .../lib/sentry/good_job/active_job_extensions.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sentry-good_job/lib/sentry/good_job/active_job_extensions.rb b/sentry-good_job/lib/sentry/good_job/active_job_extensions.rb index 17c35d070..116a1881b 100644 --- a/sentry-good_job/lib/sentry/good_job/active_job_extensions.rb +++ b/sentry-good_job/lib/sentry/good_job/active_job_extensions.rb @@ -58,11 +58,13 @@ def self.setup def self.enhance_sentry_reporter # Enhance the sentry_context method in SentryReporter ::Sentry::Rails::ActiveJobExtensions::SentryReporter.class_eval do - alias_method :original_sentry_context, :sentry_context + class << self + alias_method :original_sentry_context, :sentry_context - def sentry_context(job) - base_context = original_sentry_context(job) - Sentry::GoodJob::ActiveJobExtensions.enhance_sentry_context(job, base_context) + def sentry_context(job) + base_context = original_sentry_context(job) + Sentry::GoodJob::ActiveJobExtensions.enhance_sentry_context(job, base_context) + end end end end From c79383f46f6fed269d12ac9dccaaf215dc6b7e43 Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Fri, 17 Oct 2025 10:04:21 +0300 Subject: [PATCH 09/24] Enhance cron monitoring setup in Sentry GoodJob integration - Introduced a mechanism to prevent duplicate setup of cron monitoring by tracking setup state with a class variable. - Added a method to reset the setup state for testing purposes. - Updated the integration logic to ensure proper initialization checks for Rails applications. - Adjusted tests to utilize the new reset method, ensuring accurate monitoring setup during tests. --- .../lib/sentry/good_job/cron_helpers.rb | 15 +++++++++++++-- .../spec/sentry/good_job/cron_helpers_spec.rb | 2 ++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/sentry-good_job/lib/sentry/good_job/cron_helpers.rb b/sentry-good_job/lib/sentry/good_job/cron_helpers.rb index 936528e1c..7bc0aadab 100644 --- a/sentry-good_job/lib/sentry/good_job/cron_helpers.rb +++ b/sentry-good_job/lib/sentry/good_job/cron_helpers.rb @@ -65,11 +65,16 @@ def self.parse_cron_with_timezone(cron_expression) # Main integration class that handles all cron monitoring setup # This class follows Good Job's integration patterns and Sentry's extension guidelines class Integration + # Track whether setup has already been performed to prevent duplicates + @setup_completed = false + # Set up monitoring for all scheduled jobs from Good Job configuration def self.setup_monitoring_for_scheduled_jobs return unless ::Sentry.initialized? return unless ::Sentry.configuration.good_job.enable_cron_monitors + return if @setup_completed + return unless defined?(::Rails) && ::Rails.respond_to?(:application) && ::Rails.application cron_config = ::Rails.application.config.good_job.cron return unless cron_config.present? @@ -77,9 +82,15 @@ def self.setup_monitoring_for_scheduled_jobs setup_monitoring_for_job(cron_key, job_config) end + @setup_completed = true Sentry::GoodJob::Logger.info "Sentry cron monitoring setup for #{cron_config.keys.size} scheduled jobs" end + # Reset setup state (primarily for testing) + def self.reset_setup_state! + @setup_completed = false + end + # Set up monitoring for a specific job def self.setup_monitoring_for_job(cron_key, job_config) job_class_name = job_config[:class] @@ -128,8 +139,8 @@ def self.setup_monitoring_for_job(cron_key, job_config) # Set up monitoring when the job class is first loaded # This defers constantization until the job is actually needed - if defined?(Rails) && Rails.application - Rails.application.config.after_initialize do + if defined?(::Rails) && ::Rails.respond_to?(:application) && ::Rails.application + ::Rails.application.config.after_initialize do deferred_setup.call end else diff --git a/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb b/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb index 3efa35707..3077af9a9 100644 --- a/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb +++ b/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb @@ -185,6 +185,7 @@ end it "sets up monitoring for each job" do + described_class::Integration.reset_setup_state! expect(described_class::Integration).to receive(:setup_monitoring_for_job).with("test_job", cron_config["test_job"]) expect(described_class::Integration).to receive(:setup_monitoring_for_job).with("another_job", cron_config["another_job"]) @@ -192,6 +193,7 @@ end it "logs the setup completion" do + described_class::Integration.reset_setup_state! allow(described_class::Integration).to receive(:setup_monitoring_for_job) allow(Sentry::GoodJob::Logger).to receive(:info) From 852b0fd7f8ffadc7662ccacc6084c6da01802cab Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Fri, 17 Oct 2025 10:04:33 +0300 Subject: [PATCH 10/24] Improve error handling in Sentry ActiveJob integration - Specify error types for JSON serialization and parsing exceptions to enhance clarity in error handling. - Update comments to reflect the specific errors being rescued, ensuring better understanding of the error management strategy. --- sentry-rails/lib/sentry/rails/active_job.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index ed74e981f..dedbd0226 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -43,8 +43,8 @@ def serialize job_data["_sentry"] = _sentry.to_json end end - rescue - # Swallow errors. Better to lose Sentry context than fail to serialize the job. + rescue JSON::GeneratorError, TypeError + # Swallow JSON serialization errors. Better to lose Sentry context than fail to serialize the job. super end @@ -54,8 +54,8 @@ def deserialize(job_data) begin self._sentry = JSON.parse(job_data["_sentry"]) if job_data["_sentry"] - rescue - # Swallow errors. Better to lose Sentry context than fail to deserialize the job. + rescue JSON::ParserError + # Swallow JSON parsing errors. Better to lose Sentry context than fail to deserialize the job. end end From 5fc4b5ca573c4c66f2b1e6bc5e903d1e5a7cab40 Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Fri, 17 Oct 2025 10:15:07 +0300 Subject: [PATCH 11/24] Enhance cron monitoring logging in Sentry GoodJob integration - Updated the cron monitoring setup to log the names of successfully added scheduled jobs, improving clarity in monitoring output. - Modified the `setup_monitoring_for_job` method to return the job name upon successful setup, facilitating better tracking of job configurations. - Adjusted tests to verify the updated logging behavior, ensuring accurate reporting of scheduled jobs. --- .../lib/sentry/good_job/cron_helpers.rb | 16 ++++++++++++---- .../spec/sentry/good_job/cron_helpers_spec.rb | 11 +++++------ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/sentry-good_job/lib/sentry/good_job/cron_helpers.rb b/sentry-good_job/lib/sentry/good_job/cron_helpers.rb index 7bc0aadab..5bfd4804c 100644 --- a/sentry-good_job/lib/sentry/good_job/cron_helpers.rb +++ b/sentry-good_job/lib/sentry/good_job/cron_helpers.rb @@ -78,12 +78,19 @@ def self.setup_monitoring_for_scheduled_jobs cron_config = ::Rails.application.config.good_job.cron return unless cron_config.present? + added_jobs = [] cron_config.each do |cron_key, job_config| - setup_monitoring_for_job(cron_key, job_config) + job_name = setup_monitoring_for_job(cron_key, job_config) + added_jobs << job_name if job_name end @setup_completed = true - Sentry::GoodJob::Logger.info "Sentry cron monitoring setup for #{cron_config.keys.size} scheduled jobs" + if added_jobs.any? + job_list = added_jobs.join(", ") + Sentry::GoodJob::Logger.info "Sentry cron monitoring setup for #{added_jobs.size} scheduled jobs: #{job_list}" + else + Sentry::GoodJob::Logger.info "Sentry cron monitoring setup for #{cron_config.keys.size} scheduled jobs" + end end # Reset setup state (primarily for testing) @@ -109,7 +116,7 @@ def self.setup_monitoring_for_job(cron_key, job_config) job_class_name.constantize rescue NameError => e Sentry::GoodJob::Logger.warn "Could not find job class '#{job_class_name}' for Sentry cron monitoring: #{e.message}" - return + return nil end # Include Sentry::Cron::MonitorCheckIns module for cron monitoring @@ -131,9 +138,10 @@ def self.setup_monitoring_for_job(cron_key, job_config) monitor_config: monitor_config ) - Sentry::GoodJob::Logger.info "Added Sentry cron monitoring for #{job_class_name} (#{monitor_slug})" + return job_class_name else Sentry::GoodJob::Logger.warn "Could not create monitor config for #{job_class_name} with cron '#{cron_expression}'" + return nil end end diff --git a/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb b/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb index 3077af9a9..d6217ab6e 100644 --- a/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb +++ b/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb @@ -194,12 +194,12 @@ it "logs the setup completion" do described_class::Integration.reset_setup_state! - allow(described_class::Integration).to receive(:setup_monitoring_for_job) + allow(described_class::Integration).to receive(:setup_monitoring_for_job).and_return("TestJob", "AnotherJob") allow(Sentry::GoodJob::Logger).to receive(:info) described_class::Integration.setup_monitoring_for_scheduled_jobs - expect(Sentry::GoodJob::Logger).to have_received(:info).with("Sentry cron monitoring setup for 2 scheduled jobs") + expect(Sentry::GoodJob::Logger).to have_received(:info).with("Sentry cron monitoring setup for 2 scheduled jobs: TestJob, AnotherJob") end end end @@ -262,15 +262,14 @@ described_class::Integration.setup_monitoring_for_job("test_job", job_config) end - it "logs the setup completion" do + it "returns the job name when setup is successful" do allow(job_class).to receive(:sentry_monitor_check_ins) - allow(Sentry::GoodJob::Logger).to receive(:info) # Mock Rails.application.config.after_initialize to execute immediately allow(Rails.application.config).to receive(:after_initialize).and_yield - described_class::Integration.setup_monitoring_for_job("test_job", job_config) + result = described_class::Integration.setup_monitoring_for_job("test_job", job_config) - expect(Sentry::GoodJob::Logger).to have_received(:info).with(/Added Sentry cron monitoring for TestJob/) + expect(result).to eq("TestJob") end end end From 9c95ff4f6a2620a73df0aaa1d3afa7e5fbc133eb Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Fri, 17 Oct 2025 10:21:06 +0300 Subject: [PATCH 12/24] Refactor error handling in cron monitoring setup for Sentry GoodJob integration - Updated the `setup_monitoring_for_job` method to return `nil` instead of `nil` for better clarity in error handling. - Removed redundant checks for job class ancestors to streamline the monitoring setup process. - Enhanced logging for job class resolution failures, improving the overall robustness of the cron monitoring integration. --- sentry-good_job/lib/sentry/good_job/cron_helpers.rb | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/sentry-good_job/lib/sentry/good_job/cron_helpers.rb b/sentry-good_job/lib/sentry/good_job/cron_helpers.rb index 5bfd4804c..136507563 100644 --- a/sentry-good_job/lib/sentry/good_job/cron_helpers.rb +++ b/sentry-good_job/lib/sentry/good_job/cron_helpers.rb @@ -116,7 +116,7 @@ def self.setup_monitoring_for_job(cron_key, job_config) job_class_name.constantize rescue NameError => e Sentry::GoodJob::Logger.warn "Could not find job class '#{job_class_name}' for Sentry cron monitoring: #{e.message}" - return nil + return end # Include Sentry::Cron::MonitorCheckIns module for cron monitoring @@ -141,7 +141,7 @@ def self.setup_monitoring_for_job(cron_key, job_config) return job_class_name else Sentry::GoodJob::Logger.warn "Could not create monitor config for #{job_class_name} with cron '#{cron_expression}'" - return nil + return end end @@ -155,6 +155,9 @@ def self.setup_monitoring_for_job(cron_key, job_config) # Fallback for non-Rails environments deferred_setup.call end + + # Return the job name for logging purposes + job_class_name end # Manually add cron monitoring to a specific job @@ -178,11 +181,6 @@ def self.add_monitoring_to_job(job_class, slug: nil, cron_expression: nil, timez if monitor_config monitor_slug = slug || Sentry::GoodJob::CronHelpers::Helpers.monitor_slug(job_class.name) - # only patch if not explicitly included in job by user - unless job_class.ancestors.include?(Sentry::Cron::MonitorCheckIns) - job_class.include(Sentry::Cron::MonitorCheckIns) - end - job_class.sentry_monitor_check_ins( slug: monitor_slug, monitor_config: monitor_config From 4896a51b557885ecd4519271ee0cb9c26b7e6846 Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Fri, 17 Oct 2025 10:29:35 +0300 Subject: [PATCH 13/24] Refactor transaction initialization in Sentry integrations - Removed redundant `**options` argument from `Sentry.start_transaction` calls in multiple integrations, including Delayed Job, Active Job, Rack, and Sidekiq. - Streamlined transaction initialization for improved clarity and consistency across the Sentry integrations. --- sentry-delayed_job/lib/sentry/delayed_job/plugin.rb | 2 +- sentry-rails/lib/sentry/rails/active_job.rb | 2 +- sentry-ruby/lib/sentry/rack/capture_exceptions.rb | 2 +- sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb b/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb index 66093f340..135c4a313 100644 --- a/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb +++ b/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb @@ -102,7 +102,7 @@ def self.start_transaction(scope, env, contexts) } transaction = Sentry.continue_trace(env, **options) - Sentry.start_transaction(transaction: transaction, custom_sampling_context: contexts, **options) + Sentry.start_transaction(transaction: transaction, custom_sampling_context: contexts) end def self.finish_transaction(transaction, status) diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index dedbd0226..1560e13ae 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -78,7 +78,7 @@ def start_transaction(scope, trace_headers) } transaction = ::Sentry.continue_trace(trace_headers, **options) - ::Sentry.start_transaction(transaction: transaction, **options) + ::Sentry.start_transaction(transaction: transaction) end # Finish transaction with proper status diff --git a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb index 40cdfb4f8..b707693ff 100644 --- a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb +++ b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb @@ -72,7 +72,7 @@ def start_transaction(env, scope) } transaction = Sentry.continue_trace(env, **options) - Sentry.start_transaction(transaction: transaction, custom_sampling_context: { env: env }, **options) + Sentry.start_transaction(transaction: transaction, custom_sampling_context: { env: env }) end diff --git a/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb b/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb index c1fb4b6b1..f26a25cb9 100644 --- a/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb +++ b/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb @@ -91,7 +91,7 @@ def start_transaction(scope, env) } transaction = Sentry.continue_trace(env, **options) - Sentry.start_transaction(transaction: transaction, **options) + Sentry.start_transaction(transaction: transaction) end def finish_transaction(transaction, status) From 3a03a195c86ffb6ae842c3298c223074b8cf3c97 Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Fri, 17 Oct 2025 10:39:35 +0300 Subject: [PATCH 14/24] Refactor transaction handling in Sentry ActiveJob integration - Improved transaction initialization logic to avoid creating a new transaction for Sentry::SendEventJob, even when trace propagation headers are present. - Enhanced retry count calculation to ensure it only accesses the `executions` attribute if it is defined and an integer. - Added a new test to verify that a SendEventJob does not create a new transaction when trace propagation headers are used. --- sentry-rails/lib/sentry/rails/active_job.rb | 26 ++++++++++++------- .../spec/sentry/send_event_job_spec.rb | 26 +++++++++++++++++++ 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index 1560e13ae..f39eee9d6 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -113,22 +113,28 @@ def record(job, &block) # Set up transaction with trace propagation transaction = nil - if job._sentry && job._sentry["trace_propagation_headers"] - transaction = job.start_transaction(scope, job._sentry["trace_propagation_headers"]) - else - transaction = Sentry.start_transaction( - name: scope.transaction_name, - source: scope.transaction_source, - op: OP_NAME, - origin: SPAN_ORIGIN - ) unless job.is_a?(::Sentry::SendEventJob) + unless job.is_a?(::Sentry::SendEventJob) + if job._sentry && job._sentry["trace_propagation_headers"] + transaction = job.start_transaction(scope, job._sentry["trace_propagation_headers"]) + else + transaction = Sentry.start_transaction( + name: scope.transaction_name, + source: scope.transaction_source, + op: OP_NAME, + origin: SPAN_ORIGIN + ) + end end scope.set_span(transaction) if transaction # Add enhanced span data if transaction - retry_count = job.executions.is_a?(Integer) ? job.executions - 1 : 0 + retry_count = if job.respond_to?(:executions) && job.executions.is_a?(Integer) + job.executions - 1 + else + 0 + end job.set_span_data(transaction, job, retry_count: retry_count) end diff --git a/sentry-rails/spec/sentry/send_event_job_spec.rb b/sentry-rails/spec/sentry/send_event_job_spec.rb index 00f89b2ed..bee75a522 100644 --- a/sentry-rails/spec/sentry/send_event_job_spec.rb +++ b/sentry-rails/spec/sentry/send_event_job_spec.rb @@ -61,6 +61,32 @@ expect(event.type).to eq("event") end + it "doesn't create a new transaction even with trace propagation headers" do + make_basic_app do |config| + config.traces_sample_rate = 1.0 + end + + # Start a transaction to create trace propagation headers + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) + + # Create a SendEventJob with trace propagation headers + job = Sentry::SendEventJob.new + job._sentry = { + "trace_propagation_headers" => Sentry.get_trace_propagation_headers + } + + # Set the arguments for the job + job.arguments = [event, {}] + + # Perform the job + job.perform_now + + expect(transport.events.count).to eq(1) + event = transport.events.first + expect(event.type).to eq("event") + end + context "when ApplicationJob is not defined" do before do make_basic_app From 5cdb3d11dbb5bcf9c86b16635631635d363e11b0 Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Fri, 17 Oct 2025 10:52:07 +0300 Subject: [PATCH 15/24] Refactor logging in Sentry GoodJob integration - Removed deprecated logging configuration options and centralized logging under the standard Sentry SDK logger. - Updated documentation in CHANGELOG.md and README.md to reflect the removal of `logging_enabled` and `logger` options. - Enhanced logging statements throughout the integration to utilize the new logging approach, improving consistency and clarity in log output. - Adjusted tests to verify the updated logging behavior and ensure compatibility with the refactored integration. --- sentry-good_job/CHANGELOG.md | 2 - sentry-good_job/README.md | 47 ++++- sentry-good_job/example/app.rb | 2 +- sentry-good_job/lib/sentry-good_job.rb | 3 +- .../lib/sentry/good_job/configuration.rb | 8 - .../lib/sentry/good_job/cron_helpers.rb | 12 +- sentry-good_job/lib/sentry/good_job/logger.rb | 40 ----- .../sentry/good_job/configuration_spec.rb | 12 -- .../spec/sentry/good_job/cron_helpers_spec.rb | 16 +- .../spec/sentry/good_job/logger_spec.rb | 161 ------------------ 10 files changed, 54 insertions(+), 249 deletions(-) delete mode 100644 sentry-good_job/lib/sentry/good_job/logger.rb delete mode 100644 sentry-good_job/spec/sentry/good_job/logger_spec.rb diff --git a/sentry-good_job/CHANGELOG.md b/sentry-good_job/CHANGELOG.md index 700fb23d0..4f841795d 100644 --- a/sentry-good_job/CHANGELOG.md +++ b/sentry-good_job/CHANGELOG.md @@ -18,8 +18,6 @@ Individual gem's changelog has been deprecated. Please check the [project change #### Good Job Specific Options - `enable_cron_monitors`: Enable cron monitoring for scheduled jobs -- `logging_enabled`: Enable logging for the Good Job integration -- `logger`: Custom logger to use #### ActiveJob Options (handled by sentry-rails) - `config.rails.active_job_report_on_retry_error`: Only report errors after all retry attempts are exhausted diff --git a/sentry-good_job/README.md b/sentry-good_job/README.md index dbb7961ed..4535b088f 100644 --- a/sentry-good_job/README.md +++ b/sentry-good_job/README.md @@ -49,7 +49,6 @@ Sentry.init do |config| # Good Job specific configuration config.good_job.enable_cron_monitors = true - config.good_job.logging_enabled = false # ActiveJob configuration (handled by sentry-rails) config.rails.active_job_report_on_retry_error = false @@ -62,8 +61,6 @@ end #### Good Job Specific Options - `enable_cron_monitors` (default: `true`): Enable cron monitoring for scheduled jobs -- `logging_enabled` (default: `false`): Enable logging for the Good Job integration -- `logger` (default: `nil`): Custom logger to use (defaults to Rails.logger when available) #### ActiveJob Options (handled by sentry-rails) @@ -132,24 +129,56 @@ end ### Debugging and Detailed Logging -For debugging purposes, you can enable detailed logging to see what the integration is doing: +The integration uses the standard Sentry SDK logger (`Sentry.configuration.sdk_logger`) for all logging needs. You can configure this logger to get detailed information about what the integration is doing: ```ruby Sentry.init do |config| config.dsn = 'your-dsn-here' - # Enable detailed logging for debugging - config.good_job.logging_enabled = true - config.good_job.logger = Logger.new($stdout) + # Configure the SDK logger for debugging + config.sdk_logger = Logger.new($stdout) + config.sdk_logger.level = Logger::DEBUG + + # Or use Rails logger with debug level + # config.sdk_logger = Rails.logger + # config.sdk_logger.level = Logger::DEBUG +end +``` + +#### Custom Logger Configuration + +For more advanced logging needs, you can provide a custom logger: + +```ruby +# Custom logger with specific formatting +custom_logger = Logger.new('log/sentry-good_job.log') +custom_logger.formatter = proc do |severity, datetime, progname, msg| + "[#{datetime}] #{severity}: #{msg}\n" +end + +Sentry.init do |config| + config.dsn = 'your-dsn-here' + config.sdk_logger = custom_logger + config.sdk_logger.level = Logger::INFO end ``` -This will output detailed information about: +#### Log Levels + +The integration logs at different levels: +- **INFO**: Integration setup, cron monitoring configuration, job monitoring setup +- **WARN**: Configuration issues, missing job classes, cron parsing errors +- **DEBUG**: Detailed execution flow (when debug level is enabled) + +#### What Gets Logged + +When logging is enabled, you'll see information about: - Job execution start and completion - Error capture and reporting decisions -- Cron monitoring setup +- Cron monitoring setup and configuration - Performance metrics collection - GoodJob-specific context enhancement +- Integration initialization and setup ## Performance Monitoring diff --git a/sentry-good_job/example/app.rb b/sentry-good_job/example/app.rb index 8e096d7e6..811d1caf1 100644 --- a/sentry-good_job/example/app.rb +++ b/sentry-good_job/example/app.rb @@ -12,7 +12,7 @@ config.logger = Logger.new(STDOUT) # Good Job specific configuration - config.good_job.logging_enabled = true + # Logging is now handled by the standard Sentry SDK logger # ActiveJob configuration (handled by sentry-rails) config.rails.active_job_report_on_retry_error = false diff --git a/sentry-good_job/lib/sentry-good_job.rb b/sentry-good_job/lib/sentry-good_job.rb index c8108a3e8..3cfa7cd35 100644 --- a/sentry-good_job/lib/sentry-good_job.rb +++ b/sentry-good_job/lib/sentry-good_job.rb @@ -5,7 +5,6 @@ require "sentry/integrable" require "sentry/good_job/version" require "sentry/good_job/configuration" -require "sentry/good_job/logger" require "sentry/good_job/context_helpers" require "sentry/good_job/active_job_extensions" require "sentry/good_job/cron_helpers" @@ -38,7 +37,7 @@ def self.setup_good_job_integration Sentry::GoodJob::CronHelpers::Integration.setup_monitoring_for_scheduled_jobs end - Sentry::GoodJob::Logger.info "Sentry Good Job integration initialized automatically" + Sentry.configuration.sdk_logger.info "[sentry-good_job] Sentry Good Job integration initialized automatically" end # Delegate capture_exception so internal components can be tested in isolation diff --git a/sentry-good_job/lib/sentry/good_job/configuration.rb b/sentry-good_job/lib/sentry/good_job/configuration.rb index 976bd9db2..1ccfaad59 100644 --- a/sentry-good_job/lib/sentry/good_job/configuration.rb +++ b/sentry-good_job/lib/sentry/good_job/configuration.rb @@ -21,16 +21,8 @@ class Configuration # This is GoodJob-specific functionality for monitoring scheduled tasks attr_accessor :enable_cron_monitors - # When false, suppresses all Sentry GoodJob integration logs - attr_accessor :logging_enabled - - # Custom logger to use for Sentry GoodJob integration (defaults to Rails.logger) - attr_accessor :logger - def initialize @enable_cron_monitors = true - @logging_enabled = false - @logger = nil end end end diff --git a/sentry-good_job/lib/sentry/good_job/cron_helpers.rb b/sentry-good_job/lib/sentry/good_job/cron_helpers.rb index 136507563..50ef49f73 100644 --- a/sentry-good_job/lib/sentry/good_job/cron_helpers.rb +++ b/sentry-good_job/lib/sentry/good_job/cron_helpers.rb @@ -25,7 +25,7 @@ def self.monitor_config_from_cron(cron_expression, timezone: nil) ::Sentry::Cron::MonitorConfig.from_crontab(cron_expression) end rescue => e - Sentry::GoodJob::Logger.warn "Failed to parse cron expression '#{cron_expression}': #{e.message}" + Sentry.configuration.sdk_logger.warn "[sentry-good_job] Failed to parse cron expression '#{cron_expression}': #{e.message}" nil end @@ -87,9 +87,9 @@ def self.setup_monitoring_for_scheduled_jobs @setup_completed = true if added_jobs.any? job_list = added_jobs.join(", ") - Sentry::GoodJob::Logger.info "Sentry cron monitoring setup for #{added_jobs.size} scheduled jobs: #{job_list}" + Sentry.configuration.sdk_logger.info "[sentry-good_job] Sentry cron monitoring setup for #{added_jobs.size} scheduled jobs: #{job_list}" else - Sentry::GoodJob::Logger.info "Sentry cron monitoring setup for #{cron_config.keys.size} scheduled jobs" + Sentry.configuration.sdk_logger.info "[sentry-good_job] Sentry cron monitoring setup for #{cron_config.keys.size} scheduled jobs" end end @@ -115,7 +115,7 @@ def self.setup_monitoring_for_job(cron_key, job_config) job_class = begin job_class_name.constantize rescue NameError => e - Sentry::GoodJob::Logger.warn "Could not find job class '#{job_class_name}' for Sentry cron monitoring: #{e.message}" + Sentry.configuration.sdk_logger.warn "[sentry-good_job] Could not find job class '#{job_class_name}' for Sentry cron monitoring: #{e.message}" return end @@ -140,7 +140,7 @@ def self.setup_monitoring_for_job(cron_key, job_config) return job_class_name else - Sentry::GoodJob::Logger.warn "Could not create monitor config for #{job_class_name} with cron '#{cron_expression}'" + Sentry.configuration.sdk_logger.warn "[sentry-good_job] Could not create monitor config for #{job_class_name} with cron '#{cron_expression}'" return end end @@ -186,7 +186,7 @@ def self.add_monitoring_to_job(job_class, slug: nil, cron_expression: nil, timez monitor_config: monitor_config ) - Sentry::GoodJob::Logger.info "Added Sentry cron monitoring for #{job_class.name} (#{monitor_slug})" + Sentry.configuration.sdk_logger.info "[sentry-good_job] Added Sentry cron monitoring for #{job_class.name} (#{monitor_slug})" end end end diff --git a/sentry-good_job/lib/sentry/good_job/logger.rb b/sentry-good_job/lib/sentry/good_job/logger.rb deleted file mode 100644 index 1bb97516c..000000000 --- a/sentry-good_job/lib/sentry/good_job/logger.rb +++ /dev/null @@ -1,40 +0,0 @@ -# frozen_string_literal: true - -# Centralized logging utility for Sentry Good Job integration -module Sentry - module GoodJob - module Logger - def self.enabled? - ::Sentry.configuration.good_job.logging_enabled && logger.present? - end - - def self.logger - if ::Sentry.configuration.good_job.logger - ::Sentry.configuration.good_job.logger - elsif defined?(::Rails) && ::Rails.respond_to?(:logger) - ::Rails.logger - else - nil - end - end - - def self.info(message) - return unless enabled? - - logger.info(message) - end - - def self.warn(message) - return unless enabled? - - logger.warn(message) - end - - def self.error(message) - return unless enabled? - - logger.error(message) - end - end - end -end diff --git a/sentry-good_job/spec/sentry/good_job/configuration_spec.rb b/sentry-good_job/spec/sentry/good_job/configuration_spec.rb index a1bc0bfd0..b275708a4 100644 --- a/sentry-good_job/spec/sentry/good_job/configuration_spec.rb +++ b/sentry-good_job/spec/sentry/good_job/configuration_spec.rb @@ -12,8 +12,6 @@ describe "default values" do it "sets default values correctly" do expect(config.enable_cron_monitors).to eq(true) - expect(config.logging_enabled).to eq(false) - expect(config.logger).to be_nil end end @@ -38,15 +36,5 @@ expect(config.enable_cron_monitors).to eq(false) end - it "allows setting logging_enabled" do - config.logging_enabled = true - expect(config.logging_enabled).to eq(true) - end - - it "allows setting logger" do - logger = double("Logger") - config.logger = logger - expect(config.logger).to eq(logger) - end end end diff --git a/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb b/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb index d6217ab6e..836e6cdf5 100644 --- a/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb +++ b/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb @@ -29,12 +29,12 @@ it "handles parsing errors gracefully" do allow(Fugit).to receive(:parse_cron).and_raise(StandardError.new("Invalid cron")) - allow(Sentry::GoodJob::Logger).to receive(:warn) + allow(Sentry.configuration.sdk_logger).to receive(:warn) result = described_class::Helpers.monitor_config_from_cron("invalid") expect(result).to be_nil - expect(Sentry::GoodJob::Logger).to have_received(:warn) + expect(Sentry.configuration.sdk_logger).to have_received(:warn) end end @@ -195,11 +195,11 @@ it "logs the setup completion" do described_class::Integration.reset_setup_state! allow(described_class::Integration).to receive(:setup_monitoring_for_job).and_return("TestJob", "AnotherJob") - allow(Sentry::GoodJob::Logger).to receive(:info) + allow(Sentry.configuration.sdk_logger).to receive(:info) described_class::Integration.setup_monitoring_for_scheduled_jobs - expect(Sentry::GoodJob::Logger).to have_received(:info).with("Sentry cron monitoring setup for 2 scheduled jobs: TestJob, AnotherJob") + expect(Sentry.configuration.sdk_logger).to have_received(:info).with("[sentry-good_job] Sentry cron monitoring setup for 2 scheduled jobs: TestJob, AnotherJob") end end end @@ -216,13 +216,13 @@ let(:job_config) { { class: "NonExistentJob", cron: "0 * * * *" } } it "logs a warning and returns" do - allow(Sentry::GoodJob::Logger).to receive(:warn) + allow(Sentry.configuration.sdk_logger).to receive(:warn) # Mock Rails.application.config.after_initialize to execute immediately allow(Rails.application.config).to receive(:after_initialize).and_yield described_class::Integration.setup_monitoring_for_job("test_job", job_config) - expect(Sentry::GoodJob::Logger).to have_received(:warn).with(/Could not find job class/) + expect(Sentry.configuration.sdk_logger).to have_received(:warn).with(/Could not find job class/) end end @@ -311,11 +311,11 @@ it "logs the setup completion" do # JobMonitor removed - no setup needed allow(job_class).to receive(:sentry_monitor_check_ins) - allow(Sentry::GoodJob::Logger).to receive(:info) + allow(Sentry.configuration.sdk_logger).to receive(:info) described_class::Integration.add_monitoring_to_job(job_class) - expect(Sentry::GoodJob::Logger).to have_received(:info).with(/Added Sentry cron monitoring/) + expect(Sentry.configuration.sdk_logger).to have_received(:info).with(/Added Sentry cron monitoring/) end end end diff --git a/sentry-good_job/spec/sentry/good_job/logger_spec.rb b/sentry-good_job/spec/sentry/good_job/logger_spec.rb deleted file mode 100644 index f7ea67710..000000000 --- a/sentry-good_job/spec/sentry/good_job/logger_spec.rb +++ /dev/null @@ -1,161 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -RSpec.describe Sentry::GoodJob::Logger do - before do - perform_basic_setup - end - - let(:mock_logger) { double("Logger") } - - describe ".enabled?" do - context "when logging is disabled" do - before do - Sentry.configuration.good_job.logging_enabled = false - end - - it "returns false" do - expect(described_class.enabled?).to be false - end - end - - context "when logging is enabled but no logger is available" do - before do - Sentry.configuration.good_job.logging_enabled = true - Sentry.configuration.good_job.logger = nil - allow(described_class).to receive(:logger).and_return(nil) - end - - it "returns false" do - expect(described_class.enabled?).to be false - end - end - - context "when logging is enabled and logger is available" do - before do - Sentry.configuration.good_job.logging_enabled = true - allow(described_class).to receive(:logger).and_return(mock_logger) - end - - it "returns true" do - expect(described_class.enabled?).to be true - end - end - end - - describe ".logger" do - context "when custom logger is configured" do - before do - Sentry.configuration.good_job.logger = mock_logger - end - - it "returns the custom logger" do - expect(described_class.logger).to eq(mock_logger) - end - end - - context "when no custom logger is configured" do - before do - Sentry.configuration.good_job.logger = nil - end - - context "and Rails is available" do - before do - stub_const("Rails", double("Rails")) - allow(Rails).to receive(:respond_to?).with(:logger).and_return(true) - allow(Rails).to receive(:logger).and_return(mock_logger) - end - - it "returns Rails.logger" do - expect(described_class.logger).to eq(mock_logger) - end - end - - context "and Rails is not available" do - before do - hide_const("Rails") - end - - it "returns nil" do - expect(described_class.logger).to be_nil - end - end - end - end - - describe ".info" do - context "when logging is enabled" do - before do - Sentry.configuration.good_job.logging_enabled = true - allow(described_class).to receive(:logger).and_return(mock_logger) - end - - it "logs the message" do - expect(mock_logger).to receive(:info).with("test message") - described_class.info("test message") - end - end - - context "when logging is disabled" do - before do - Sentry.configuration.good_job.logging_enabled = false - end - - it "does not log the message" do - expect(mock_logger).not_to receive(:info) - described_class.info("test message") - end - end - end - - describe ".warn" do - context "when logging is enabled" do - before do - Sentry.configuration.good_job.logging_enabled = true - allow(described_class).to receive(:logger).and_return(mock_logger) - end - - it "logs the warning" do - expect(mock_logger).to receive(:warn).with("test warning") - described_class.warn("test warning") - end - end - - context "when logging is disabled" do - before do - Sentry.configuration.good_job.logging_enabled = false - end - - it "does not log the warning" do - expect(mock_logger).not_to receive(:warn) - described_class.warn("test warning") - end - end - end - - describe ".error" do - context "when logging is enabled" do - before do - Sentry.configuration.good_job.logging_enabled = true - allow(described_class).to receive(:logger).and_return(mock_logger) - end - - it "logs the error" do - expect(mock_logger).to receive(:error).with("test error") - described_class.error("test error") - end - end - - context "when logging is disabled" do - before do - Sentry.configuration.good_job.logging_enabled = false - end - - it "does not log the error" do - expect(mock_logger).not_to receive(:error) - described_class.error("test error") - end - end - end -end From 8a4e074ea151445a3f92be24b3a195161ec3c5d9 Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Fri, 17 Oct 2025 11:15:40 +0300 Subject: [PATCH 16/24] Enhance Sentry GoodJob integration with improved context and span data handling - Added checks to ensure the proper inclusion of the sentry-rails ActiveJobExtensions module. - Refactored span data handling to utilize a new private method for setting GoodJob-specific span data. - Updated context enhancement methods to streamline the integration with GoodJob attributes. - Removed outdated test files and added new integration tests to validate the functionality of GoodJob extensions. - Ensured compatibility with the latest changes in the Sentry and GoodJob integrations. --- .../sentry/good_job/active_job_extensions.rb | 42 ++--- .../good_job/active_job_extensions_spec.rb | 142 --------------- .../sentry/good_job/configuration_spec.rb | 1 - .../sentry/good_job/context_helpers_spec.rb | 114 ------------ .../spec/sentry/good_job/integration_spec.rb | 163 ++++++++++++++++++ sentry-rails/lib/sentry/rails/active_job.rb | 10 +- 6 files changed, 189 insertions(+), 283 deletions(-) delete mode 100644 sentry-good_job/spec/sentry/good_job/active_job_extensions_spec.rb delete mode 100644 sentry-good_job/spec/sentry/good_job/context_helpers_spec.rb create mode 100644 sentry-good_job/spec/sentry/good_job/integration_spec.rb diff --git a/sentry-good_job/lib/sentry/good_job/active_job_extensions.rb b/sentry-good_job/lib/sentry/good_job/active_job_extensions.rb index 116a1881b..a4c3260c1 100644 --- a/sentry-good_job/lib/sentry/good_job/active_job_extensions.rb +++ b/sentry-good_job/lib/sentry/good_job/active_job_extensions.rb @@ -74,6 +74,13 @@ def self.setup_good_job_extensions ActiveSupport.on_load(:active_job) do # Add GoodJob-specific attributes and methods include GoodJobExtensions + + # Ensure the sentry-rails integration is properly set up + # by checking if the ActiveJobExtensions module is already included + if defined?(::Sentry::Rails::ActiveJobExtensions) && !ancestors.include?(::Sentry::Rails::ActiveJobExtensions) + require "sentry/rails/active_job" + prepend ::Sentry::Rails::ActiveJobExtensions + end end end @@ -88,37 +95,30 @@ module GoodJobExtensions # Create enqueue span with GoodJob-specific data ::Sentry.with_child_span(op: "queue.publish", description: job.class.name) do |span| - set_span_data(span, job) + _sentry_set_span_data(span, job) block.call end end - # Set up around_perform hook for GoodJob-specific tags and span data - around_perform do |job, block| - next block.call unless ::Sentry.initialized? - - # Add GoodJob-specific tags to current scope - good_job_tags = { - queue_name: job.queue_name, - executions: job.executions - } - good_job_tags[:priority] = job.priority if job.respond_to?(:priority) - - Sentry.with_scope do |scope| - scope.set_tags(good_job_tags) - block.call - end - end + # GoodJob-specific context is now handled through the enhanced sentry_context method + # The sentry-rails integration handles error capturing through SentryReporter.record end private - # Override set_span_data to add GoodJob-specific functionality - def set_span_data(span, job, retry_count: nil) + # Override _sentry_set_span_data to add GoodJob-specific functionality + def _sentry_set_span_data(span, job, retry_count: nil) return unless span - # Call the base implementation - super(span, job, retry_count: retry_count) + # Call the base implementation if it exists (from sentry-rails) + if respond_to?(:_sentry_set_span_data, true) && method(:_sentry_set_span_data).super_method + super(span, job, retry_count: retry_count) + else + # Fallback: implement base functionality directly + span.set_data("messaging.message.id", job.job_id) + span.set_data("messaging.destination.name", job.queue_name) if job.respond_to?(:queue_name) + span.set_data("messaging.message.retry.count", retry_count) if retry_count + end # Add GoodJob-specific span data (latency) latency = calculate_job_latency(job) diff --git a/sentry-good_job/spec/sentry/good_job/active_job_extensions_spec.rb b/sentry-good_job/spec/sentry/good_job/active_job_extensions_spec.rb deleted file mode 100644 index 8aacc3f9f..000000000 --- a/sentry-good_job/spec/sentry/good_job/active_job_extensions_spec.rb +++ /dev/null @@ -1,142 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -RSpec.describe Sentry::GoodJob::ActiveJobExtensions do - let(:job) do - double("Job", - class: double("JobClass", name: "TestJob"), - job_id: "123", - provider_job_id: "provider_123", - queue_name: "default", - executions: 2, - enqueued_at: Time.now, - scheduled_at: Time.now + 1.hour, - priority: 5, - arguments: ["arg1", "arg2"], - locale: "en" - ) - end - - let(:base_context) { { active_job: "TestJob" } } - let(:base_tags) { { job_id: "123" } } - - before do - perform_basic_setup - end - - describe ".enhance_sentry_context" do - it "enhances sentry context with GoodJob-specific data" do - result = described_class.enhance_sentry_context(job, base_context) - - expect(result[:good_job]).to include( - queue_name: "default", - executions: 2, - enqueued_at: job.enqueued_at, - priority: 5 - ) - end - - it "preserves base context" do - result = described_class.enhance_sentry_context(job, base_context) - - expect(result[:active_job]).to eq("TestJob") - end - - it "returns base context when job doesn't respond to required methods" do - incomplete_job = double("IncompleteJob", job_id: "123") - result = described_class.enhance_sentry_context(incomplete_job, base_context) - - expect(result).to eq(base_context) - end - end - - describe ".enhance_sentry_tags" do - it "enhances sentry tags with GoodJob-specific data" do - result = described_class.enhance_sentry_tags(job, base_tags) - - expect(result).to include( - job_id: "123", - queue_name: "default", - executions: 2, - priority: 5 - ) - end - - it "preserves base tags" do - result = described_class.enhance_sentry_tags(job, base_tags) - - expect(result[:job_id]).to eq("123") - end - - it "returns base tags when job doesn't respond to required methods" do - incomplete_job = double("IncompleteJob", job_id: "123") - result = described_class.enhance_sentry_tags(incomplete_job, base_tags) - - expect(result).to eq(base_tags) - end - end - - describe ".setup" do - it "sets up GoodJob-specific ActiveJob extensions" do - # Mock Rails and Sentry to ensure conditions are met - stub_const("Rails", double("Rails")) - allow(Sentry).to receive(:initialized?).and_return(true) - - # Mock SentryReporter to be defined - stub_const("Sentry::Rails::ActiveJobExtensions::SentryReporter", Class.new) - - # Mock ActiveSupport.on_load to test it's called - allow(ActiveSupport).to receive(:on_load).with(:active_job).and_yield - - # Mock the private methods to test they are called - allow(described_class).to receive(:enhance_sentry_reporter) - allow(described_class).to receive(:setup_good_job_extensions) - - described_class.setup - - expect(described_class).to have_received(:enhance_sentry_reporter) - expect(described_class).to have_received(:setup_good_job_extensions) - end - end - - describe "GoodJobExtensions" do - let(:job_class) do - Class.new(ActiveJob::Base) do - include Sentry::GoodJob::ActiveJobExtensions::GoodJobExtensions - - def self.name - "TestJob" - end - - def perform - "test" - end - end - end - - let(:job) { job_class.new } - - before do - perform_basic_setup - end - - describe "around_enqueue" do - it "creates child span for enqueue with GoodJob-specific data" do - expect(Sentry).to receive(:with_child_span).with(op: "queue.publish", description: "TestJob") - job.enqueue - end - end - - describe "around_perform" do - it "adds GoodJob-specific tags during perform" do - scope = double("Scope") - allow(scope).to receive(:set_tags) - allow(scope).to receive(:span).and_return(nil) - - expect(Sentry).to receive(:with_scope).and_yield(scope) - job.perform_now - end - end - end -end diff --git a/sentry-good_job/spec/sentry/good_job/configuration_spec.rb b/sentry-good_job/spec/sentry/good_job/configuration_spec.rb index b275708a4..c1c8b92c1 100644 --- a/sentry-good_job/spec/sentry/good_job/configuration_spec.rb +++ b/sentry-good_job/spec/sentry/good_job/configuration_spec.rb @@ -35,6 +35,5 @@ config.enable_cron_monitors = false expect(config.enable_cron_monitors).to eq(false) end - end end diff --git a/sentry-good_job/spec/sentry/good_job/context_helpers_spec.rb b/sentry-good_job/spec/sentry/good_job/context_helpers_spec.rb deleted file mode 100644 index 1023d6e35..000000000 --- a/sentry-good_job/spec/sentry/good_job/context_helpers_spec.rb +++ /dev/null @@ -1,114 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -RSpec.describe Sentry::GoodJob::ContextHelpers do - let(:job) do - double("Job", - class: double("JobClass", name: "TestJob"), - job_id: "123", - provider_job_id: "provider_123", - queue_name: "default", - executions: 2, - enqueued_at: Time.now, - scheduled_at: Time.now + 1.hour, - priority: 5, - arguments: ["arg1", "arg2"], - locale: "en" - ) - end - - let(:incomplete_job) do - double("IncompleteJob", - class: double("JobClass", name: "TestJob"), - job_id: "123" - ) - end - - describe ".add_context" do - let(:base_context) { { existing: "context" } } - - it "adds GoodJob-specific context" do - result = described_class.add_context(job, base_context) - - expect(result[:good_job]).to include( - queue_name: "default", - executions: 2, - enqueued_at: job.enqueued_at, - priority: 5 - ) - end - - it "preserves base context" do - result = described_class.add_context(job, base_context) - - expect(result[:existing]).to eq("context") - end - - it "when job doesn't respond to required methods" do - result = described_class.add_context(incomplete_job, base_context) - - expect(result).to eq(base_context) - end - end - - describe ".add_tags" do - let(:base_tags) { { existing: "tag" } } - - it "adds GoodJob-specific tags" do - result = described_class.add_tags(job, base_tags) - - expect(result).to include( - queue_name: "default", - executions: 2, - priority: 5 - ) - end - - it "preserves base tags" do - result = described_class.add_tags(job, base_tags) - - expect(result[:existing]).to eq("tag") - end - - it "when job doesn't respond to required methods" do - result = described_class.add_tags(incomplete_job, base_tags) - - expect(result).to eq(base_tags) - end - end - - describe ".enhanced_context" do - it "returns enhanced context with both ActiveJob and GoodJob data" do - result = described_class.enhanced_context(job) - - expect(result[:active_job]).to include( - active_job: "TestJob", - job_id: "123", - provider_job_id: "provider_123", - locale: "en" - ) - - expect(result[:good_job]).to include( - queue_name: "default", - executions: 2, - enqueued_at: job.enqueued_at, - priority: 5 - ) - end - end - - describe ".enhanced_tags" do - it "returns enhanced tags with both ActiveJob and GoodJob data" do - result = described_class.enhanced_tags(job) - - expect(result).to include( - job_id: "123", - provider_job_id: "provider_123", - queue_name: "default", - executions: 2, - priority: 5 - ) - end - end -end diff --git a/sentry-good_job/spec/sentry/good_job/integration_spec.rb b/sentry-good_job/spec/sentry/good_job/integration_spec.rb new file mode 100644 index 000000000..3160cce88 --- /dev/null +++ b/sentry-good_job/spec/sentry/good_job/integration_spec.rb @@ -0,0 +1,163 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe "Sentry GoodJob Integration", type: :job do + before do + perform_basic_setup + # Use :inline adapter to get real GoodJob behavior with immediate execution + # This provides actual GoodJob attributes like queue_name, executions, priority, etc. + ActiveJob::Base.queue_adapter = :inline + + # Set up the GoodJob integration + Sentry::GoodJob.setup_good_job_integration + end + + let(:transport) do + Sentry.get_current_client.transport + end + + # Test job classes that will be used in integration tests + class TestGoodJob < ActiveJob::Base + def perform(message) + "Processed: #{message}" + end + end + + class FailingGoodJob < ActiveJob::Base + def perform(message) + raise StandardError, "Job failed: #{message}" + end + end + + class GoodJobWithContext < ActiveJob::Base + def perform(user_id) + Sentry.set_user(id: user_id) + Sentry.set_tags(job_type: "context_test") + raise StandardError, "Context test error" + end + end + + describe "GoodJob extensions integration" do + it "successfully executes jobs without errors" do + result = TestGoodJob.perform_now("test message") + expect(result).to eq("Processed: test message") + # No events should be captured for successful jobs + expect(transport.events.size).to eq(0) + end + + it "verifies GoodJob extensions are properly included" do + # Test that the GoodJob extensions are working by checking that + # the GoodJobExtensions module is included in ActiveJob::Base + expect(ActiveJob::Base.ancestors).to include(Sentry::GoodJob::ActiveJobExtensions::GoodJobExtensions) + end + + it "verifies GoodJob-specific methods are available" do + # Test that the GoodJob-specific methods are available + job = TestGoodJob.new("test") + + # The GoodJob extensions should add the _sentry_set_span_data method + # Note: This method is private, so we test it indirectly + expect(job.class.private_method_defined?(:_sentry_set_span_data)).to be true + end + + it "verifies GoodJob context helpers work correctly" do + # Test that the GoodJob context helpers work correctly + job = TestGoodJob.new("test") + + # Test the enhance_sentry_context method + base_context = { "active_job" => "TestGoodJob" } + enhanced_context = Sentry::GoodJob::ActiveJobExtensions.enhance_sentry_context(job, base_context) + + # The enhanced context should include GoodJob-specific data + expect(enhanced_context).to include(:good_job) + expect(enhanced_context[:good_job]).to include(:queue_name, :executions) + end + end + + describe "integration setup" do + it "sets up GoodJob extensions when integration is enabled" do + # This test verifies that the integration properly sets up the extensions + # by checking that the GoodJobExtensions module is included in ActiveJob::Base + expect(ActiveJob::Base.ancestors).to include(Sentry::GoodJob::ActiveJobExtensions::GoodJobExtensions) + end + + it "verifies GoodJob integration is properly configured" do + # Test that the GoodJob integration is properly configured + expect(Sentry.configuration.good_job).to be_present + expect(Sentry.configuration.good_job.enable_cron_monitors).to be true + end + end + + describe "GoodJob extensions functionality" do + # Test job classes that simulate real application jobs + class AppUserJob < ActiveJob::Base + def perform(user_id, action) + # Simulate user-related job processing + Sentry.set_user(id: user_id) + Sentry.set_tags(job_type: "user_processing", action: action) + + # Simulate some work + sleep(0.01) if action == "slow_processing" + + raise StandardError, "User processing failed: #{action}" if action == "fail" + + "User #{user_id} processed for #{action}" + end + end + + class AppDataJob < ActiveJob::Base + def perform(data, count) + # Simulate data processing job + Sentry.set_tags(job_type: "data_processing", data_size: data.length, count: count) + + # Simulate some work + sleep(0.01) if count > 2 + + raise StandardError, "Data processing failed: #{data}" if data == "fail" + + "Processed #{count} items of data: #{data}" + end + end + + it "verifies GoodJob extensions work with different job types" do + # Test that the GoodJob extensions work with different job types + user_job = AppUserJob.new("user1", "update") + data_job = AppDataJob.new("test", 3) + + # Both jobs should have the GoodJob extensions + expect(user_job.class.ancestors).to include(Sentry::GoodJob::ActiveJobExtensions::GoodJobExtensions) + expect(data_job.class.ancestors).to include(Sentry::GoodJob::ActiveJobExtensions::GoodJobExtensions) + end + + it "verifies GoodJob context enhancement works correctly" do + # Test that the GoodJob context enhancement works correctly + job = AppUserJob.new("user1", "update") + + # Test the enhance_sentry_context method + base_context = { "active_job" => "AppUserJob" } + enhanced_context = Sentry::GoodJob::ActiveJobExtensions.enhance_sentry_context(job, base_context) + + # The enhanced context should include GoodJob-specific data + expect(enhanced_context).to include(:good_job) + expect(enhanced_context[:good_job]).to include(:queue_name, :executions) + expect(enhanced_context[:good_job][:queue_name]).to eq("default") + expect(enhanced_context[:good_job][:executions]).to eq(0) + end + + it "verifies GoodJob span data methods work correctly" do + # Test that the GoodJob span data methods work correctly + job = AppUserJob.new("user1", "update") + + # Create a mock span that expects the set_data calls + span = double("span") + allow(span).to receive(:set_data) + + # Test that the private method exists and can be called via send + expect { job.send(:_sentry_set_span_data, span, job) }.not_to raise_error + + # Test that the method is properly defined + expect(job.class.private_method_defined?(:_sentry_set_span_data)).to be true + end + end +end diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index f39eee9d6..b97ab0f0f 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -60,7 +60,7 @@ def deserialize(job_data) end # Set span data with messaging semantics - def set_span_data(span, job, retry_count: nil) + def _sentry_set_span_data(span, job, retry_count: nil) return unless span span.set_data("messaging.message.id", job.job_id) @@ -69,7 +69,7 @@ def set_span_data(span, job, retry_count: nil) end # Start transaction with trace propagation - def start_transaction(scope, trace_headers) + def _sentry_start_transaction(scope, trace_headers) options = { name: scope.transaction_name, source: scope.transaction_source, @@ -82,7 +82,7 @@ def start_transaction(scope, trace_headers) end # Finish transaction with proper status - def finish_transaction(transaction, status) + def _sentry_finish_transaction(transaction, status) return unless transaction transaction.set_http_status(status) @@ -115,7 +115,7 @@ def record(job, &block) transaction = nil unless job.is_a?(::Sentry::SendEventJob) if job._sentry && job._sentry["trace_propagation_headers"] - transaction = job.start_transaction(scope, job._sentry["trace_propagation_headers"]) + transaction = job._sentry_start_transaction(scope, job._sentry["trace_propagation_headers"]) else transaction = Sentry.start_transaction( name: scope.transaction_name, @@ -135,7 +135,7 @@ def record(job, &block) else 0 end - job.set_span_data(transaction, job, retry_count: retry_count) + job._sentry_set_span_data(transaction, job, retry_count: retry_count) end yield.tap do From beb09273e4aab52b99616b8fd7efc0309902a297 Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Fri, 17 Oct 2025 11:36:08 +0300 Subject: [PATCH 17/24] Update README.md for Sentry GoodJob integration - Added dependencies for `sentry-rails` and `good_job` in the installation instructions. - Enhanced configuration section with optional logging setup for debugging. - Updated usage examples to reflect the new `sentry_monitor_check_ins` method for cron job monitoring. - Clarified documentation regarding the integration's automatic setup and logging capabilities. --- sentry-good_job/README.md | 42 ++++++++++++--------------------------- 1 file changed, 13 insertions(+), 29 deletions(-) diff --git a/sentry-good_job/README.md b/sentry-good_job/README.md index 4535b088f..9b4172640 100644 --- a/sentry-good_job/README.md +++ b/sentry-good_job/README.md @@ -25,6 +25,8 @@ The official Ruby-language client and integration layer for the [Sentry](https:/ ```ruby gem "sentry-ruby" +gem "sentry-rails" +gem "good_job" gem "sentry-good_job" ``` @@ -52,7 +54,10 @@ Sentry.init do |config| # ActiveJob configuration (handled by sentry-rails) config.rails.active_job_report_on_retry_error = false - config.send_default_pii = false # Controls job arguments inclusion + config.send_default_pii = false + + # Optional: Configure logging for debugging + config.sdk_logger = Rails.logger end ``` @@ -66,20 +71,12 @@ end - `config.rails.active_job_report_on_retry_error` (default: `false`): Only report errors after all retry attempts are exhausted - `config.send_default_pii` (default: `false`): Include job arguments in error context (be careful with sensitive data) +- `config.sdk_logger` (default: `nil`): Configure the SDK logger for custom logging needs (general Sentry configuration) **Note**: The Good Job integration now leverages sentry-rails for core ActiveJob functionality, including trace propagation, user context preservation, and error reporting. This provides better integration and reduces duplication. ## Usage -### Basic Setup - -The integration works automatically once installed. It will: - -1. Capture exceptions from ActiveJob workers -2. Set up performance monitoring for job execution -3. Automatically configure cron monitoring for scheduled jobs -4. Preserve user context and trace propagation - ### Automatic Setup The integration works automatically once installed. It will: @@ -108,7 +105,12 @@ You can also manually set up cron monitoring: ```ruby class MyScheduledJob < ApplicationJob - sentry_cron_monitor "0 * * * *", timezone: "UTC" + include Sentry::Cron::MonitorCheckIns + + sentry_monitor_check_ins( + slug: "my_scheduled_job", + monitor_config: Sentry::Cron::MonitorConfig.from_crontab("0 * * * *", timezone: "UTC") + ) end ``` @@ -145,24 +147,6 @@ Sentry.init do |config| end ``` -#### Custom Logger Configuration - -For more advanced logging needs, you can provide a custom logger: - -```ruby -# Custom logger with specific formatting -custom_logger = Logger.new('log/sentry-good_job.log') -custom_logger.formatter = proc do |severity, datetime, progname, msg| - "[#{datetime}] #{severity}: #{msg}\n" -end - -Sentry.init do |config| - config.dsn = 'your-dsn-here' - config.sdk_logger = custom_logger - config.sdk_logger.level = Logger::INFO -end -``` - #### Log Levels The integration logs at different levels: From 8ff00238c76774feef4f08d74febc2c843ea544a Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Fri, 17 Oct 2025 14:33:33 +0300 Subject: [PATCH 18/24] Rubocop cleanup --- sentry-good_job/lib/sentry/good_job/cron_helpers.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry-good_job/lib/sentry/good_job/cron_helpers.rb b/sentry-good_job/lib/sentry/good_job/cron_helpers.rb index 50ef49f73..92727629a 100644 --- a/sentry-good_job/lib/sentry/good_job/cron_helpers.rb +++ b/sentry-good_job/lib/sentry/good_job/cron_helpers.rb @@ -138,10 +138,10 @@ def self.setup_monitoring_for_job(cron_key, job_config) monitor_config: monitor_config ) - return job_class_name + job_class_name else Sentry.configuration.sdk_logger.warn "[sentry-good_job] Could not create monitor config for #{job_class_name} with cron '#{cron_expression}'" - return + nil end end From 471e61352cd8372dcc0fb9fd440ea00782f7fc6f Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Fri, 17 Oct 2025 14:33:45 +0300 Subject: [PATCH 19/24] Enhance user context preservation in Sentry integrations - Added tests to verify that user context is preserved after job execution, both for successful and failed jobs, in both GoodJob and ActiveJob integrations. - Updated the Sentry ActiveJob integration to store and restore the current user context during job execution, ensuring consistency across job processing. - Improved integration tests to validate the correct behavior of user context handling in various scenarios. --- .../spec/sentry/good_job/integration_spec.rb | 33 +++++++++++++++++++ sentry-rails/lib/sentry/rails/active_job.rb | 10 ++++++ .../spec/sentry/rails/activejob_spec.rb | 32 ++++++++++++++++++ 3 files changed, 75 insertions(+) diff --git a/sentry-good_job/spec/sentry/good_job/integration_spec.rb b/sentry-good_job/spec/sentry/good_job/integration_spec.rb index 3160cce88..e5094fe2a 100644 --- a/sentry-good_job/spec/sentry/good_job/integration_spec.rb +++ b/sentry-good_job/spec/sentry/good_job/integration_spec.rb @@ -73,6 +73,39 @@ def perform(user_id) expect(enhanced_context).to include(:good_job) expect(enhanced_context[:good_job]).to include(:queue_name, :executions) end + + it "preserves Current.user context after job execution" do + # Set user context before job execution + Sentry.configure_scope do |scope| + scope.set_user(id: "123", email: "test@example.com") + end + + # Verify user context is set + expect(Sentry.get_current_scope.user).to eq({ id: "123", email: "test@example.com" }) + + # Execute a job + result = TestGoodJob.perform_now("test message") + expect(result).to eq("Processed: test message") + + # Verify user context is still preserved after job execution + expect(Sentry.get_current_scope.user).to eq({ id: "123", email: "test@example.com" }) + end + + it "preserves Current.user context after job execution with error" do + # Set user context before job execution + Sentry.configure_scope do |scope| + scope.set_user(id: "456", email: "error@example.com") + end + + # Verify user context is set + expect(Sentry.get_current_scope.user).to eq({ id: "456", email: "error@example.com" }) + + # Execute a job that raises an error + expect { FailingGoodJob.perform_now("error message") }.to raise_error(StandardError, "Job failed: error message") + + # Verify user context is still preserved after job execution with error + expect(Sentry.get_current_scope.user).to eq({ id: "456", email: "error@example.com" }) + end end describe "integration setup" do diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index b97ab0f0f..75bf91a98 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -102,6 +102,9 @@ class SentryReporter class << self def record(job, &block) + # Store the current user context before creating the temporary scope + current_user = Sentry.get_current_scope&.user + Sentry.with_scope do |scope| begin scope.set_transaction_name(job.class.name, source: :task) @@ -149,6 +152,13 @@ def record(job, &block) raise end end + + # Restore the user context to the current thread's scope after job execution + if current_user.present? + Sentry.configure_scope do |scope| + scope.set_user(current_user) + end + end end def capture_exception(job, e) diff --git a/sentry-rails/spec/sentry/rails/activejob_spec.rb b/sentry-rails/spec/sentry/rails/activejob_spec.rb index 8431bffff..38977fac1 100644 --- a/sentry-rails/spec/sentry/rails/activejob_spec.rb +++ b/sentry-rails/spec/sentry/rails/activejob_spec.rb @@ -160,6 +160,38 @@ def post.to_global_id expect(Sentry.get_current_scope.extra).to eq({}) end + it "preserves user context after job execution" do + # Set user context before job execution + Sentry.configure_scope do |scope| + scope.set_user(id: "123", email: "test@example.com") + end + + # Verify user context is set + expect(Sentry.get_current_scope.user).to eq({ id: "123", email: "test@example.com" }) + + # Execute a job + NormalJob.perform_now + + # Verify user context is still preserved after job execution + expect(Sentry.get_current_scope.user).to eq({ id: "123", email: "test@example.com" }) + end + + it "preserves user context after job execution with error" do + # Set user context before job execution + Sentry.configure_scope do |scope| + scope.set_user(id: "456", email: "error@example.com") + end + + # Verify user context is set + expect(Sentry.get_current_scope.user).to eq({ id: "456", email: "error@example.com" }) + + # Execute a job that raises an error + expect { FailedJob.perform_now }.to raise_error(FailedJob::TestError) + + # Verify user context is still preserved after job execution with error + expect(Sentry.get_current_scope.user).to eq({ id: "456", email: "error@example.com" }) + end + context "with tracing enabled" do before do make_basic_app do |config| From 3137ce514bf01b74f04ef7e0bbd4bf73e730b9a9 Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Fri, 17 Oct 2025 14:56:56 +0300 Subject: [PATCH 20/24] Remove redundant user context preservation tests from Sentry integrations - Deleted tests that verify user context preservation after job execution in both GoodJob and ActiveJob integrations, as the functionality is already covered by existing tests. - Cleaned up the Sentry ActiveJob integration by removing unnecessary user context handling code, streamlining the job execution process. --- .../spec/sentry/good_job/integration_spec.rb | 32 ------------------- sentry-rails/lib/sentry/rails/active_job.rb | 10 ------ .../spec/sentry/rails/activejob_spec.rb | 31 ------------------ 3 files changed, 73 deletions(-) diff --git a/sentry-good_job/spec/sentry/good_job/integration_spec.rb b/sentry-good_job/spec/sentry/good_job/integration_spec.rb index e5094fe2a..59bc5cce4 100644 --- a/sentry-good_job/spec/sentry/good_job/integration_spec.rb +++ b/sentry-good_job/spec/sentry/good_job/integration_spec.rb @@ -74,38 +74,6 @@ def perform(user_id) expect(enhanced_context[:good_job]).to include(:queue_name, :executions) end - it "preserves Current.user context after job execution" do - # Set user context before job execution - Sentry.configure_scope do |scope| - scope.set_user(id: "123", email: "test@example.com") - end - - # Verify user context is set - expect(Sentry.get_current_scope.user).to eq({ id: "123", email: "test@example.com" }) - - # Execute a job - result = TestGoodJob.perform_now("test message") - expect(result).to eq("Processed: test message") - - # Verify user context is still preserved after job execution - expect(Sentry.get_current_scope.user).to eq({ id: "123", email: "test@example.com" }) - end - - it "preserves Current.user context after job execution with error" do - # Set user context before job execution - Sentry.configure_scope do |scope| - scope.set_user(id: "456", email: "error@example.com") - end - - # Verify user context is set - expect(Sentry.get_current_scope.user).to eq({ id: "456", email: "error@example.com" }) - - # Execute a job that raises an error - expect { FailingGoodJob.perform_now("error message") }.to raise_error(StandardError, "Job failed: error message") - - # Verify user context is still preserved after job execution with error - expect(Sentry.get_current_scope.user).to eq({ id: "456", email: "error@example.com" }) - end end describe "integration setup" do diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index 75bf91a98..b97ab0f0f 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -102,9 +102,6 @@ class SentryReporter class << self def record(job, &block) - # Store the current user context before creating the temporary scope - current_user = Sentry.get_current_scope&.user - Sentry.with_scope do |scope| begin scope.set_transaction_name(job.class.name, source: :task) @@ -152,13 +149,6 @@ def record(job, &block) raise end end - - # Restore the user context to the current thread's scope after job execution - if current_user.present? - Sentry.configure_scope do |scope| - scope.set_user(current_user) - end - end end def capture_exception(job, e) diff --git a/sentry-rails/spec/sentry/rails/activejob_spec.rb b/sentry-rails/spec/sentry/rails/activejob_spec.rb index 38977fac1..e0904cae2 100644 --- a/sentry-rails/spec/sentry/rails/activejob_spec.rb +++ b/sentry-rails/spec/sentry/rails/activejob_spec.rb @@ -160,37 +160,6 @@ def post.to_global_id expect(Sentry.get_current_scope.extra).to eq({}) end - it "preserves user context after job execution" do - # Set user context before job execution - Sentry.configure_scope do |scope| - scope.set_user(id: "123", email: "test@example.com") - end - - # Verify user context is set - expect(Sentry.get_current_scope.user).to eq({ id: "123", email: "test@example.com" }) - - # Execute a job - NormalJob.perform_now - - # Verify user context is still preserved after job execution - expect(Sentry.get_current_scope.user).to eq({ id: "123", email: "test@example.com" }) - end - - it "preserves user context after job execution with error" do - # Set user context before job execution - Sentry.configure_scope do |scope| - scope.set_user(id: "456", email: "error@example.com") - end - - # Verify user context is set - expect(Sentry.get_current_scope.user).to eq({ id: "456", email: "error@example.com" }) - - # Execute a job that raises an error - expect { FailedJob.perform_now }.to raise_error(FailedJob::TestError) - - # Verify user context is still preserved after job execution with error - expect(Sentry.get_current_scope.user).to eq({ id: "456", email: "error@example.com" }) - end context "with tracing enabled" do before do From 91b022b648c940f16fce5a5f92941148a0487f38 Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Tue, 4 Nov 2025 14:58:35 +0200 Subject: [PATCH 21/24] Refactor logging messages in Sentry Good Job integration to remove namespace prefix. This change simplifies log output for cron monitoring and job initialization, enhancing readability in logs. --- sentry-good_job/lib/sentry-good_job.rb | 2 +- sentry-good_job/lib/sentry/good_job/cron_helpers.rb | 12 ++++++------ .../spec/sentry/good_job/cron_helpers_spec.rb | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/sentry-good_job/lib/sentry-good_job.rb b/sentry-good_job/lib/sentry-good_job.rb index 3cfa7cd35..d06c2daa4 100644 --- a/sentry-good_job/lib/sentry-good_job.rb +++ b/sentry-good_job/lib/sentry-good_job.rb @@ -37,7 +37,7 @@ def self.setup_good_job_integration Sentry::GoodJob::CronHelpers::Integration.setup_monitoring_for_scheduled_jobs end - Sentry.configuration.sdk_logger.info "[sentry-good_job] Sentry Good Job integration initialized automatically" + Sentry.configuration.sdk_logger.info "Sentry Good Job integration initialized automatically" end # Delegate capture_exception so internal components can be tested in isolation diff --git a/sentry-good_job/lib/sentry/good_job/cron_helpers.rb b/sentry-good_job/lib/sentry/good_job/cron_helpers.rb index 92727629a..490ef2d78 100644 --- a/sentry-good_job/lib/sentry/good_job/cron_helpers.rb +++ b/sentry-good_job/lib/sentry/good_job/cron_helpers.rb @@ -25,7 +25,7 @@ def self.monitor_config_from_cron(cron_expression, timezone: nil) ::Sentry::Cron::MonitorConfig.from_crontab(cron_expression) end rescue => e - Sentry.configuration.sdk_logger.warn "[sentry-good_job] Failed to parse cron expression '#{cron_expression}': #{e.message}" + Sentry.configuration.sdk_logger.warn "Failed to parse cron expression '#{cron_expression}': #{e.message}" nil end @@ -87,9 +87,9 @@ def self.setup_monitoring_for_scheduled_jobs @setup_completed = true if added_jobs.any? job_list = added_jobs.join(", ") - Sentry.configuration.sdk_logger.info "[sentry-good_job] Sentry cron monitoring setup for #{added_jobs.size} scheduled jobs: #{job_list}" + Sentry.configuration.sdk_logger.info "Sentry cron monitoring setup for #{added_jobs.size} scheduled jobs: #{job_list}" else - Sentry.configuration.sdk_logger.info "[sentry-good_job] Sentry cron monitoring setup for #{cron_config.keys.size} scheduled jobs" + Sentry.configuration.sdk_logger.info "Sentry cron monitoring setup for #{cron_config.keys.size} scheduled jobs" end end @@ -115,7 +115,7 @@ def self.setup_monitoring_for_job(cron_key, job_config) job_class = begin job_class_name.constantize rescue NameError => e - Sentry.configuration.sdk_logger.warn "[sentry-good_job] Could not find job class '#{job_class_name}' for Sentry cron monitoring: #{e.message}" + Sentry.configuration.sdk_logger.warn "Could not find job class '#{job_class_name}' for Sentry cron monitoring: #{e.message}" return end @@ -140,7 +140,7 @@ def self.setup_monitoring_for_job(cron_key, job_config) job_class_name else - Sentry.configuration.sdk_logger.warn "[sentry-good_job] Could not create monitor config for #{job_class_name} with cron '#{cron_expression}'" + Sentry.configuration.sdk_logger.warn "Could not create monitor config for #{job_class_name} with cron '#{cron_expression}'" nil end end @@ -186,7 +186,7 @@ def self.add_monitoring_to_job(job_class, slug: nil, cron_expression: nil, timez monitor_config: monitor_config ) - Sentry.configuration.sdk_logger.info "[sentry-good_job] Added Sentry cron monitoring for #{job_class.name} (#{monitor_slug})" + Sentry.configuration.sdk_logger.info "Added Sentry cron monitoring for #{job_class.name} (#{monitor_slug})" end end end diff --git a/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb b/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb index 836e6cdf5..a92e9e983 100644 --- a/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb +++ b/sentry-good_job/spec/sentry/good_job/cron_helpers_spec.rb @@ -199,7 +199,7 @@ described_class::Integration.setup_monitoring_for_scheduled_jobs - expect(Sentry.configuration.sdk_logger).to have_received(:info).with("[sentry-good_job] Sentry cron monitoring setup for 2 scheduled jobs: TestJob, AnotherJob") + expect(Sentry.configuration.sdk_logger).to have_received(:info).with("Sentry cron monitoring setup for 2 scheduled jobs: TestJob, AnotherJob") end end end From 712df33a4850bf9a9fbb579a16e7016d2ea03732 Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Mon, 10 Nov 2025 08:36:09 +0200 Subject: [PATCH 22/24] Update dependency for sentry-ruby to version 6.1.0 in the sentry-good_job gemspec file. --- sentry-good_job/sentry-good_job.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-good_job/sentry-good_job.gemspec b/sentry-good_job/sentry-good_job.gemspec index 86b7432bf..d0c2aa21b 100644 --- a/sentry-good_job/sentry-good_job.gemspec +++ b/sentry-good_job/sentry-good_job.gemspec @@ -30,6 +30,6 @@ Gem::Specification.new do |spec| spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } spec.require_paths = ["lib"] - spec.add_dependency "sentry-ruby", "~> 5.28.0" + spec.add_dependency "sentry-ruby", "~> 6.1.0" spec.add_dependency "good_job", ">= 3.0" end From 2d5dbdd7d1df87c4c773cc27ec46fe0f6c342d6d Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Tue, 11 Nov 2025 08:55:40 +0200 Subject: [PATCH 23/24] Apply rubocop fixes --- sentry-good_job/bin/console | 1 + sentry-good_job/bin/setup | 2 ++ sentry-good_job/spec/sentry/good_job/integration_spec.rb | 1 - 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/sentry-good_job/bin/console b/sentry-good_job/bin/console index 05657330f..e1d4256d0 100755 --- a/sentry-good_job/bin/console +++ b/sentry-good_job/bin/console @@ -1,4 +1,5 @@ #!/usr/bin/env ruby +# frozen_string_literal: true require "bundler/setup" require "sentry-good_job" diff --git a/sentry-good_job/bin/setup b/sentry-good_job/bin/setup index bffaa8904..14cfaf024 100755 --- a/sentry-good_job/bin/setup +++ b/sentry-good_job/bin/setup @@ -1,4 +1,6 @@ #!/usr/bin/env ruby +# frozen_string_literal: true + require "fileutils" # path to your application root. diff --git a/sentry-good_job/spec/sentry/good_job/integration_spec.rb b/sentry-good_job/spec/sentry/good_job/integration_spec.rb index 59bc5cce4..3160cce88 100644 --- a/sentry-good_job/spec/sentry/good_job/integration_spec.rb +++ b/sentry-good_job/spec/sentry/good_job/integration_spec.rb @@ -73,7 +73,6 @@ def perform(user_id) expect(enhanced_context).to include(:good_job) expect(enhanced_context[:good_job]).to include(:queue_name, :executions) end - end describe "integration setup" do From 22a6a6e1178c1a5c101579fdfac618a4e6f56dc6 Mon Sep 17 00:00:00 2001 From: Andrei Makarov Date: Tue, 11 Nov 2025 08:58:59 +0200 Subject: [PATCH 24/24] Update CHANGELOG to include sentry-good_job integration feature --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b99e80414..40135364a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## Unreleased + +### Features + +- Introduce sentry-good_job integration ([#2751](https://github.com/getsentry/sentry-ruby/pull/2751)) + ## 6.1.0 ### Features