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

Check-ins not working #8100

Closed
3 tasks done
simplenotezy opened this issue May 11, 2023 · 13 comments · Fixed by #8116
Closed
3 tasks done

Check-ins not working #8100

simplenotezy opened this issue May 11, 2023 · 13 comments · Fixed by #8116

Comments

@simplenotezy
Copy link

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/browser

SDK Version

7.51.2

Framework Version

Nest.js 9.3.9

Link to Sentry event

https://doubble.sentry.io/crons/sync-amplitude/?environment=production&project=4505119808946176

SDK Setup

Sentry.init({
	integrations: [
		new RewriteFrames({
			root: global.__rootdir__,
		}),
	],
	dsn: process.env.SENTRY_DSN,
	serverName:
		process.env.HOST_REGION || process.env.HOSTNAME
			? `${process.env.HOST_REGION ? `${process.env.HOST_REGION}_` : ''}${process.env.HOSTNAME || 'unknown'}`
			: undefined,
	release: process.env.APP_VERSION,
	environment: process.env.NODE_ENV, // XXX-production
	shutdownTimeout: 10000,
});

Steps to Reproduce

Basically I follow the documentation:

    this.monitorCheckInId = Sentry.captureCheckIn({
      monitorSlug: this.monitorSlug,
      status: status as any,
      ...(this.monitorCheckInId && {
        checkInId: this.monitorCheckInId,
      }),
    });

If I inspect the crons running I see:

[Nest] 1  - 05/11/2023, 9:15:30 AM     LOG [CRON] Will check in with Sentry: in_progress for slug YYYYY...
[Nest] 1  - 05/11/2023, 9:15:30 AM     LOG [CRON] Refreshing XXXX view...
[Nest] 1  - 05/11/2023, 9:18:19 AM     LOG [CRON] Done
[Nest] 1  - 05/11/2023, 9:18:19 AM     LOG [CRON] Will check in with Sentry: ok for slug YYYYY (b3eb88439b864bb8b72ee6d358195789)...

Heres an example of what I did before, and how I refactored the code to use the built-in feature:

