Skip to content

Conversation

themaxgoldman
Copy link
Contributor

@themaxgoldman themaxgoldman commented Jul 21, 2025

The checkInMarshalJSON function was missing FailureIssueThreshold and RecoveryThreshold fields when serializing MonitorConfig, causing these values to be omitted from the JSON payload sent to Sentry's API. This results in Sentry using default values instead of the configured values.

This adds the missing fields to the JSON serialization and includes a regression test to prevent future occurrences.

Root Cause:

  • checkInMarshalJSON in interfaces.go was not including FailureIssueThreshold and RecoveryThreshold fields
  • These fields were never sent to Sentry's API, regardless of SDK configuration
  • Sentry defaults to FailureIssueThreshold=1 and RecoveryThreshold=1 when not provided

Changes:

  • Fixed checkInMarshalJSON in interfaces.go to include missing fields in JSON payload
  • Added regression test in marshal_test.go with corresponding golden file

Impact:

  • Monitor configurations will now use the SDK-configured thresholds instead of defaults
  • The Sentry UI will display the correct configured values

Testing:

  • go test -run TestCheckInEventMarshalJSON - New test passes

Fixes #1059

Copy link
Member

@cleptric cleptric left a comment

Choose a reason for hiding this comment

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

Thanks

Copy link

codecov bot commented Jul 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.88%. Comparing base (7b375b0) to head (427eb39).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1060      +/-   ##
==========================================
+ Coverage   86.86%   86.88%   +0.02%     
==========================================
  Files          55       55              
  Lines        5840     5842       +2     
==========================================
+ Hits         5073     5076       +3     
  Misses        624      624              
+ Partials      143      142       -1     

☔ 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.

@cleptric cleptric merged commit 7b8885f into getsentry:master Jul 21, 2025
18 checks passed
@themaxgoldman
Copy link
Contributor Author

Hi @cleptric! Thanks for merging! Do you have an ETA for the next patch release, or would it be possible to cut a release with this change? This is affecting us in production and I'd like to get the fix out. Happy to help in any way if I can.

@cleptric
Copy link
Member

Likely next week.

@cleptric
Copy link
Member

This was released as part of v0.35.0

@themaxgoldman
Copy link
Contributor Author

This was released as part of v0.35.0

Thanks!

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.

MonitorConfig.FailureIssueThreshold and RecoveryThreshold not included in JSON payload to Sentry API
2 participants