Bringing Sentry to your Pull Requests

[Dhrumil-Sentry]

Developers leverage Sentry for triage in one of two ways:

1. Reactive: Viewing issues when Sentry notifies them.
2. Investigative: Periodic/Ad-hoc triage sessions to review recent Sentry issues.

In both scenarios, the developer notes important issues, creates a Jira/Linear ticket for them, and adds this to their to-do list. Minor bugs often fall through the cracks in such a workflow.

We believe Sentry can better serve developers by enabling a more proactive, context-specific triage workflow by becoming part of the development process. Specifically, by surfacing insights within Pull Requests (PRs), Sentry can help developers spot, consider, and address issues more effectively.

PRs offer a critical point in a developer's workflow, where proposed code changes undergo scrutiny and present an opportunity to catch potential problems before they reach the main codebase.

Sentry in Pull Requests

Typically, a PR goes through these stages

1. Review and Discussion: Teams review the PR, hold discussions, and collaboratively enhance the code.
2. Test Coverage and Quality Checks: Automated checks ensure the PR meets certain standards and has sufficient test coverage.
3. Merge and Post-Merge Monitoring: After approval and merging, developers monitor the application to understand the impact of their changes in production.

By surfacing context about existing issues, Sentry can enable a proactive workflow that continuously improves code quality.

Improving PR Reviews with Sentry

Sentry can analyze open PRs and comment on any existing issues related to the areas of code being modified. This approach alerts developers and reviewers to potential pitfalls and helps them resolve existing issues while they're working on the same areas of code.

Using Codecov with Sentry

When developers utilize both Sentry and Codecov, they can identify untested code and existing errors in the functions they're updating. Developers can then use this information to expand their test coverage.

Post-Merge Monitoring

Even with comprehensive testing and review, sometimes code changes can introduce errors that only appear in the live production environment. Sentry can track these code changes' impacts right back to the PR.

Using Suspect Commits, Sentry can identify issues likely caused by a merged PR and comment with a summary of these new issues. This approach provides a consolidated record for developers investigating incidents or bugs.

We want your feedback

We’d love to hear what matters to you. Please share your thoughts in the comments below:

• Would more context in PR comments help you?
• What type of insights would you like to see in your PRs?
• How can Sentry better support a proactive triage process in your workflow?

[cquanu]

Where can we set this up?

[Dhrumil-Sentry]

Hi @cquanu we're still currently building this- We hope to make it available to EA customers in a few weeks. There will be no setup needed for this if you have installed the GitHub integration for Sentry

Please let us know if there's any other information you'd like to see on the PR

[bruno-garcia]

I see the toggle on /settings/integrations/github/ and it's on by default:

[shahsagar]

Would be great if Sentry could even let me know about what potential issues were resolved by my PR

[Dhrumil-Sentry]

Thanks for the feedback! Would certainly be great to tell developers if any of the issues in their code areas were resolved by the PR

[guru-kini]

Sounds promising. Does this need the git codebase to be copied over to Sentry/CodeCov servers? Or will the GitHub PR API be enough?

[Dhrumil-Sentry]

@guru-kini Thanks for asking! We don't need to copy any code, the errors you send to Sentry already include the file and function names in the stacktrace so we would simply need to use GitHub APIs and webhooks to build something this

[jasikpark]

Excited to see this implemented! I rarely associate Sentry issues w/ Github issues, but the "suspect commits" functionality is often useful to me for triaging a bug - I hope it's not too false-positive prone though when the stacktrace doesn't point to the true culprit 💜

[Dhrumil-Sentry]

@jasikpark Thanks a lot for the feedback! For our first iteration we're going to restrict comments to recent PRs that caused issues to minimize false positives

I hope it's not too false-positive prone though when the stacktrace doesn't point to the true culprit

Could you please expand a bit more on this? 🙏

Are you referring to cases when an issue is caused by a change made to the 2nd or 3rd line in the stacktrace instead of the change made to the first stacktrace line?

[jasikpark]

Sure? I've just seen stack traces that were pretty compact and not very useful, so I could see them being weaker indicators, but I guess y'all call them "suspect commits" for a reason 😄

[Dhrumil-Sentry]

We've made Post-Merge PR Comments available for few early-adopter customers 🎉

Sentry will comment on recent PRs suspected of causing issues

There's no action needed to enable the feature if you have the GitHub integration already set up.

If needed, you can disable the feature in your organization's GitHub Integration settings.

We'll be rolling out this feature to all our customers in the next few weeks. Please let us know if you have any questions or feedback 🙏

[xicond]

Is this only for github ? How about bitbucket ?

[Dhrumil-Sentry]

Hi @xicond - Currently this feature is available for GitHub only, Adding support for other source code integrations is in our backlog but we don't have a timeline yet.

[jarstelfox]

Hey there @Dhrumil-Sentry,

We love this feature!

I think there is a small edge case/configuration missing which would help some users. Here is a concrete example.

Let's say this code exists on the main branch: https://codesandbox.io/p/sandbox/nextjs-sentry-v6-4-1-example-forked-vxynxs

If I dev this code locally and made the following changes:

@@ -10,6 +10,7 @@ export default function IndexPage() {
 }
 
 function MyComponent() {
+  example_syntax_error;
   return (
     <div>
       Hello World.{" "} 

The syntax error from the dev environment would trigger an error. That error is configured to be sent to Sentry.

At this point, we are seeing the suspect commit comments on the previous pull which did not add the code (as the code came from a dev environment).

Some notes:

• Here is a link to an actual sentry issue in which this occurred in our code base
• Here is a link to the pull comment
• It may be possible that this is related to the warning seen on the error reported from the dev environment: Event missing Release tag

My initial thought was to look for a way to configure certain sentry environments to not comment on pulls. In this case, we want to send errors in our dev environments to Sentry's dev environment, but also not hear about the errors that came from a dev environment on pulls.

Note: I can't seem to reliably reproduce the comment bug from the dev environment, but have reproduced it once (see the second link on the GH comment)

[Dhrumil-Sentry]

Hi @jarstelfox

Thanks a lot for this feedback! These PR Comments are powered by our Suspect Commits feature.

The way PR Comments works today is that when an issue is observed and if Sentry can identify that the issue was caused by a recent commit then we comment on that pull request.

In this example issue that you shared would you say that the Suspect Commit of the issue is incorrect too?

Your feedback about letting Sentry disregard suspect commits and PR comments for certain environments is pretty helpful.

@brianthi @rachrwang @cathteng some helpful feedback for Suspect Commits and PR Comments

[jarstelfox]

In this example issue that you shared would you say that the Suspect Commit of the issue is incorrect too?

Yep! It looks like the Suspect Commit is from the pull request, but the code running is actually a dev environment that includes this commit.

[sandstrom]

@Dhrumil-Sentry This overall idea is great!

The security problem

One thing we're running into with Sentry and Github, is that for security reasons we don't want to give you read access to source code, nor write access to repo hooks.

Issue read access and ability to post comments to PRs could be alright, but probably also something we'd like to avoid.

My suggestion

A great approach is to leverage Github Actions and CLI tools (you already have an excellent one) to push data to Sentry, instead of you pulling. The benefit with running your CLI tool in our CI is that we can pin the version to a SHA hash, so if you get hacked your hackers still can't do anything.

For example, a PR check (Github Actions) could ask Sentry for what's described above:

Sentry can analyze open PRs and comment on any existing issues related to the areas of code being modified. [...] helps them resolve existing issues while they're working on the same areas of code.

And a scheduled job running in Github Actions could use your CLI to ask for "Post-Merge PR Comments".

That way, you wouldn't need read access to our source code.

Is this something you have considered internally?

Prior art

Bugsnag has this approach for Github source code linking. We just tell them about our repo URL, branch name and they'll generate links automatically, without having source code access.

[sandstrom]

@Dhrumil-Sentry Great that you're thinking about this use case!

I understand most of your customers don't mind giving full source-code access, but if it's given thought during the design phase, I think a lot of Github functionality can also be implement by having a Github Actions job push/pull data to/from your API, avoiding the problems with source-code access altogether.

Also, to clarify, we're totally fine with sending smaller pieces of source-code to your servers, e.g. via your CLI. Those small pieces aren't very useful in isolation, so them getting leaked is fine.

The thing we're not allowed to is to give full source-code access, due to the frequency of supply-chain attacks via Github App API tokens and similar (for example what happened to Heroku and Travis CI, where full read access to tons of repos was leaked).

Commit information

Great that it works, will check it out!

However, your current docs says "If you have repositories configured within your Sentry organization you can associate commits with your release.", maybe this could be updated to clarify that having an integration setup isn't a requirement.

Bad releases

What you're planning sounds great! 🎉 (I read the discussion)

Improving PR Reviews with Sentry

Sentry can analyze open PRs and comment on any existing issues related to the areas of code being modified. [...] helps them resolve existing issues while they're working on the same areas of code.

For this one to work without source-code access, perhaps a Github Actions runner with your CLI could just send a list of modified files (or the whole git diff) to your servers and get a response back. That way, no Github Integration with source code access is required.

[shaedrich]

Please keep in mind that there are also other platforms besides GitHub with CI/CD functionality 😉 So whatever CLI solution you envision, please take into consideration.

[Dhrumil-Sentry]

@sandstrom Thanks a lot for the detailed feedback.

perhaps a Github Actions runner with your CLI could just send a list of modified files (or the whole git diff) to your servers and get a response back.

This is certainly interesting and could potentially address the comment #43826 (comment) you made on the Codecov integration as well. I will share this with our Codecov teams too

@vivianyentran There's some helpful docs feedback on this comment.

@shaedrich we'll definitely consider other CI/CD platforms if we go down this route- GitHub is usually the starting point

[sandstrom]

We have a mechanism to send Commit information to Sentry even without the the GitHub integration, Please find more details here

@Dhrumil-Sentry I talked with your support (Rodolfo) about this, and they said this won't help with stack trace linking.

My feedback there would be that the way Bugsnag solved this, was that we could set the base repo URL for each project, and the SHA/tag for a release, and they would be able to link directly from stack traces into our source code on Github, without having access.

Setting the base repo per project could be handled either via the CLI, or manually in project settings.

Such functionality in Sentry would be awesome!

[Dhrumil-Sentry]

@sandstrom This is a great idea and I have created an issue to track this suggestion here #56255

[callms]

Can we possibly opt-out of receiving issues for certain kind of errors?

We just got a comment from the Sentry app o our backend Python-based service on a line that raises a NotFoundError which is just a fancy way for us to return 404 to the caller.

Receiving a comment on GitHub that our backend services handles 404 certainly seems overkill, and we would gladly opt-out from receiving comments from these kind of errors that we consider more to be warnings, while keeping this functionality.

[Dhrumil-Sentry]

Hi @callms - Thanks a lot for this feedback !- It's not possible to do so today but we certainly have been looking for ways to make this more actionable and even considered commenting for unhandled issues only.

We just got a comment from the Sentry app on our backend Python-based service on a line that raises a NotFoundError which is just a fancy way for us to return 404 to the caller.

Do you send this error to Sentry to just track as a metric today? I was wondering if you'd like to see this error in Sentry at all.

Are there other issues you think Sentry should not notify you of?

[davidshepherd7]

We're using the post-merge monitoring and it's magical!

I have a feature request though: I'd like to be able to exclude info-level errors from notifying people on their PRs. We use info level errors to track events that we want to know about but that aren't actually causing problems.

[Dhrumil-Sentry]

@davidshepherd7 thanks a lot for this feedback!

Excluding info-level error issues is a valid ask. Are there any other cases you'd like to exclude too?

cc @cathteng @leedongwei

[davidshepherd7]

Thanks, no I can't think of anything even after a quick look through our other alerting rules.

I could imagine someone asking to configure it to ignore certain warnings, but I think in that case I would ask them to instead change those events to be info level, because that would fit in better with the rest of our alerting setup.

[cathteng]

Hello! We recently added this functionality to merged PR comments #66110

[mf1882]

This looks incredible and something we would absolutely want to have. However we use gitlab, are there plans to bring this feature to Gitlab anytime soon. Is there a way to do this using releases and your api? Every commit is a release for us, it would be great if automate issues by hitting sentry by release and adding them as comments to our PRs.

Thanks

[Dhrumil-Sentry]

@mf1882 Thanks for this feedback- There is no way to support this for GitLab for now but this is certainly in the backlog and we will update the discussion when its available

[mf1882]

if i create a release in sentry for a feature in development. Does your api support being able to grab issues that only happened within that release?

[sandstrom]

If you'd go in the direction I proposed above (#49996 (reply in thread)), then you'd be agnostic to the source code system (VCS), and it would work with GitLab and others too, out of the box.

[Dhrumil-Sentry]

@sandstrom I've shared your feedback with my Eng team, the reason we usually prioritize Source code integration based approaches is that once we build the feature for one integration its much easier as an Eng effort to port it to other integrations and more importantly it works out of the box for a lot of our users who have installed the integration. We def want to support more CLI based flows for such cases in the future.

@mf1882 It's definitely possible to query for issues first seen in a given release via this API: https://docs.sentry.io/api/events/list-a-projects-issues/ . You'd have to pass a query string that uses the firstRelease property

[sandstrom]

I understand, and realize the out-of-the-box experience is important.

Sometimes I think a neat model for you (and other companies in your situation) is to aim for the following:

• base level; a client library in JS, C++, Rust or another suitable language (internal only)
• mid level; a CLI tool wrapping this library, available to customers
• high-level; a pre-built Github action or similar, available as a "1 click install" for customers

That way, most of the code is shared, and you get a nice out of the box experience. Then, if you want to provide a nice integration for Gitlab too, you'd just make a pre-built action for their CI/CD toolset (as a thin wrapper on top of the mid-level and base-level components mentioned above).

Anyway, thanks for listening!

Overall happy with Sentry, even though our security requirements forces us to skip using some Sentry functionality. 😄

[Dhrumil-Sentry]

We're very excited to share that we've GA'd Comments on Open PRs!!

When you open a pull request in GitHub, Sentry will comment and inform you if there are any unhandled issues being caused by the functions you are modifying in your PR so that you can fix issues while you already have the necessary context. This works for Javascript and Python platforms today. We plan on extending support to more languages and source code integrations in the future

Please read our docs to learn more. We've provided an option to disable these as well

[sandstrom]

Great stuff!

Is this something that would be supported in the Sentry CLI in the future?

We'd post the diff, and get back a list of issues (the data in the comment as JSON), and can then use Github Actions and the CLI to post this to our PR.

[Dhrumil-Sentry]

We'd like to publish APIs first that could take the function being modified as the input and return a list of issues relevant to those functions and then work on a Sentry CLI command for it.

[sandstrom]

Sounds awesome! 😄

[bruno-garcia]

I created a ticket specifically for new language support:

• Support more programming languages on GitHub PR Comments #69824

And yes I did this because I use C# and it's not supported yet 😅