enum SentryJobStatus {
   JobStarted = 'in_progress',
@@ -128,7 +126,9 @@ export abstract class BaseCommand extends CommandRunner {
   public async notifyJobCompleted(): Promise<void> {
     await this.jobCheckin(SentryJobStatus.JobCompleted);

-    Sentry.setContext('completed-job', {});
+    Sentry.configureScope((scope) => {
+      scope.setContext('completed-job', {});
+    });
   }

   private async jobCheckin(status: SentryJobStatus, isRunning = true) {
@@ -142,43 +142,30 @@ export abstract class BaseCommand extends CommandRunner {

-    Sentry.setContext('monitor', {
-      slug: this.monitorSlug,
+    Sentry.configureScope((scope) => {
+      scope.setContext('monitor', {
+        slug: this.monitorSlug,
+      });
     });

-    const endpointUrl = `https://sentry.io/api/0/organizations/XXXX/monitors/${this.monitorSlug}/checkins${
-      isRunning ? `/${this.monitorCheckInId ? this.monitorCheckInId : `/latest`}` : ''
-    }/`;
-
-    this.log(`Will check in with Sentry: ${status}...`);
-    await fetch(endpointUrl, {
-      method: !isRunning ? 'POST' : 'PUT',
-      headers: {
-        Authorization: `DSN ${dsn}`,
-        'Content-Type': 'application/json',
-      },
-      body: JSON.stringify({ status }),
-    })
-      .then(async (response) => {
-        if (!response.ok) {
-          return response.text().then((body) => {
-            throw new Error(`Request failed with status ${response.status}: ${body}`);
-          });
-        }
-        return response.json();
-      })
-      .then((data) => {
-        this.log(`Checked in with Sentry: ${data.id}`);
-        this.monitorCheckInId = data.id;
-      })
-      .catch((error) => {
-        this.error(`Job monitoring HTTP error. Status: ${error}`);
-      });
+    this.log(
+      `Will check in with Sentry: ${status} for slug ${this.monitorSlug}${
+        this.monitorCheckInId ? ` (${this.monitorCheckInId})` : ''
+      }...`,
+    );
+
+    this.monitorCheckInId = Sentry.captureCheckIn({
+      monitorSlug: this.monitorSlug,
+      status: status as any,
+      ...(isRunning && {
+        checkInId: this.monitorCheckInId,
+      }),
+    });
   }

Expected Result

It should report OK in the Cron status console, because they are all running as expected

Actual Result

They all report "MISSED"

image

It worked fine when we weren't using the Sentry SDK but just sending normal http requests.

@AbhiPrasad
Copy link
Member

Hey @simplenotezy, thanks for writing in! It's not good that the checkins are not sending. Could you try enabling debug: true for Sentry via:

Sentry.init({
  debug: true,
});

and checking if there are any Sentry debug logs about sending checkins?

I'm going to try to repro w/ and example nest.js app myself, but if you get the chance to provide a reproduction would be amazing for use to debug further!

@teslitsky
Copy link

teslitsky commented May 14, 2023

Had this issue too after begin using Sentry SDK for check-ins.
With debug: true Sentry logs the following lines:

Sentry Logger [log]: Integration installed: InboundFilters
Sentry Logger [log]: Integration installed: FunctionToString
Sentry Logger [log]: Integration installed: Console
Sentry Logger [log]: Integration installed: Http
Sentry Logger [log]: Integration installed: Undici
Sentry Logger [log]: Integration installed: OnUncaughtException
Sentry Logger [log]: Integration installed: OnUnhandledRejection
Sentry Logger [log]: Integration installed: ContextLines
Sentry Logger [log]: Integration installed: LocalVariables
Sentry Logger [log]: Integration installed: Context
Sentry Logger [log]: Integration installed: Modules
Sentry Logger [log]: Integration installed: RequestData
Sentry Logger [log]: Integration installed: LinkedErrors
Screenshot 2023-05-14 at 10 01 10

@AbhiPrasad
Copy link
Member

Hey folks, we've identified this bug - will fix with the next release. Sorry for the trouble!

@AbhiPrasad
Copy link
Member

Hey we've released https://github.com/getsentry/sentry-javascript/releases/tag/7.52.0 with this being fixed. Could you guys upgrade and give it a try? Thanks!

@simplenotezy
Copy link
Author

@AbhiPrasad Don't think it has worked. I tried locally but all jobs are still marked as 'missing'. I have pushed to production and will see how it goes over the next hours though, but don't think it's working

@simplenotezy
Copy link
Author

simplenotezy commented May 15, 2023

@AbhiPrasad Nope, they still don't work unfortunately

Also, for our Nest.js application, if we enable debug: true nothing is outputted.

@AbhiPrasad
Copy link
Member

I tried reproducing this w/ a basic nestjs example: https://github.com/AbhiPrasad/gh-8100-nestjs-crons, but was unable to.

Seems like there was a regression of us no longer emitting logs when sending checkIn's, let me fix that. Thanks for your patience!

In the meantime, could you try adding this and seeing what gets logged out?

// log out check in envelopes
  Sentry.getCurrentHub()
    .getClient()
    .on('beforeEnvelope', (env) => {
      const body = env[1];
      const item = body[0];
      if (item[0].type === 'check_in') {
        console.log(item[1]);
      }
    });

You should see something like:

{
  check_in_id: '6efd0547722d4d10ae800c4400117c92',
  monitor_slug: 'runEveryMinute',
  status: 'in_progress',
  release: undefined,
  environment: undefined
}
{
  check_in_id: '6efd0547722d4d10ae800c4400117c92',
  monitor_slug: 'runEveryMinute',
  status: 'ok',
  release: undefined,
  environment: undefined,
  duration: undefined
}

where the monitor slug/check in id are identical, and only the status changes (from in progress to ok/error).

@simplenotezy
Copy link
Author

@AbhiPrasad I tried running the code you provided but that won't run for some reason.

The .getClient() returns undefined.

@simplenotezy
Copy link
Author

simplenotezy commented May 16, 2023

We started using cURL as mentioned, and then switched to your SDK. Since we did not pass over environment it defauled to production (using cURL).

However, after switching to using the SDK it defaults to our init() configuration which sets environment to athena-production.

Not sure if this could be a help for you in debugging the issue.

@AbhiPrasad
Copy link
Member

However, after switching to using the SDK it defaults to our init() configuration which sets environment to athena-production.

This is intended behaviour, as the SDK API will use the SDK's set environment variable.

The .getClient() returns undefined.

This is really strange, since a client has to be defined for Sentry to work. I recommend you put the snippet right below where you call Sentry.init.

@simplenotezy
Copy link
Author

Aha @AbhiPrasad I think that was the issue. Sentry was only being initialised during the main (http) server start, not the start of CLI. Let me push to prod and see if it changes anything. The cron popped up instantly:

{
  check_in_id: 'XXX',
  monitor_slug: 'XXX',
  status: 'in_progress',
  release: undefined,
  environment: 'local-mattias'
}

@AbhiPrasad
Copy link
Member

Sentry was only being initialised during the main (http) server start, not the start of CLI

Great! This maybe is something we have to document better. We can add a note to #5578 while we work on a more fleshed out NestJS SDK.

@simplenotezy
Copy link
Author

@AbhiPrasad It's confirmed to be working! 😊

Sentry was only being initialised during the main (http) server start, not the start of CLI

Great! This maybe is something we have to document better. We can add a note to #5578 while we work on a more fleshed out NestJS SDK.

Really great news you're working on a NestJS SDK - that is sooo much needed. Let me know if you need any support in this regard / test case. We'd be happy to participate in Doubble!

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

Successfully merging a pull request may close this issue.

3 participants