Skip to content

feat(node): Instrumentation for node-cron library#9904

Merged
AbhiPrasad merged 17 commits intogetsentry:developfrom
timfish:feat/node-cron-wrap
Jan 3, 2024
Merged

feat(node): Instrumentation for node-cron library#9904
AbhiPrasad merged 17 commits intogetsentry:developfrom
timfish:feat/node-cron-wrap

Conversation

@timfish
Copy link
Copy Markdown
Collaborator

@timfish timfish commented Dec 18, 2023 •
edited
Loading

Closes #9892

Usage

import * as Sentry from "@sentry/node";
import cron from "node-cron";

const cronWithCheckIn = Sentry.cron.instrumentNodeCron(cron);

cronWithCheckIn.schedule(
  "* * * * *",
  () => {
    console.log("running a task every minute");
  },
  { name: "my-cron-job" },
);

@timfish timfish requested a review from AbhiPrasad December 19, 2023 11:32
AbhiPrasad
AbhiPrasad reviewed Dec 19, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

IMO let's namespace all the cron instrumentation under Sentry.cron.instrumentNodeCron. We don't have to care about treeshaking for this, and we avoid polluting global namespace.

@AbhiPrasad AbhiPrasad added the ci-codecov-ai-review AI Review by Codecov label Dec 19, 2023
@AbhiPrasad
Copy link
Copy Markdown
Contributor

AbhiPrasad commented Dec 19, 2023

gonna test something out with this PR

@timfish timfish marked this pull request as ready for review December 19, 2023 18:17
anonrig
anonrig reviewed Dec 19, 2023
View reviewed changes
@AbhiPrasad AbhiPrasad requested a review from gaprl December 19, 2023 19:20
AbhiPrasad
AbhiPrasad reviewed Dec 20, 2023
View reviewed changes
@timfish timfish requested a review from AbhiPrasad December 21, 2023 14:09
@timfish
Copy link
Copy Markdown
Collaborator Author

timfish commented Dec 21, 2023

I've moved replaceCronNames to common.ts as I need this for the other instrumentations!

AbhiPrasad
AbhiPrasad reviewed Dec 21, 2023
View reviewed changes
@AbhiPrasad AbhiPrasad mentioned this pull request Dec 21, 2023
Merged
@timfish timfish changed the title feat(node): Add wrapping of node-cron for check-ins feat(node): Instrumentation for node-cron library Dec 31, 2023
@AbhiPrasad AbhiPrasad mentioned this pull request Jan 2, 2024
Merged
@timfish
Copy link
Copy Markdown
Collaborator Author

timfish commented Jan 3, 2024 •
edited
Loading

This is now exported from all the downstream packages.

@AbhiPrasad AbhiPrasad merged commit da7c959 into getsentry:develop Jan 3, 2024
@AbhiPrasad AbhiPrasad mentioned this pull request Jan 4, 2024
Merged
@timfish timfish deleted the feat/node-cron-wrap branch January 10, 2024 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gaprl gaprl Awaiting requested review from gaprl

2 more reviewers

@anonrig anonrig anonrig left review comments

@AbhiPrasad AbhiPrasad AbhiPrasad approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ci-codecov-ai-review AI Review by Codecov

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Instrumentation for node-cron library

3 participants

@timfish @AbhiPrasad @anonrig