Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add API for sending checkins (CRONS) manually #2935

Merged
merged 9 commits into from Sep 19, 2023
Merged

Add API for sending checkins (CRONS) manually #2935

merged 9 commits into from Sep 19, 2023

Conversation

adinauer
Copy link
Member

📜 Description

Add API for sending checkins (CRONS) manually.

Simplest usage looks like:

Sentry.captureCheckIn(new CheckIn("my_monitor", CheckInStatus.OK))

💡 Motivation and Context

One of the parts required for #2875

💚 How did you test it?

Manually, automated tests

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Copy link
Member

@markushi markushi left a comment

Choose a reason for hiding this comment

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

Nice one!

@github-actions
Copy link
Contributor

github-actions bot commented Sep 13, 2023

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 44a3c75

@github-actions
Copy link
Contributor

github-actions bot commented Sep 13, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 407.42 ms 483.84 ms 76.42 ms
Size 1.72 MiB 2.29 MiB 575.93 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
f274c79 334.86 ms 348.54 ms 13.68 ms
9e60fc1 310.37 ms 359.48 ms 49.11 ms
9e60fc1 309.74 ms 362.30 ms 52.56 ms
adf8fe3 300.49 ms 357.36 ms 56.87 ms
9e60fc1 331.71 ms 374.28 ms 42.57 ms
f60279b 324.60 ms 345.33 ms 20.73 ms
f274c79 313.96 ms 355.96 ms 42.00 ms
fe10f05 314.71 ms 360.62 ms 45.90 ms
9246ed4 281.79 ms 352.08 ms 70.29 ms
695d3a3 300.98 ms 376.90 ms 75.92 ms

App size

Revision Plain With Sentry Diff
f274c79 1.72 MiB 2.29 MiB 575.01 KiB
9e60fc1 1.72 MiB 2.29 MiB 575.91 KiB
9e60fc1 1.72 MiB 2.29 MiB 575.91 KiB
adf8fe3 1.72 MiB 2.29 MiB 575.24 KiB
9e60fc1 1.72 MiB 2.29 MiB 575.91 KiB
f60279b 1.72 MiB 2.29 MiB 575.23 KiB
f274c79 1.72 MiB 2.29 MiB 575.01 KiB
fe10f05 1.72 MiB 2.29 MiB 575.54 KiB
9246ed4 1.72 MiB 2.28 MiB 572.22 KiB
695d3a3 1.72 MiB 2.29 MiB 575.53 KiB

Previous results on branch: feat/crons

Startup times

Revision Plain With Sentry Diff
ed3dc1f 414.77 ms 558.58 ms 143.82 ms
99121a6 373.85 ms 449.48 ms 75.63 ms

App size

Revision Plain With Sentry Diff
ed3dc1f 1.72 MiB 2.29 MiB 575.93 KiB
99121a6 1.72 MiB 2.29 MiB 575.94 KiB

Copy link
Collaborator

@lbloder lbloder left a comment

Choose a reason for hiding this comment

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

Good Job 👍
Left some comments


private @Nullable Map<String, Object> unknown;

public CheckIn(String monitorSlug, CheckInStatus status) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Constructor Params should be @NotNull annotated and final

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do in a follow up PR.

}

@ApiStatus.Internal
public CheckIn(SentryId checkInId, String monitorSlug, String status) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above @NotNull annotations and final for the params

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do in a follow up PR.

sentry/src/main/java/io/sentry/SentryClient.java Outdated Show resolved Hide resolved
sentry/src/main/java/io/sentry/SentryClient.java Outdated Show resolved Hide resolved
SentryEnvelopeItem.fromCheckIn(options.getSerializer(), checkIn);
envelopeItems.add(checkInItem);

// TODO do we need trace context?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is that already decided? If so, we can remove the TODO

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, shouldn't hurt to add it. Will do. Maybe in a follow up PR.

sentry/src/main/java/io/sentry/CheckInStatus.java Outdated Show resolved Hide resolved
sentry/src/main/java/io/sentry/Hub.java Outdated Show resolved Hide resolved
sentry/src/main/java/io/sentry/Hub.java Outdated Show resolved Hide resolved
sentry/src/main/java/io/sentry/MonitorScheduleType.java Outdated Show resolved Hide resolved
sentry/src/main/java/io/sentry/MonitorScheduleUnit.java Outdated Show resolved Hide resolved
@adinauer adinauer merged commit 8d29d97 into main Sep 19, 2023
19 of 21 checks passed
@adinauer adinauer deleted the feat/crons branch September 19, 2023 04:35
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.

None yet

3 participants