You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository was archived by the owner on Nov 19, 2025. It is now read-only.
Once initialized, your application can continue as expected:
26
68
27
-
## Things that could possibly improve in the long term
69
+
```js
70
+
//...example express setup
71
+
constexpress=require('express');
72
+
constport=3000;
73
+
constapp=express();
28
74
29
-
1. Due to the nature of node coverage profiling (essentially calling `takeCoverage` again and again and using snapshots), there doesn't seem to be a pause/resume method on collecting coverage. This makes it so, even on logic we don't want to track coverage on, coverage tracking is still running. This can have a bad impact on performance.
30
-
- Calling `stopCoverage` is not a possibility, because once `stopCoverage` is called, `takeCoverage` starts raising errors
31
-
- It doesn't seem (or at least first experiments don't seem to show) this has a bad impact on performance. Probably because it's native. The imaect from this is way less than the impact of saving and reading data from disk
75
+
app.get('/', (req, res) => {
76
+
res.send('Hello World!');
77
+
})
78
+
79
+
```
80
+
81
+
## Current Caveats and Limitations
82
+
83
+
### NodeJS and `takeCoverage`
84
+
This package relies heavily on the `takeCoverage` and other supporting methods added to with Node 15.1.0. While these methods are generally useful and allow Impact Analysis to function properly, there are some caveats to consider:
85
+
86
+
1. Due to the nature of node coverage profiling (essentially calling `takeCoverage` again and again and using snapshots), coverage tracking cannot be paused, and may run in a way that poses an impact on performance.
87
+
- On approach to address this problem, calling `stopCoverage` is not a possibility, because once `stopCoverage` is called, `takeCoverage` raises errors.
88
+
- Initial experiments do not indicate a significant hit to performance, likely due to `takeCoverage()`_et al_ being native.
32
89
2. There seems to be no way to avoid the disk-write in a naive way
33
-
- If we can map `NODE_V8_COVERAGE` to memory, that could be more performant
34
-
3. For problems 1. and 2. we might be able to go one level lower with the profiler. But it involved calling C code more directly, which I don't understand enough to do.
35
-
3. Getting the saved filename is a bit error-prone. We can't know for sure what the filename will be because it's generated on the fly: https://github.com/nodejs/node/blob/c18ad4b01297548582a04000aae5ba7d862377f5/src/inspector_profiler.cc#L172 So there is the possibility that at some point we will use the wrong filename in some race condition.
36
-
4. There seems to be a bug in node opentelem in that is calls spancontext as a function, but that is not a function. We have to solve it before people use this.
- There was one more place. But I can't find it now
39
-
5. The output from the profiler is in the format:
90
+
- If `NODE_V8_COVERAGE` could be mapped to memory, that could be more performant solution.
91
+
3. Getting the saved filename is a bit error-prone. We can't know for sure what the filename will be because it's generated on the fly. See: [https://github.com/nodejs/node/.../src/inspector_profiler.cc#L172](https://github.com/nodejs/node/blob/c18ad4b01297548582a04000aae5ba7d862377f5/src/inspector_profiler.cc#L172) So there is the possibility that at some point race conditions may occur, although this is not likely.
92
+
4. There seems to be a bug in node opentelem in that calls `spancontext` as a function, but that is not a function. Before fielding this in a production context, opentelemetry-js will need a fix.
93
+
- example of incorrect use: [https://github.com/open-telemetry/opentelemetry-js/.../src/export/BatchSpanProcessorBase.ts#L83](https://github.com/open-telemetry/opentelemetry-js/blob/610808d3b64b9f660f4dd640ad961b8c9f67be66/packages/opentelemetry-sdk-trace-base/src/export/BatchSpanProcessorBase.ts#L83)
94
+
95
+
5. There are some minor concerns with block coverage versus line coverage. For example, the output from the profiler is in the format:
40
96
```
41
97
// ...
42
98
{
@@ -58,11 +114,14 @@ will not have the interface to V8 that we need to collect traces.
58
114
}
59
115
// ...
60
116
```
61
-
which shows byte ranges (1062 to 1107 in this case). This means that coverage is more on the statement block level, rather than line coverage. To convert to line coverage, we need to make some smart analysis to tell which lines are being executed. Otherwise the best we can do is say: bytes A to B involves lines C to D, therefore all lines from C to D are covered (and some easy removals like empty lines). But this is not really exact and we might need some static analysis to tell which lines are actually lines. For now we are doing the naive thing, in the hopes this is enough for this version.
62
-
6. We are assuming that the "byte intervals" that show up in node coverage are presented in pre-order when looking at the interval tree. If that is correct, we don't need to reorder the intervals. If that is not, we need to reorder to avoid one interval wrongly overwriting its subintervals data.
63
-
- We are reordering it, but we are still assuming they are tree intervals and that there will be no unusual overlaps (as in, two intervals that overlap but are not contained one inside another)
64
-
7. Due to the nature of async js, opentelemetry tracks the request from the moment it comes until the moment it is responded. So for example, on:
65
-
```
117
+
118
+
which shows byte ranges (1062 to 1107 in this case). This means that coverage is on the statement block level, rather than line coverage. To compensate for this discrepancy, for now, this package assumes that if bytes A to B involve lines C to D, then all lines from C to D are covered.
119
+
6. This package assumes that the "byte intervals" that show up in node coverage are presented in pre-order when looking at the interval tree. This package makes no assumption that byte intervals are presented in pre-order, and thus will reorder if needed, However, the package still assumes they are tree intervals and that there will be no unusual overlaps (as in, two intervals that overlap but are not contained one inside another).
120
+
121
+
### OpenTelemetry Caveats
122
+
1. Due to the nature of async js, opentelemetry tracks the request from the moment it is received until the moment of response. So for example, on:
The opentelemetry will execute `onEnd` right after `res.send` happens. Which means that it won't wait for the extra logic to run. I don't know if the extra logic post-response is a common practice at Node, but we have no way of checking if coverage is run there, even though it does run.
137
+
Opentelemetry will execute `onEnd` right after `res.send` happens. Which means that it won't wait for the extra logic to run.
79
138
80
-
8. Connected to the above point, but a different problem: there is the fact that js coverage output doesn't seem to split a 'stataments block' into two when needed. The idea is that two consecutive statements, unless separated by an if/while/return or whatnot, are always either both be executed or neither (which makes sense on its own)
81
-
- So, still on the above example, lines 69 (`let a ...;`) and 70 (`let b = ...;`) are in the same statement block as line 67 (`console.log("WE...")`). Due to the async nature of js, coverage stopped tracking before they were executed, so they should not show up on the coverage result. But they do, because since line 67 was executed, and they are part of the same statement block, it doesn't make sense for them to not have been executed.
82
-
- One might think "Great, this undoes the problem from item 7.". But the issue is inside the `if`, for example. Line 72 (`console.log("It's higher...")`) also clearly runs on some cases (where a > 10), but is always considered not covered on the reports, because it is on a separate statement block and happens after a res.send.
83
-
- It's almost like the coverage tooling static analysis that produces the execution tree is not considering the dynamic possibilities of the code.
84
-
9. It's not clear to me that the callbacks on export are working. We set it seemingly right, but it's hard to tell if the tool expects something else.
139
+
2. JS coverage output doesn't seem to split a 'stataments block' into two when needed. The idea is that two consecutive statements, unless separated by an if/while/return/etc, are always either both executed or neither.
140
+
- So, still on the above example, lines 68 (`let a ...;`) and 69 (`let b =...;`) are in the same statement block as line 66 (`console.log("WE...")`). Due to the async nature of js, coverage stopped tracking before they were executed, so they should not show up on the coverage result. But they do, because since line 66 was executed, and they are part of the same statement block, it doesn't make sense for them to not have been executed.
141
+
- While this is generally preferred, problems do arise in some cases. Consider the `if` statement on line 70, for example. Line 71 (`console.log("It's higher...")`) also clearly runs on some cases (where `a >10`), but is always considered not covered on the reports, because it is on a separate statement block and happens after a `res.send`.
0 commit comments