feat: span statistics -- ensure dropped span objects are still created#2694
feat: span statistics -- ensure dropped span objects are still created#2694
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
|
|
||
| const span = new Span(this, ...args, opts) | ||
|
|
||
| if (this._builtSpans >= this._agent._conf.transactionMaxSpans) { |
There was a problem hiding this comment.
Note: The meaning of _buildSpans ends up changing slightly in this new world. The _buildSpans property is always incremented regardless of whether a span will be dropped or not. To get the number of started spans we subtract the number of dropped spans from built spans (see below)
| context: undefined, | ||
| span_count: { | ||
| started: this._builtSpans | ||
| started: this._builtSpans - this._droppedSpans |
There was a problem hiding this comment.
The _builtSpans property now tracks all span objects that were created. To se the number of spans started we subtract the number dropped. (see above)
| newTransOrSpan = trans.createSpan(name, spanOpts) | ||
|
|
||
| // There might be no span, e.g. if transactionMaxSpans was hit. We have | ||
| // There might be no span, e.g. if the span is an exit span. We have |
There was a problem hiding this comment.
Note: transactionMaxSpans being reached now creates a span -- but sets its recorded value to false.
|
jenkins run the tests please |
trentm
left a comment
There was a problem hiding this comment.
This looks mostly good.
- I think it is worth discussing the behaviour change with other agent teams.
- Then if we are good with that change, I'd like to update or remove the OTel Bridge "hit-transaction-max-spans.js" test case, because it will no longer be testing what it was meant to be testing.
I think it might be worth creating a separate ticket to investigate the performance impact of this change when the agent does hit transactionMaxSpans. (Or perhaps we just stay aware of it, if it comes up.) For example, transactionMaxSpans is no longer any help in reducing the CPU load of calculating breakdown metrics, or of captureSpanStackTraces if that happens to be enabled (hopefully not).
|
jenkins run the tests please |
…eSpan-returns-null.js to test the intended OTelTracer.startSpan case where the internal createSpan API returns a null
trentm
left a comment
There was a problem hiding this comment.
See the 'addEndedSpan' comment below.
I also think it would good to determine if breakdown metrics calculation should be skipped for these non-recording spans (for both correctness and perf), and if gatherStackTrace could be skipped for these non-recording spans (for perf). Both of these are in Span.prototype.end. However, I'm not following up on those.
The change away from `NODE_VERSION` in #2627 surprisingly broke running the benchmarks. `NODE_VERSION` is used by nvm and exporting it (?) makes a difference?
…ue with github deps (#2696) mysql2@2.2.3 (and only that version) has a github dep: "@types/mysql": "types/mysql", Attempting to install that version with npm v6 (the npm in node v10, v12, and v14) hits npm/cli#4896 which results in an install so slow that is hits the default 2 minute 'npm install' timeout in the `tav` tool.
This adds a 'links' option to `createTransaction` and `createSpan` APIs
for specifying span links.
apm.startSpan('name', { links: [ ... ] })
https://github.com/elastic/apm/blob/main/specs/agents/span-links.md
Span links support is added to the OTel Bridge as well:
tracer.startSpan('name', { links: [ ... ] })
This adds a `traceContinutationStrategy` configuration option to allow
some control over how the APM Agent uses incoming trace-context headers
for context propagation.
https://github.com/elastic/apm/blob/main/specs/agents/trace-continuation.md
This also fixes a whitespace parsing issue in TraceState.
Closes: #2592
Closes: #2673
Closes: #2554
Obsoletes: #2555
AFAIK the "compatible runtimes" flag on Lambda layers is just advisory.
Co-authored-by: apmmachine <infra-root-apmmachine@elastic.co>
This avoids the name collision with the "exitSpan" option when creating a span in the public API. Closes: #2680
The old 'hapi' package was itself deprecated about 2 years ago. The APM agent now deprecates instrumenting it. The instrumentation code remains, but it is no longer tested and will be removed in the next major. Note that '@hapi/hapi' is still supported, this is just about the old package name and versions. Any versions starting with 17.9.0, 18.2.0, 19 and later are the newer '@hapi/hapi'. Refs: #2691
Co-authored-by: apmmachine <infra-root-apmmachine@elastic.co>
Part of #2302 (2 of 3):
This intent of this PR is to ensure that in cases when a span will be dropped that the span is still created (matching the behavior of the java agent). This is necessary for the span statistics work, as the collected statistics includes capturing the sum of time for dropped spans -- meaning that even a dropped span needs to start and end.
Prior to this work the node agent "dropping" a span meant that the span would not be created. Now, the span object is created but the span's context object has its recorded property set to false. Non-recorded spans are then excluded from sending, but otherwise have a full start/end lifecycle.
The next (and final) step after this PR is to implement the logic for capturing the dropped span statistics and sending them on to APM Server.
Checklist
[ ] Update TypeScript typings[ ] Update documentation[ ] Add CHANGELOG.asciidoc entry