angular / angularfire

Angular + Firebase = ❤️
https://firebaseopensource.com/projects/angular/angularfire2
MIT License
7.64k stars 2.2k forks source link

enableProdMode stops AngularFirePerformanceModule? #2502

Closed jonbcampos-alto closed 3 years ago

jonbcampos-alto commented 4 years ago

Version info

"@angular/animations": "~9.1.0", "@angular/cdk": "~9.2.0", "@angular/common": "~9.1.0", "@angular/compiler": "~9.1.0", "@angular/core": "~9.1.0", "@angular/fire": "^6.0.0", "@angular/forms": "~9.1.0", "@angular/material": "^9.2.0", "@angular/platform-browser": "~9.1.0", "@angular/platform-browser-dynamic": "~9.1.0", "@angular/platform-server": "~9.1.0", "@angular/router": "~9.1.0", "firebase": "^7.14.6",

Other (e.g. Ionic/Cordova, Node, browser, operating system): browser

How to reproduce these conditions

I have multiple environments: integration, testing, staging, production. AngularFirePerformanceModule doesn't seem to work in anything except for integration. The only difference between these environments is the environment variables, and the only one that I can think of that makes materially differences is the enableProdMode from production: true.

Failing test unit, Stackblitz demonstrating the problem

Steps to set up and reproduce

see above. 1 project, 2 env variables

Sample data and security rules export const environment = { production: false }

vs

export const environment = { production: true }

Debug output

Errors in the JavaScript console no console log errors

Output from firebase.database().enableLogging(true); not db related

Screenshots can't show screenshots of working performance for other reasons

Expected behavior

performance logged

Actual behavior

doesn't even show that the sdk is setup. contacted google support and they say they haven't gotten anything from this project (in production)

jamesdaniels commented 4 years ago

Perhaps the same as #2505, see my comment on that thread for a possible work around.

vamsiambati commented 4 years ago

Using the below code in app.component is registering all the logs in production mode constructor(private perfSvc:AngularFirePerformance) { this.perfSvc.dataCollectionEnabled.then(dcp=>{ console.log('dcp enabled',dcp) }) }

kyleabens commented 4 years ago

@jonbcampos-alto I think I'm having the same issue as you. My performance monitoring was logging just fine in the Firebase dashboard up until a couple months ago. Now nothing is being sent over. Do we have to include PerformanceMonitoringService as a provider now since v6?

jonbcampos-alto commented 4 years ago

based on the thread it looks like the tree shaking is overly aggressive and stopping this functionality from working. until there is a fix I think the only option is to turn prod: false. which I don't recommend.

On Thu, Jul 16, 2020 at 10:51 AM Kyle Abens notifications@github.com wrote:

@jonbcampos-alto https://github.com/jonbcampos-alto I think I'm having the same issue as you. My performance monitoring was logging just fine in the Firebase dashboard up until a couple months ago. Now nothing is being sent over. Do we have to include PerformanceMonitoringService as a provider now since v6?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/angular/angularfire/issues/2502#issuecomment-659502242, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKJW2HWGZMRTJIB7N4NPLALR34OX3ANCNFSM4N5COB7A .

--

Jonathan Campos CTO and Vice-President Of Engineering

E: jonathan@ridealto.com | W: ridealto.com P: 214-682-4070 900 Dragon St. Suite 100 Dallas, TX 75207

https://www.facebook.com/ridealto/ https://www.instagram.com/ridealto/ https://www.linkedin.com/company/ridealto/ https://twitter.com/ridealto?lang=en

madHorse54 commented 3 years ago

Here my solution:

  1. in your app.module.ts import INSTRUMENTATION_ENABLED

import { AngularFirePerformanceModule, PerformanceMonitoringService, INSTRUMENTATION_ENABLED } from '@angular/fire/performance';

  1. and set INSTRUMENTATION_ENABLED to true

providers: [ . . . PerformanceMonitoringService, {provide: INSTRUMENTATION_ENABLED, useValue: true}, . . . ],

jonbcampos-alto commented 3 years ago

tested @madHorse54 's solution. no effect.

Splaktar commented 3 years ago

It's possible that this was broken due to lint fixes similar to how https://github.com/angular/angularfire/commit/23e2f1e2e3909fd07c8f56106226e69a2514e6b5 caused https://github.com/angular/angularfire/issues/2505.

The related lint fixes for Performance are in https://github.com/angular/angularfire/commit/ae0ef48a78318907cc9307f4ca167bea8ef7d21e. I'm going to take a look and do some debugging there.

jamesdaniels commented 3 years ago

Yeah, looks like this is the case https://github.com/angular/angularfire/commit/ae0ef48a78318907cc9307f4ca167bea8ef7d21e#diff-473350a8df09472fdfe71659bcae05daR36 the logic with the "default" was broken in that commit. If it's null, which is what an @Optional defaults to, it will disable metrics. I'm testing, will cut an RC with a fix for this and the analytics shortly.

Splaktar commented 3 years ago

Yeah, I saw that too but I ran into some issues trying to get the full cycle of build lib, yarn in sample project, ng serve in sample project, etc to see the changes. I haven't found any contributor docs that explain how to best do this development cycle for this project. It seems like I need to rm -rf sample/node_modules/;yarn after every time that I run yarn build to build the library.

Splaktar commented 3 years ago

Posted PR https://github.com/angular/angularfire/pull/2595 to solve this after I was finally able to verify the fix locally.

Splaktar commented 3 years ago

Here my solution:

  1. in your app.module.ts import INSTRUMENTATION_ENABLED

import { AngularFirePerformanceModule, PerformanceMonitoringService, INSTRUMENTATION_ENABLED } from '@angular/fire/performance';

  1. and set INSTRUMENTATION_ENABLED to true

providers: [ . . . PerformanceMonitoringService, {provide: INSTRUMENTATION_ENABLED, useValue: true}, . . . ],

In the meantime, I tried this workaround, but it didn't work for me. I also tried applying the fix in my node_modules/@angular/fire/ directory directly, but that also did not work for me. I guess that I'll wait for the next canary or release 😄

jamesdaniels commented 3 years ago

Will be fixed in 6.0.3 which should be up on NPM shortly.

Splaktar commented 3 years ago

While I verified that the changes in PR https://github.com/angular/angularfire/pull/2595 alone fixed this, I didn't test PR https://github.com/angular/angularfire/pull/2597 locally. Now that 6.0.3 is released and I've tested it on a few sites, I can confirm that this is not fixed. I am still not getting Firebase Performance logs sent to the server from any of the apps.

It looks like maybe this was actually needed?

HerbertBodner commented 3 years ago

Sorry to confirm that it is NOT fixed. I built a completely new angular app from scratch with v6.0.3 and performance data is only logged when I deploy with: ng build; firebase use my-project; firebase deploy --only hosting

but not with: ng build --prod; firebase use my-project; firebase deploy --only hosting

-----------Update----------- It turns out that custom metrics work fine.

kyleabens commented 3 years ago

Any update on this or workarounds for the time being with v6.0.3?

HerbertBodner commented 3 years ago

I think that tree shaking is the issue. My workaround: use a custom trace somewhere at startup (eg logon), then also the non-custom/built-in traces seem to work fine!

Splaktar commented 3 years ago

I think that tree shaking is the issue. My workaround: use a custom trace somewhere at startup (eg logon), then also the non-custom/built-in traces seem to work fine!

Thank you for this information. @jamesdaniels I was able to confirm that this is the case. Just adding a simple start/stop trace in your AppComponent seems to avoid having the PerformanceModule tree shaken and then performance logging seems to start working (I need to wait 24 hours to verify that all of the built-in data appears).

jamesdaniels commented 3 years ago

Odd... AngularFirePerformanceModule makes a dummy call to AngularFirePerformance, which should keep it from tree-shaking out & trip the lazy load. Maybe that's getting shaken out since I'm not doing anything with it or the implementation of the promise in AngularFirePerformance requires it to be awaited.

Splaktar commented 3 years ago

@jamesdaniels initially I tried in an app to just do this.angularFirePerformance.trace('app-component-trace') and that got tree shaken.

Then I tried

    this.angularFirePerformance.trace('app-component-trace').then((trace: Trace) => {});

And that seemed to be tree shaken too.

I finally settled on the following to make it start working:

  trace: Trace | null = null;

  constructor(private angularFirePerformance: AngularFirePerformance) {
    this.angularFirePerformance.trace('app-component-trace').then((trace: Trace) => {
      trace.start();
      this.trace = trace;
    });
  }

  ngOnInit(): void {
    if (this.trace) {
      this.trace.stop();
    }
  }

I didn't use await since it complained that the constructor wasn't an async function.