Simplify Angular SDK Provider Setup

[Lms24]

Problem Statement

Currently, users have to add 2 to 3 providers in their app.module.ts or app.config.ts to fully configure error and performance monitoring in Angular projects. This is a bit cumbersome - let's simplify this setup!

Solution Brainstorm

As suggested by @zip-fa, we can reduce the setup to 2 LoC here, by exporting helper functions from the SDK that add the respective provider entries:

providers: [
  // returns Sentry's `ErrorHandler` provider entry
  // takes the same `ErrorHandlerOptions` that are currently passed to `createErrorHandler`
  Sentry.provideErrorHandler(errorHandlerOptions),

  // returns Sentry's `TraceService` and the `APP_INITIALIZER` entries
  // currently takes no options
  Sentry.provideTraceService(), 
]

AFAICT, these functions should work identically for both, app.module.ts and app.config.ts files.

[zip-fa]

Is it tree-shackable to use these:

import * as Sentry from '@sentry/angular-ivy'

providers: [
  // returns Sentry's `ErrorHandler` provider entry
  // takes the same `ErrorHandlerOptions` that are currently passed to `createErrorHandler`
  Sentry.provideErrorHandler(errorHandlerOptions),

  // returns Sentry's `TraceService` and the `APP_INITIALIZER` entries
  // currently takes no options
  Sentry.provideTraceService(), 
]

Maybe you should import only what you need?

import { provideErrorHandler, provideTraceService  } from '@sentry/angular-ivy'

providers: [
  // returns Sentry's `ErrorHandler` provider entry
  // takes the same `ErrorHandlerOptions` that are currently passed to `createErrorHandler`
  provideErrorHandler(errorHandlerOptions),

  // returns Sentry's `TraceService` and the `APP_INITIALIZER` entries
  // currently takes no options
  provideTraceService(), 
]

As i can see, there should be minor refactor in angular-sentry package because of linking to old angular package.
index.ts must be refactored etc..

[Lms24]

Is it tree-shackable to use these

yes, namespace imports are tree-shakeable in modern bundlers. I'd like to use namespace imports in the documentation later on because it's in line with how we document Sentry SDK usage. However, for implementing this, it shouldn't make a difference.

As i can see, there should be minor refactor in angular-sentry package because of linking to old angular package.
index.ts must be refactored etc..

My idea is that (for the moment) we make this change in the @sentry/angular package and continue to link it to @sentry/angular-ivy. I don't think we need to adjust the index.ts files for angular-ivy specifically. I'd like to keep diverging from the two packages minimal because it's not entirely clear yet, if and how we're going to maintain support for both when we start working on the new major version of our SDKs (v8).

[zip-fa]

Your idea won't work, because @sentry/angular depends on old angular version, where there is no EnvironmentProviders type export in @angular/core.
So there is two options:

1. leave everything as-is and create provider functions in @sentry/angular package, but there will be no typing in return type
2. Create independent index.ts files in both packages and create two independent files inside angular-ivy package: errorhandler.provider.ts, tracing.provider.ts

Which way do you chose? I will create PR

[Lms24]

Oh that's a good catch, thanks! I think then, we should go with option 2 indeed as it gives users the most benefits.

errorhandler.provider.ts, tracing.provider.ts

I'd prefer it if we just created one providers.ts file per package but I'll let you make the call depending on what works best in your PR.

Thanks a lot for working with us!

[zip-fa]

Okay. One more question:

There is non-angular way to init sentry with explicit call of init function in polyfills.ts or main.ts. Maybe we should create provideSentryConfig({}) provider-function? This will affect code execution before sentry init (eg. angular init, 3rd party libs etc), but i don’t think it would break something, because these kind of errors will happen only in dev env.

[Lms24]

There is non-angular way to init sentry

Just to confirm, we're talking about the Sentry.init call in main.ts? https://docs.sentry.io/platforms/javascript/guides/angular/#configure

This needs to stay as-is because the Sentry SDK needs to be initialized as early as possible in the application's life cycle. Otherwise the SDK could miss errors and performance spans that happen before Angular is bootstrapped. The SDK also catches errors without the ErrorHandler because it hooks into global error handlers like window.onerror.

because these kind of errors will happen only in dev env

hmm I'm not too sure about this. People can technically do all kinds of things before Angular is loaded and coming in as early as possible is especially important for accurate performance monitoring. For instance, this allows keeping track of Angular bootstrapping duration and more accurate pageload spans.

Unless I'm totally missing something, let's not do this for the moment and stick with the two provider functions for the error handler and tracing.

[zip-fa]

Okay now i am agree with you. I will land new PR’s with a bit changes for providers in few hours.

[zip-fa]

Hi again. I will land my PR, but i have some new observations and concerns.
You created separate "ivy" package, just to supress "view engine" deprecation error, right?
So now you struggle with esbuild and you think what to do in this case.
My proposition is to do this:

1. Rename @sentry/angular to @sentry/angular-legacy
2. Rename @sentry/angular-ivy to @sentry/angular and support only last 3 major versions

In this case, in legacy branch, you will get "View engine is deprecated" error in console, but the package will work without any issue, because view engine support is removed completely only in v16. In new "angular" package you will make angular v15 as peer dependecy and resolve issue with esbuild. How does it sound to you? I am ready to land PR with providerFunctions but i can't change code for testsing because there literally no tests dedicated to angular v12+ :)
Should i land PR without re-written test? We need 1st class support for angular, but now it looks horrible..

[zip-fa]

ah.. i can't land my PR because EnvironmentProviders type is only available from v15.
There is chaos in angular package, which must be resolved. Let's collaborate and find some way..

[Lms24]

Hey @zip-fa, you're bringing up good points and I already thought about the exact package renaming you proposed.

I talked with my team about our Angular strategy moving forward and we're probably going to bump the angular-ivy package to NG15. However, we can only do this, when we cut a new major version for the entire JS SDK repo. Dropping version support is a breaking change and we can't do that in a minor version. The new major will likely happen in a few months but not at this moment.

You created separate "ivy" package, just to supress "view engine" deprecation error, right?

Mostly, yes. For Angular versions pre NG16, our users got build warnings because the @sentry/angular package was compiled for VE (see #7265 and #5268). The package wouldn't work at all with NG16 (at which point we luckily had already created the @sentry/angular-ivy package).

1. Rename @sentry/angular to @sentry/angular-legacy
2. Rename @sentry/angular-ivy to @sentry/angular and support only last 3 major versions

Regarding your renaming proposition: I have to think about this a bit more. I completely agree that this would be the more "organic" or logical naming scheme. We had to introduce "angular-ivy" without renaming because we added the package within the v7 major. Again, renaming a package would be a hard breaking change and can only be done in a major.
Then there's the argument around upgrade paths and benefits vs. costs. Renaming provides little value for existing users but a lot of costs for upgrading to that new major. Swapping out a package isn't hard but annoying to do. We might still do it, but I can't promise anything.

Alright, so if I understand correctly, we have two problems right now to move forward with this task:

1. The correct type that the functions should return is only exported for NG15+.
   Let's solve this before we move forward and discuss (2). If Angular doesn't expose any types for object passed into the providers array, can we create our own type AFAICT the API didn't change over the versions so I assume we should be safe to "shim" the type. This would need to be tested though to make sure typescript is fine.

2. No tests in @sentry/anguly-ivy.
   That's a good point. At the time of creating the ivy package, we didn't add tests because everything was tested in @sentry/angular (albeit with an older version). Maybe this is a good reason to stop symlinking everything in the @sentry/angular-ivy package, start diverging slightly and add tests. I have a feeling this will have to happen anyway sooner or later so maybe we do it just now. Would you be comfortable with working on this? I'm also fine if this happens after the introduction of the functions. I think creating two functions that return mostly static objects shouldn't be a too risky change. Plus we can still test the @sentry/angular version of the functions with our current setup.

[zip-fa]

Your proposal sounds like hack to me, just to support everything and everywhere. I understand why you chose that way.

I don’t think there is something “breaking” in what i proposed.
Old “angular-legacy” package will be available and all effort needed to do for users is just rename package name in their package.json.
That’s much better than hacking and writing temporary code.

But okay, you who decide here. If i understood correctly, we make this:

1. Create provider functions in @sentry/angular-ivy package
2. Remove symlinks from index.ts inside ivy package
3. Create type “EnvironmentProviders” + add description that it is temporary and will be replaced with angular’s one soon

I dont think we can re-write tests now, because ng version is still at v12 - no standalone components, to standalone apis.
For me, it is better to bump everything for at least to V15 - it will open window of possibilities: standalone apis, needed type, ability to write tests, ability to refactor some places etc etc.

How about creating separate package called angular-next with every new feature, better code organisation, etc? Are you still sure that we need to hack something just to add two little functions?

[zip-fa]

Some takes about code structure. With current exports webpack will bundle tracing module (which is very heavy) when it’s not used.
With esbuild it’s not a problem, but because of this line, it will not work good/expected:

export * from '@sentry/browser';

There are many issues related to star exports, eg:

evanw/esbuild#1737

I propose to remove this re-export for newer version completly, let user choose what to import and when.
Also i propose to remove tracing export from barrel file. This will reduce bundle weight.
For now, my main.js built with webpack weights 700kb, from which 250kb is sentry.

proposed imports realisation:

import { provideSentryTracing } from ‘@sentry/angular/tracing’;
import { provideSentryErrorHandler } from ‘@sentry/angular’;

[Lms24]

Your proposal sounds like hack to me, just to support everything and everywhere

Maybe yes, but that's the price we have to pay :/ Also, supporting "everything" is important to us.

I don’t think there is something “breaking” in what i proposed.

There is because:

Rename @sentry/angular to @sentry/angular-legacy

This breaks everyone who currently uses @sentry/angular because suddenly there won't be any new version published anymore (with your second suggestion) the next patch or minor version will actually be Ivy code.

Rename @sentry/angular-ivy to @sentry/angular and support only last 3 major versions

This actually breaks everyone who uses @sentry/angular-ivy with NG12-14. There's no guarantee for backwards compatibility of Angular libraries. Also, again, people would need to switch to another package which requires a code change.

For basically the same reason, we still publish @sentry/hub and @sentry/tracing, although both packages are no longer used actively and have been hoisted/replaced with other packages.

---

My goal for this task would be to still get the helpers in both packages, if possible.

Can we do this for a minimal solution to get started?

1. Create type “EnvironmentProviders” in @sentry/angular. We add a comment where we take it from and why.
2. Create provider functions in @sentry/angular package and add some basic tests for it.
3. Check if the symlinked version still works as expected in @sentry/angular-ivy.

If we need to un-link anything then maybe we do it but I don't see why this wouldn't work.

[Lms24]

proposed imports realisation:

import { provideSentryTracing } from ‘@sentry/angular/tracing’;
import { provideSentryErrorHandler } from ‘@sentry/angular’;

I don't see why we'd need this. If users don't import provideSentryTracing, then the bundler will not include it in the final bundle. Regardless of the import path. Also worth noting, the heavy part around performance monitoring is probably the BrowserTracing integration which will only be included in the bundle if it is added to integrations in Sentry.init (main.ts).

With current exports webpack will bundle tracing module (which is very heavy) when it’s not used.

Hmm if this is true even if nothing about tracing (TraceService, TraceModule, any of our decorators and most importantly the BrowserTracing integration) is included in user code we'd need to revisit things. I haven't heard of this before and I think it's not relevant to this issue so please open a new one if this applies.

[mackelito]

What's the status on this?

[Lms24]

Hi @mackelito, not much movement here since the last answer. We can probably tackle this after we release v8 (main focus right now). Just need some time to implement it.

[mackelito]

Hi @mackelito, not much movement here since the last answer. We can probably tackle this after we release v8 (main focus right now). Just need some time to implement it.

Got it! Stay focused! 😉
Is there some temporary docs to get Sentry up and running using app.config.ts/main.ts in a angular 17 setup?

[Lms24]

I wanted to add these docs some time ago but didn't get to it: getsentry/sentry-docs#8342

Setup shouldn't be too different from app.module.ts. You mostly just need to add providers in the config: https://angular.io/guide/dependency-injection-providers#configuring-dependency-providers but I fully understand that we should add a better guide for it. I need to do some work for Angular when we work on v8 (soon) so I'll try to get this incorporated.

[Lms24]

I also posted some instruction how to configure sentry in app.config.ts here: #9376 (comment)

[Lms24]

I will close this given that it has been open for quite some time and we didn't get to it. While I think it'd be worthwhile to simplify the setup a bit, we also need to recognize that types and the provider APIs have changed over Angular versions. This might make such a new export from the SDK a bit challenging.

If anyone comes across this issue: PRs are still welcome.