Skip to content

Add Integrable module#1177

Merged
st0012 merged 5 commits intomasterfrom
integrable
Dec 30, 2020
Merged

Add Integrable module#1177
st0012 merged 5 commits intomasterfrom
integrable

Conversation

@st0012
Copy link
Copy Markdown
Contributor

@st0012 st0012 commented Dec 29, 2020

One of the goals of the new SDK is to provide clear documentation + convenient helpers to let communities develop their own extensions around the SDK. And the Integrable module added in this PR is our first step to make that happen.

The integrable module provides a simple DSL for registering the extension. Let me use sentry-rails as an example:

module Sentry
  module Rails
    extend Integrable
    register_integration name: "rails", version: Sentry::Rails::VERSION
  end
end

Once the extension is registered, 2 things will happen:

  1. It generates Sentry::Rails.capture_exception and Sentry::Rails.capture_message methods.
  2. It also generates the SDK meta for the extension, {name: "sentry.ruby.rails", version: Sentry::Rails::VERSION} in this case.

Then in the extension gem, all the reporting should be done via the newly generated helpers. This is because:

  • Those helpers will inject { integration: "integration_name" } to the event hints. So the users can later identify the source of each event.
  • Events created from those helpers will have the integration meta as their SDK information.
  • In the future, we might also introduce more advanced integration-based features that need to be triggered by these helpers.

This change also fixes the current SDK-info overridden issue, which is when using sentry-rails and sentry-sidekiq are used together, the SDK info will be decided by the order of the gem definitions in the Gemfile instead of where is the event being created.

But besides the above bug fix, this change shouldn't affect normal users.

@st0012 st0012 added this to the 4.1.2 milestone Dec 29, 2020
@st0012 st0012 self-assigned this Dec 29, 2020
@st0012 st0012 requested a review from HazAT December 29, 2020 16:30
@codecov-io
Copy link
Copy Markdown

codecov-io commented Dec 29, 2020

Codecov Report

❌ Patch coverage is 97.89474% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.52%. Comparing base (f3bc6a9) to head (4804bbf).
⚠️ Report is 1145 commits behind head on master.

Files with missing lines Patch % Lines
...s/lib/sentry/rails/overrides/streaming_reporter.rb 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1177      +/-   ##
==========================================
+ Coverage   97.93%   98.52%   +0.59%     
==========================================
  Files         190       97      -93     
  Lines        7973     4211    -3762     
==========================================
- Hits         7808     4149    -3659     
+ Misses        165       62     -103     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@st0012 st0012 merged commit c2bff60 into master Dec 30, 2020
@st0012 st0012 deleted the integrable branch December 30, 2020 13:55
agrobbin added a commit to agrobbin/sentry-ruby that referenced this pull request Jan 26, 2021
This was replaced in getsentry#1177, but the middleware wasn't actually removed.
st0012 pushed a commit that referenced this pull request Jan 29, 2021
…1238)

* remove now-unused Sidekiq CleanupMiddleware

This was replaced in #1177, but the middleware wasn't actually removed.

* extract wrapped Sidekiq job class names in both Sidekiq integrations

The Active Job `wrapped` job class name logic was only happening in the lower-level `ErrorHandler`, and was actually being ignored since `transaction_name` was already set via `SentryContextMiddleware`.

Now, the logic to properly extract the `transaction_name` has been refactored into `ContextFilter`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants