Skip to content

Migrate from sentry-raven to sentry-ruby#8818

Merged
abdulapopoola merged 9 commits intomainfrom
jamiemagee/sentry-ruby
Jan 23, 2024
Merged

Migrate from sentry-raven to sentry-ruby#8818
abdulapopoola merged 9 commits intomainfrom
jamiemagee/sentry-ruby

Conversation

@JamieMagee
Copy link
Copy Markdown
Member

@JamieMagee JamieMagee commented Jan 17, 2024

This migrates from the deprecated sentry-raven gem to the replacement sentry-ruby gem. sentry-raven was deprecated in March 2021.

The changes mostly follow the Sentry migration guide. The largest changes were around event processors. Previously, sentry-raven allowed you to inherit from Raven::Processor and pass it to processors during configuration. In sentry-ruby that has been replaced with before_send which accepts a single lambda. I've recreated the same behaviour by using a reducer to apply processors consecutively. See the "scrubbing sensitive data" and "Filtering for Ruby" documentation for more information.

Once this is merged, it opens up the possibility of using the sentry-opentelemetry gem as well.

@JamieMagee JamieMagee force-pushed the jamiemagee/sentry-ruby branch from cf68e11 to 970d210 Compare January 17, 2024 19:29
@deivid-rodriguez
Copy link
Copy Markdown
Contributor

Awesome, it was about time! See #7553 for previous art. This PR can supersede that one but maybe you find something useful in the code/comments there!

@JamieMagee JamieMagee force-pushed the jamiemagee/sentry-ruby branch 11 times, most recently from 9c2a457 to fe8e3f2 Compare January 18, 2024 19:51
@JamieMagee JamieMagee force-pushed the jamiemagee/sentry-ruby branch 2 times, most recently from ce2bfe8 to ca42235 Compare January 18, 2024 23:10
Comment on lines 18 to 22
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This prevents Sentry adding Sentry-Trace and Baggage headers by default to all outgoing requests. It's designed to make it easier to do distributed tracing between microservices or frontend/backend. As we don't have that architecture, and it currently breaks our smoke test cache, I've chosen to disable it by default.

If we want to get this distributed trace information, we'll need to ignore these headers in our updater proxy.

References:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think disabling is the right call as Dependabot makes most calls to external endpoints that don't care about our tracing.

Comment on lines 106 to 116
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member Author

@JamieMagee JamieMagee Jan 18, 2024

Choose a reason for hiding this comment

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

tapioca gem sentry-ruby generates an empty file, so this is minimal handwritten types for our uses.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

config.processors has been removed in favour of a lambda before_send. I've replicated the behaviour by using a reducer to call processors in order.

I had to migrate raven_context to a custom processor as well, because raven_context is no longer attached to events by default.

@JamieMagee JamieMagee force-pushed the jamiemagee/sentry-ruby branch from ca42235 to 85251aa Compare January 18, 2024 23:25
@JamieMagee JamieMagee marked this pull request as ready for review January 18, 2024 23:27
@JamieMagee JamieMagee requested a review from a team as a code owner January 18, 2024 23:27
Copy link
Copy Markdown
Contributor

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

Fantastic job! ❤️

@jeffwidman
Copy link
Copy Markdown
Member

jeffwidman commented Jan 23, 2024

Thanks @JamieMagee for tackling this, one less hanging TODO that I feel bad about not having the time to finish 😊 plus I was always annoyed that Dependabot was relying on an EOL'd library.

Reading through this PR was a lot of fun 🎉 because it answered several of my questions from #7553.

In case you didn't see this note from my PR:

Events now default to sending asynchronously using a thread pool whereas raven used to send them synchronously.

I suspect this change is harmless, but still, might be worth double-checking when this gets deployed, just in case. More details here, including how to preserve the old behavior through configuration if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants