Skip to content

Change order of event filtering mechanisms and only send session update for dropped events if session state changed#2028

Merged
adinauer merged 2 commits intomainfrom
fix/change-order-of-event-filtering
May 11, 2022
Merged

Change order of event filtering mechanisms and only send session update for dropped events if session state changed#2028
adinauer merged 2 commits intomainfrom
fix/change-order-of-event-filtering

Conversation

@adinauer
Copy link
Copy Markdown
Member

📜 Description

Also apply changes from #2001 and #2002 to 5.x.

💡 Motivation and Context

To align event filter order and update of sessions for dropped events across SDKs (getsentry/develop#551)

💚 How did you test it?

Unit Tests

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

🔮 Next steps

  • New release after merge

... and only send session update for dropped events if session state changed
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 10, 2022

Codecov Report

Merging #2028 (346de30) into main (dc680dc) will increase coverage by 0.06%.
The diff coverage is 92.50%.

@@             Coverage Diff              @@
##               main    #2028      +/-   ##
============================================
+ Coverage     75.60%   75.67%   +0.06%     
- Complexity     2270     2286      +16     
============================================
  Files           225      225              
  Lines          8072     8098      +26     
  Branches        852      862      +10     
============================================
+ Hits           6103     6128      +25     
+ Misses         1558     1557       -1     
- Partials        411      413       +2     
Impacted Files Coverage Δ
sentry/src/main/java/io/sentry/SentryClient.java 86.47% <92.50%> (+0.48%) ⬆️
sentry/src/main/java/io/sentry/ISentryClient.java 90.47% <0.00%> (+4.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc680dc...346de30. Read the comment docs.

@vibin
Copy link
Copy Markdown

vibin commented May 10, 2022

does this mean that, going forward if I have a BeforeSendCallback that is skipping events with event.isCrashed is false, then the crashes count (on Releases page) and errors count (on Release details page) would actually match exactly?

The configuration which I'm using is:

sampleRate = null // Send all errors. No sampling.
isAnrEnabled = false
beforeSend = BeforeSendCallback { event: SentryEvent, hint: Any? ->
    return@BeforeSendCallback if (event.isCrashed) {
        event
    } else {
        null // Swallow non-fatal errors.
    }
}

@bruno-garcia
Copy link
Copy Markdown
Member

does this mean that, going forward if I have a BeforeSendCallback that is skipping events with event.isCrashed is false, then the crashes count (on Releases page) and errors count (on Release details page) would actually match exactly?

The configuration which I'm using is:

sampleRate = null // Send all errors. No sampling.
isAnrEnabled = false
beforeSend = BeforeSendCallback { event: SentryEvent, hint: Any? ->
    return@BeforeSendCallback if (event.isCrashed) {
        event
    } else {
        null // Swallow non-fatal errors.
    }
}

If you return null then no session update is sent (so a session won't potentially change from healthy to errored).

@adinauer adinauer merged commit 4ebc5ef into main May 11, 2022
@adinauer adinauer deleted the fix/change-order-of-event-filtering branch May 11, 2022 16:47
@vibin
Copy link
Copy Markdown

vibin commented May 11, 2022

Thanks @bruno-garcia!

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.

5 participants