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

feat: Sentry integration #141

Merged
merged 9 commits into from
Jan 2, 2024
Merged

feat: Sentry integration #141

merged 9 commits into from
Jan 2, 2024

Conversation

benjackwhite
Copy link
Collaborator

@benjackwhite benjackwhite commented Dec 18, 2023

Clone of #120

Changes

Resolves #106 by porting the Sentry integration from posthog-js to posthog-js-lite. I have decided to port this for Node only for now, since I don’t think (??) posthog-js is in production yet. If we were to rely on a common core here some of the code would have to change.

I am also unsure of how this library wants to differ from posthog-js—the main edits I made were stylistic, and I tried to type everything a bit more strongly, which was the… vibe I’m getting from this repo? So I’m submitting this PR early and we can figure out what you actually want here (I also didn’t write tests). I will leave comments with questions.

Testing Strategy

I tested this manually by compiling the library & patching it into my own app. I tested that:

  • Sentry URL is attached to PostHog events and resolves correctly (only on sentry.io)
  • PostHog Person URL is attached to Sentry events and resolves correctly (only on eu.posthog.com)

I am happy to write a test (how?) or test more extensively, wanted to wait until we had a more-reviewed PR before we do that.

(@benjackwhite - added tests)

Release info Sub-libraries affected

Bump level

  • Major
  • Minor
  • Patch

Libraries affected

  • All of them
  • posthog-web
  • posthog-node
  • posthog-react-native

Changelog notes

  • Added posthog.SentryIntegration to posthog-node. Usage:
Sentry.init({
     dsn: 'https://example',
     integrations: [
         new posthog.SentryIntegration(posthog)
     ]
})

Sentry.setTag(posthog.POSTHOG_ID_TAG, 'some distinct id');

@huw
Copy link

huw commented Dec 18, 2023

Easy, I’m following this one now

@benjackwhite benjackwhite marked this pull request as ready for review January 2, 2024 13:57
@marandaneto
Copy link
Member

After this PR it'd be cool to have docs -> https://posthog.com/docs/libraries/node similar to https://posthog.com/docs/libraries/python#sentry so it's more discoverable.

Copy link
Member

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

LGTM

@benjackwhite
Copy link
Collaborator Author

After this PR it'd be cool to have docs -> https://posthog.com/docs/libraries/node similar to https://posthog.com/docs/libraries/python#sentry so it's more discoverable.

Yep was just doing this :D

Copy link
Member

@pauldambra pauldambra left a comment

Choose a reason for hiding this comment

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

looks good to me!

@benjackwhite benjackwhite merged commit cca013a into main Jan 2, 2024
3 checks passed
@benjackwhite benjackwhite deleted the feat/sentry-integration branch January 2, 2024 14:52
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.

Sentry Integration for posthog-node
4 participants