chore: fixing zone from which to fork a new zone

[obecny]

Which problem is this PR solving?

• This will fix partly the bug mentioned here
  Web tracer, zone context manager and User Interaction plugin does not work with Angular #1206

Another PR will be in contrib

[codecov]

Codecov Report

Merging #1209 into master will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1209   +/-   ##
=======================================
  Coverage   92.32%   92.32%           
=======================================
  Files         122      122           
  Lines        3533     3533           
  Branches      714      714           
=======================================
  Hits         3262     3262           
  Misses        271      271           

Impacted Files	Coverage Δ
...ry-context-zone-peer-dep/src/ZoneContextManager.ts	85.36% <100.00%> (ø)

[dyladan]

Does this preclude the need for the rxjs/angular plugin, or are you still doing that?

[dyladan]

Can you add a test? Since all the tests were passing before I would like to see an example of one that would be broken without this change to more easily understand what is going on.

[obecny]

Does this preclude the need for the rxjs/angular plugin, or are you still doing that?

No the mentioned plugin will be needed in case to fully support context propagation in angular between our plugins like with promiseQ etc. So this is to fix creating new zone from active instead of root always - this was wrong from the beginning.

[dyladan]

If it was always wrong how did the tests pass?

[obecny]

If it was always wrong how did the tests pass?

cus it wasn't tested ?

[owais]

This specific case obviously wasn't tested but it raises the question if we have tests that actually test a real world scenario with some popular frameworks. In addition to unit tests that test specific behavior of some functions or classes, we should have functional or integration tests that instrument a demo application and test the full lifecycle of the app making sure expected spans are generated at the end.

[owais]

@obecny Thank you so much for the fast turn around on this. You mentioned there will be another PR in contrib to fix this and probably that's why but this fix alone does not fix the bug. You probably are obviously aware of it but just wanted to share in case there was some miscommunication.

[obecny]

This specific case obviously wasn't tested but it raises the question if we have tests that actually test a real world scenario with some popular frameworks.
We don't and to achieve that there was already issue raised long time ago

#650

Without having e2e test we can't test real world scenario

@mayurkale22 @dyladan ^^

[mayurkale22]

We need to add integration tests sooner rather than later. This is must requirement for GA / 1.0.

[dyladan]

@open-telemetry/javascript-approvers this is a high priority bugfix please review