Skip to content

fix(node): Make sure we use same ID for checkIns #8050

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

Merged
merged 4 commits into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions packages/node/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,25 +153,30 @@ export class NodeClient extends BaseClient<NodeClientOptions> {
* @param checkIn An object that describes a check in.
* @param upsertMonitorConfig An optional object that describes a monitor config. Use this if you want
* to create a monitor automatically when sending a check in.
* @returns A string representing the id of the check in.
*/
public captureCheckIn(checkIn: CheckIn, monitorConfig?: MonitorConfig): void {
public captureCheckIn(checkIn: CheckIn, monitorConfig?: MonitorConfig): string {
const id = checkIn.status !== 'in_progress' && checkIn.checkInId ? checkIn.checkInId : uuid4();
if (!this._isEnabled()) {
__DEBUG_BUILD__ && logger.warn('SDK not enabled, will not capture checkin.');
return;
return id;
}

const options = this.getOptions();
const { release, environment, tunnel } = options;

const serializedCheckIn: SerializedCheckIn = {
check_in_id: uuid4(),
check_in_id: id,
monitor_slug: checkIn.monitorSlug,
status: checkIn.status,
duration: checkIn.duration,
release,
environment,
};

if (checkIn.status !== 'in_progress') {
serializedCheckIn.duration = checkIn.duration;
}

if (monitorConfig) {
serializedCheckIn.monitor_config = {
schedule: monitorConfig.schedule,
Expand All @@ -183,6 +188,7 @@ export class NodeClient extends BaseClient<NodeClientOptions> {

const envelope = createCheckInEnvelope(serializedCheckIn, this.getSdkMetadata(), tunnel, this.getDsn());
void this._sendEnvelope(envelope);
return id;
}

/**
Expand Down
10 changes: 8 additions & 2 deletions packages/node/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
logger,
nodeStackLineParser,
stackParserFromStackParserOptions,
uuid4,
} from '@sentry/utils';

import { setNodeAsyncContextStrategy } from './async';
Expand Down Expand Up @@ -273,12 +274,17 @@ export function captureCheckIn(
checkIn: CheckIn,
upsertMonitorConfig?: MonitorConfig,
): ReturnType<NodeClient['captureCheckIn']> {
const capturedCheckIn =
checkIn.status !== 'in_progress' && checkIn.checkInId ? checkIn : { ...checkIn, checkInId: uuid4() };

const client = getCurrentHub().getClient<NodeClient>();
if (client) {
return client.captureCheckIn(checkIn, upsertMonitorConfig);
client.captureCheckIn(capturedCheckIn, upsertMonitorConfig);
} else {
__DEBUG_BUILD__ && logger.warn('Cannot capture check in. No client defined.');
}

__DEBUG_BUILD__ && logger.warn('Cannot capture check in. No client defined.');
return capturedCheckIn.checkInId;
}

/** Node.js stack parser */
Expand Down
31 changes: 25 additions & 6 deletions packages/node/test/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,8 @@ describe('NodeClient', () => {
// @ts-ignore accessing private method
const sendEnvelopeSpy = jest.spyOn(client, '_sendEnvelope');

client.captureCheckIn(
{ monitorSlug: 'foo', status: 'ok', duration: 1222 },
const id = client.captureCheckIn(
{ monitorSlug: 'foo', status: 'in_progress' },
{
schedule: {
type: 'crontab',
Expand All @@ -314,10 +314,9 @@ describe('NodeClient', () => {
[
expect.any(Object),
{
check_in_id: expect.any(String),
duration: 1222,
check_in_id: id,
monitor_slug: 'foo',
status: 'ok',
status: 'in_progress',
release: '1.0.0',
environment: 'dev',
monitor_config: {
Expand All @@ -333,6 +332,26 @@ describe('NodeClient', () => {
],
],
]);

client.captureCheckIn({ monitorSlug: 'foo', status: 'ok', duration: 1222, checkInId: id });

expect(sendEnvelopeSpy).toHaveBeenCalledTimes(2);
expect(sendEnvelopeSpy).toHaveBeenCalledWith([
expect.any(Object),
[
[
expect.any(Object),
{
check_in_id: id,
monitor_slug: 'foo',
duration: 1222,
status: 'ok',
release: '1.0.0',
environment: 'dev',
},
],
],
]);
});

it('does not send a checkIn envelope if disabled', () => {
Expand All @@ -342,7 +361,7 @@ describe('NodeClient', () => {
// @ts-ignore accessing private method
const sendEnvelopeSpy = jest.spyOn(client, '_sendEnvelope');

client.captureCheckIn({ monitorSlug: 'foo', status: 'ok', duration: 1222 });
client.captureCheckIn({ monitorSlug: 'foo', status: 'in_progress' });

expect(sendEnvelopeSpy).toHaveBeenCalledTimes(0);
});
Expand Down
20 changes: 20 additions & 0 deletions packages/node/test/sdk.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { getCurrentHub } from '@sentry/core';
import type { Integration } from '@sentry/types';

import type { NodeClient } from '../build/types';
import { init } from '../src/sdk';
import * as sdk from '../src/sdk';

Expand Down Expand Up @@ -90,3 +92,21 @@ describe('init()', () => {
expect(newIntegration.setupOnce as jest.Mock).toHaveBeenCalledTimes(1);
});
});

describe('captureCheckIn', () => {
it('always returns an id', () => {
const hub = getCurrentHub();
const client = hub.getClient<NodeClient>();
expect(client).toBeDefined();

const captureCheckInSpy = jest.spyOn(client!, 'captureCheckIn');

// test if captureCheckIn returns an id even if client is not defined
hub.bindClient(undefined);

expect(captureCheckInSpy).toHaveBeenCalledTimes(0);
expect(sdk.captureCheckIn({ monitorSlug: 'gogogo', status: 'in_progress' })).toBeTruthy();

hub.bindClient(client);
});
});
15 changes: 13 additions & 2 deletions packages/types/src/checkin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,26 @@ export interface SerializedCheckIn {
};
}

export interface CheckIn {
interface InProgressCheckIn {
// The distinct slug of the monitor.
monitorSlug: SerializedCheckIn['monitor_slug'];
// The status of the check-in.
status: SerializedCheckIn['status'];
status: 'in_progress';
}

export interface FinishedCheckIn {
// The distinct slug of the monitor.
monitorSlug: SerializedCheckIn['monitor_slug'];
// The status of the check-in.
status: 'ok' | 'error';
// Check-In ID (unique and client generated).
checkInId: SerializedCheckIn['check_in_id'];
// The duration of the check-in in seconds. Will only take effect if the status is ok or error.
duration?: SerializedCheckIn['duration'];
}

export type CheckIn = InProgressCheckIn | FinishedCheckIn;

type SerializedMonitorConfig = NonNullable<SerializedCheckIn['monitor_config']>;

export interface MonitorConfig {
Expand Down