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

Port @sentry/remix to be powered by OpenTelemetry #11040

Closed
6 tasks done
Tracked by #9508
mydea opened this issue Mar 12, 2024 · 1 comment
Closed
6 tasks done
Tracked by #9508

Port @sentry/remix to be powered by OpenTelemetry #11040

mydea opened this issue Mar 12, 2024 · 1 comment
Assignees

Comments

@mydea
Copy link
Member

mydea commented Mar 12, 2024

Tasks

@mydea mydea mentioned this issue Mar 12, 2024
@AbhiPrasad AbhiPrasad added Feature: Performance Package: remix Issues related to the Sentry Remix SDK labels Apr 24, 2024
@AbhiPrasad AbhiPrasad added this to the 8.0.0 milestone Apr 24, 2024
@AbhiPrasad
Copy link
Member

@mydea mydea closed this as completed May 15, 2024
AbhiPrasad pushed a commit that referenced this issue Jun 13, 2024
Ref: #11040

Migrates Remix server-side SDK to
[opentelemetry-instrumentation-remix](https://www.npmjs.com/package/opentelemetry-instrumentation-remix).

This PR also keeps the original implementation not the break the
developer experience for non-Express Remix projects.
Remix projects using Express are supported as is using the new
`autoInstrumentRemix` option.

**Usage with Express:**

```javascript
// instrument.(cjs | mjs)
const Sentry = require('@sentry/remix');

Sentry.init({
  dsn: YOUR_DSN
  // ...
  // auto instrument Remix with OpenTelemetry
  autoInstrumentRemix: true,
  // Optionally capture action formData attributes with errors.
  // This requires `sendDefaultPii` set to true as well.
  captureActionFormDataKeys: {
    file: true,
    text: true,
  },
  // To capture action formData attributes.
  sendDefaultPii: true
});
``` 

```javascript
// server.(cjs | mjs)
// import the Sentry instrumentation file before anything else.
import './instrument.cjs';
// alternatively `require('./instrument.cjs')`

// ...

const app = express();

// ...
```

**Usage with built-in Remix server:**

You need to run the Remix server with `NODE_OPTIONS=`--require(...)`
set.

```json
// package.json

// ...
  "scripts": {
    "start": "NODE_OPTIONS='--require=./instrument.server.cjs' remix-serve build"
    // or
    "start": "NODE_OPTIONS='--import=./instrument.server.mjs' remix-serve build"
  }
// ...
```

This PR _**removes**_:
- Express server adapter.
There is no need to use `wrapExpressCreateRequestHandler` anymore even
if you don't opt-in to `autoInstrumentRemix`.
`wrapExpressCreateRequestHandler` is kept exported as a no-op function.

- Built in HTTP incoming request instrumentation for both `legacy` and
`otel` implementations. Instead we mark `requestHandler` spans as the
root `http.server` spans.

When `autoInstrumentRemix` is set to `true`, this update _**replaces**_:

- Performance tracing on `action` / `loader` / `documentRequest`
functions. Leaving them to be traced by
`opentelemetry-instrumentation-remix`
- Request handler instrumentation as they are also traced by
`opentelemetry-instrumentation-remix`
- Auto-instrumentation for http as default integration for Remix SDK.


- With the new instrumentation, `pageload` span is the child of the
`loader` span. (Example:
[Trace](https://sentry-sdks.sentry.io/performance/trace/e51c640933786fd491bb428fb1eff826/?fov=0%2C436.5&node=error-cd2dc8610c5e4affb7356147daa77393&statsPeriod=7d&timestamp=1718029546))

- Legacy instrumentation keeps recording `pageload` span as the child of
the `http.server` span. (Example:
[Trace](https://sentry-sdks.sentry.io/performance/trace/01107a555eeee9b44d1f8c25ea0c45f3/?fov=0%2C818.599853515625&node=txn-0695246d026b4da792cf8f5e0a372b50&statsPeriod=7d&timestamp=1718103519))


Also:

Migrates Remix integration tests from Jest to Vitest.
- Related issues: jestjs/jest#15033
elastic/require-in-the-middle#50

Fixes Backlogged Issues:
- Fixes: #12082 -
[Code](https://github.com/getsentry/sentry-javascript/pull/12110/files#diff-754e32c1c14ac3c3c93eedeb7680548b986bc02a8b0bc63d2efc189210322acdR416-R440)
- Fixes: #9737 -
[Test](https://github.com/getsentry/sentry-javascript/pull/12110/files#diff-1484a5311699a814a39921d440f831e9babe69f8225a0de667ebd8cc63ca5accR109)
- Fixes: #12285
@AbhiPrasad AbhiPrasad removed this from the 8.0.0 milestone Jun 13, 2024
@AbhiPrasad AbhiPrasad reopened this Jun 13, 2024
@AbhiPrasad AbhiPrasad changed the title [v8] Port @sentry/remix to be powered by OpenTelemetry Port @sentry/remix to be powered by OpenTelemetry Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants