Skip to content

feat: Add Sidecar connect function#92

Merged
HazAT merged 1 commit into
mainfrom
feat/connect-sidecar
Nov 21, 2023
Merged

feat: Add Sidecar connect function#92
HazAT merged 1 commit into
mainfrom
feat/connect-sidecar

Conversation

@HazAT

@HazAT HazAT commented Nov 21, 2023

Copy link
Copy Markdown
Member

To hook into Sentry's Node SDK this is what you have to do:

import * as Sentry from '@sentry/node';
import { connect } from '@spotlightjs/sidecar/connect'

Sentry.init({debug: true});
connect(Sentry.getCurrentHub()); // After Sentry init, call connect and pass the hub
...
Sentry.captureMessage('Something went wrong');

ref: #76

@vercel

vercel Bot commented Nov 21, 2023

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
spotlightjs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 21, 2023 7:42pm

@dcramer

dcramer commented Nov 21, 2023

Copy link
Copy Markdown
Member

Is there any reason we cant automatically do this somehow?

@dcramer

dcramer commented Nov 21, 2023

Copy link
Copy Markdown
Member

Also lets open a ticket on the core JS repo to make sure whatever we do here becomes a native API

@HazAT

HazAT commented Nov 21, 2023

Copy link
Copy Markdown
Member Author

Only to avoid the SDK even knowing about Spotlight/Sidecar
If not, this fetch has to be in our Core Node SDK that every customer runs which is not a good idea imho

fetch("http://localhost:8969/stream", {
      method: "POST",
      body: serializeEnvelope(envelope),
      headers: {
        "Content-Type": "application/x-sentry-envelope",
      },
      mode: "cors",
    }).catch((err) => {
      console.error('[Spotlight]', err);
    });

This is only a problem for Node (and other Server SDKs) - not for Browser

@HazAT

HazAT commented Nov 21, 2023

Copy link
Copy Markdown
Member Author

The advantage of an explicit connect function from the sidecar is that it stays in sync with the actual sidecar
If we add this code to the SDKs you need a specific version + if we change the URL / Port, it gets outdated.

I think initially this is fine until this all is a bit more robust.

"./vite-plugin": {
"import": "./src/vite-plugin.js"
},
"./connect": {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is there no other sane way to define these main exports e.g. in like an index.ts or something?

@dcramer dcramer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

did not test

@HazAT HazAT merged commit f685f85 into main Nov 21, 2023
@HazAT HazAT deleted the feat/connect-sidecar branch November 21, 2023 20:54
@dcramer

dcramer commented Nov 21, 2023

Copy link
Copy Markdown
Member

We should reconsider this and put a spotlight: true/false into Sentry.init() in SDKs imo.

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.

2 participants