Fix Example

[mydea]

This seems to make it work for me.
If you remove tracesSampleRate, only minimal instrumentation will be added (=Http). You can then add other instrumentation (e.g. http) on your own, only http should be skipped. We'll look into making this more ergonomic too...!

[mydea]

we can skip the sampler if we really don't care about distributed tracing etc. I'd still recommend to wrap your own sampler with wrapSamplingDecision but it should generally work this way too.

[mydea]

Removing this leads to Sentry not adding non-essential OTEL instrumentation out of the box.

[mydea]

I just reduced this to make it easier to parse the user/tags etc.

[mydea]

Express has to be added if you care about it, only http can/should be skipped here.

[sbriceland]

Gotcha - that makes sense. I thought this could be it. But since Sentry.httpIntegration took care of OTEL httpInstrumentation, i thought the Sentry Express integration may do the same.

[sbriceland]

@mydea - i didn't notice any changes to forking the isolation scope for this update. Do we still need to fork it? I wasn't sure as you seemed to mention that Middleware is correctly isolated in express (so long as we don't manually register our own Http Instrumentation)

[mydea]

Yes, so as long as our custom http instrumentation is there, no need to do any custom forking - this is basically what we do in there. I opened a PR to provide a way to also avoid this, but this is WIP: getsentry/sentry-javascript#13719

[sbriceland]

I'm not sure that's the case. For example, in your branch here, if you comment simply remove the Sentry.withIsolationScope surrounding the setTag & setUser, then the API will show inaccurate scope results.

Furthermore, if registerInstrumentation is not given the httpInstrumentation instance, those spans will not be exported.

The diag logger will even show that only express instrumentation is applied:

In order for Sentry scope request isolation and OTEL to export spans, I only seem to get it working by:

1. Using withIsolationScope per request and wrapping the middleware next function
2. creating my own HttpInstrumentation instance and passing it to registerInstrumentations

Here's an updated branch of mine that i've included some of your recommendations here including a custom sampler that uses wrapSamplingDecision:
https://github.com/sbriceland/sentry-debug/tree/debug-improvements

[sbriceland]

Okay it looks like i figured the major point of confusion:

• If tracesSampleRate is NOT set, Sentry will register ONLY: HttpInstrumentation
  • NOTE: SENTRY_DSN must be set as well which is very confusing.
  • IMO Sentry behavior should not change with the exception of whether it reports events or now
• If tracesSampleRate is set, Sentry WILL register both: HttpInstrumentation & ExpressInstrumentation

If you remove tracesSampleRate, only minimal instrumentation will be added (=Http

• YES, IF SENTRY_DSN is set

Express has to be added if you care about it, only http can/should be skipped here.

• Not fully true. It has to be added it you decide to NOT set tracesSampleRate

Next - to fork or not to fork

If tracesSampleRate is set (which subsequently triggers Sentry to register Express + HTTP), THEN forking the isolation scope per request is NOT necessary.

If tracesSampleRate is NOT set, the isolation scope must be manually forked per request.

Conclusion

Here are the following choices we have:

1. Do not to set tracesSampleRate, register all of our necessary instrumentation (EXCEPT for http), fork the isolation scope per request

OR

2. Set tracesSampleRate, Register all of our necessary instrumentation (EXCEPT for http & express), & do not need to fork the isolation scope.

Some wish lists from Sentry

• Better documentation describing the how the following configuration options work with and against each other in regards to OTEL, Scopes, & integrations:
  • dsn
  • skipOpenTelemetry
  • tracesSampleRate
  • integrations - Im actually not sure what happens if i decided to omit httpIntegration or the expressIntegration
• Maybe theirs a way to streamline all the options?
• It would be preferred if Sentry had a way to return a list of instrumentation that it enables internally. That way the developer can use each of them as needed with their OTEL setup.
• A better alternative would be for people that want to keep their existing OTEL setup, let us pass in the instrumentation instances INTO Sentry.intit() config

[mydea]

so, if no DSN is set, sentry is disabled and no integrations are run. Why would you care about sentry stuff being isolated if nothing is sent/done? This should not affect your app at all - but for the sake of testing sentry stuff, you always need to set a DSN in order to test how stuff behaves :)

Regarding docs, we have docs specifically for this case here: https://docs.sentry.io/platforms/javascript/guides/node/opentelemetry/custom-setup/#using-sentry-for-error-monitoring-only - there are for sure some rough edges there that we're striving to improve, but by following the docs there it should be possible to set up everything in a way that works (this page also describes the limitations around http instrumentation).

If it is the case that the http spans do not work with a custom tracer, that would be something we need to fix of course! But we have an E2E test app specifically for this scenario here: https://github.com/getsentry/sentry-javascript/blob/develop/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts which seems to me to work, http spans are sent to my custom tracer if I do not manually add the http instrumentation 🤔

[sbriceland]

no DSN is set, sentry is disabled and no integrations are run

Yea I am definitely nitpicking on that one! Ultimately, we didn't expect something like Scope behavior to change based on the libraries enabled state.

Regarding docs, have docs specifically for this case here...

100% agree - The documentation is quite vast and to IMO it's pretty darn good. Admittedly our setup quite custom and we more than appreciate your time in helping us understand how all of the config works together.

which seems to me to work, http spans are sent to my custom tracer if I do not manually add the http instrumentation

You are correct. Sentry's registration of both HttpInstrumentation & ExpressInstrumentation properly export spans. It just took a bit of work to understand the best way to set it up for our custom needs.

I opened a PR to provide a way to also avoid this, but this is WIP: getsentry/sentry-javascript#13719

I think your idea to provide a method that returns the instance of HttpInstrumentation registered by Sentry is fabulous. It might be even better if Sentry could return a list of all registered instrumentation instances. For example, I can see Sentry registers Http and Express instrumentation based on our setup here.

[mydea]

Thanks you for all the feedback! We will continue to work on improving the http instrumentation story - we are investigating a few options there, but we def. want to make this as un-confusing as possible 🙏 Conversations like this help us a lot in figuring out rough spots in our docs and setup and making this as easy to use as we can! I hope you've got stuff working for now - if you have any more questions, please never hesitate to reach out to us!

[sbriceland]

Sounds great! Thanks for all your time - we actually deployed some updates yesterday that appear to be working as intended.