[DEBUG-2972] Add initial support for Code Origin for Spans (Fastify entry spans only) #4449

watson · 2024-07-01T08:44:55Z

What does this PR do?

Note: This PR relates to an upcoming Datadog feature that's not available yet.

This PR does two things:

It lays the groundwork for a new feature called "Code Origin for Spans" (previously called "Span Origin").
To showcase this feature, it adds limited support for just Fastify entry-spans.

To enable, set DD_CODE_ORIGIN_FOR_SPANS_ENABLED=true.

This feature will, in its most basic form, allow users to see where in their code base a span was started/created.

Code origin is only set for spans whos callstack contains user land stack frames.

For reference, here's a PR that implements Code Origin in the Java tracer: DataDog/dd-trace-java#7001

Motivation

This makes it easier to understand what each span shown in the flame graph represents.

Performance considerations

Keeping high performance in mind, we'll only compute the code origin for certain pre-defined span types. Specifically what we call entry and exit spans.

Entry spans are defined as either the root span in a trace, or the "local" root span within a given service (it has a parent span, but that belongs to a different service).
Exit spans are defined as spans "calling" out to other services, e.g. database spans, outgoing HTTP request spans etc.

Scope

Adding support for all types of entry and exist spans in a single PR will make it harder to review, so this PR will only concern itself with adding support for Fastify entry spans; That is spans representing incoming HTTP requests to a Fastify app.

Middlewares have to be handled in a special way, as a middleware might respond early to an incoming HTTP request in which case the route handler is never called. In that instance, we want to the code origin to point to where the middleware function was registered. Doing that for Fastify however, is out of scope of this PR and support for this will be added in an upcoming PR.

Note to reviewers

Since this feature has not yet been rolled out to staging, you need a hash-link to test it. If you need access, please contact me.

Tasks

Benchmarks?

github-actions · 2024-07-01T08:45:41Z

Overall package size

Self size: 7.45 MB
Deduped: 63.22 MB
No deduping: 63.5 MB

Dependency sizes

| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/native-appsec | 8.1.1 | 18.67 MB | 18.68 MB | | @datadog/native-iast-taint-tracking | 3.1.0 | 12.27 MB | 12.28 MB | | @datadog/pprof | 5.3.0 | 9.85 MB | 10.22 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.5.0 | 2.51 MB | 2.59 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 2.0.0 | 898.77 kB | 1.3 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | jsonpath-plus | 9.0.0 | 580.4 kB | 1.03 MB | | import-in-the-middle | 1.11.2 | 112.74 kB | 826.22 kB | | msgpack-lite | 0.1.26 | 201.16 kB | 281.59 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.0 | 109.9 kB | 109.9 kB | | semver | 7.6.3 | 95.82 kB | 95.82 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | lru-cache | 7.14.0 | 74.95 kB | 74.95 kB | | ignore | 5.3.1 | 51.46 kB | 51.46 kB | | int64-buffer | 0.1.10 | 49.18 kB | 49.18 kB | | shell-quote | 1.8.1 | 44.96 kB | 44.96 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.3.1 | 25.21 kB | 25.21 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | dc-polyfill | 0.1.4 | 23.1 kB | 23.1 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | path-to-regexp | 0.1.10 | 6.38 kB | 6.38 kB | | module-details-from-path | 1.0.3 | 4.47 kB | 4.47 kB |

_{🤖 This report was automatically generated by heaviest-objects-in-the-universe}

pr-commenter · 2024-07-01T11:25:02Z

Benchmarks

Benchmark execution time: 2024-10-09 12:48:12

Comparing candidate commit fa9b265 in PR branch watson/span-origin with baseline commit 5eea208 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 263 metrics, 3 unstable metrics.

codecov · 2024-07-01T13:07:59Z

Codecov Report

Attention: Patch coverage is 91.48936% with 4 lines in your changes missing coverage. Please review.

Project coverage is 65.95%. Comparing base (bba5f3d) to head (2ff305f).
Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
packages/dd-trace/src/plugins/util/stacktrace.js	88.57%	4 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #4449       +/-   ##
===========================================
- Coverage   88.04%   65.95%   -22.09%     
===========================================
  Files         294      285        -9     
  Lines       12266    12359       +93     
  Branches       33       33               
===========================================
- Hits        10799     8151     -2648     
- Misses       1467     4208     +2741

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

watson · 2024-07-19T13:17:24Z

[DEBUG-2972] Add initial support for Code Origin for Spans (Fastify entry spans only) #4449 👈
master

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @watson and the rest of your teammates on Graphite

shatzi

very cool

shatzi · 2024-10-07T15:51:14Z

packages/dd-trace/src/plugins/util/stacktrace.js

+
+    return returnTopUserLandFrameOnly
+      ? callsite
+      : (i === 0 ? callsites : callsites.slice(i))


not sure I follow this logic...

The algorithm is as follows:

We start with an array of callsites. The top n callsites might not be user-land callsites, so we want to remove those from the array. In case i === 0 it means that the top callsite is actually a user-land frame, so there's no need to slice the array (which will shallow-clone it). Additionally, we want to limit the number of frames to limit. If there's less callsites in the array then the max limit and i === 0, again, there's no need to call slice.

packages/dd-trace/src/plugins/util/stacktrace.js

This commit does two things: 1. It lays the groundwork for a new feature called "Code Origin for Spans". 2. To showcase this feature, it adds limited support for just Fastify entry-spans. To enable, set `DD_CODE_ORIGIN_FOR_SPANS_ENABLED=true`.

juan-fernandez

LGTM. Only some minor comments

packages/datadog-instrumentations/src/fastify.js

juan-fernandez

LGTM, but it might be worth waiting for some reviewer with more experience with the fastify plugin 😄

rochdev · 2024-10-09T18:19:15Z

packages/datadog-instrumentations/src/fastify.js

@@ -27,6 +28,11 @@ function wrapFastify (fastify, hasParsingEvents) {
      app.addHook('preHandler', preValidation)
    }

+    // No need to add the onRoute hook unless Code Origin for Spans is enabled
+    if (codeOriginForSpansChannel.hasSubscribers) {


This should be in the hook instead because subscribers can be enabled/disabled at runtime.

rochdev · 2024-10-09T18:22:48Z

packages/datadog-instrumentations/src/fastify.js

@@ -5,6 +5,7 @@ const { addHook, channel, AsyncResource } = require('./helpers/instrument')

 const errorChannel = channel('apm:fastify:middleware:error')
 const handleChannel = channel('apm:fastify:request:handle')
+const codeOriginForSpansChannel = channel('datadog:code-origin-for-spans')


There shouldn't be explicit references to tracing in instrumentation.

Also, channels are easier to reason about if they match the event that they represent. For example, different subscribers may be interested in different aspects of routing, so it would be better to have something like a routeChannel instead which would add the route when any subscriber is interested in that.

rochdev · 2024-10-09T18:26:05Z

packages/datadog-plugin-fastify/src/index.js

      this.setFramework(req, 'fastify', this.config)
+      const tags = routeConfig?.[kCodeOriginForSpansTagsSym]
+      if (tags) this.setSpanTags(req, tags)


Could this be done in a separate plugin instead? The reasoning is that adding product-specific code directly to plugin increases size and complexity even when those products are not used, whereas we could disable product-specific plugins entirely and not even import them when a product is disabled for example.

That would actually also remove the need to modify web since you could then have something like setOrigin on your di-specific base class.

rochdev · 2024-10-09T18:27:04Z

packages/datadog-plugin-fastify/test/index.spec.js

@@ -542,7 +542,193 @@ describe('Plugin', () => {
            })
          }
        })
+
+        describe('with tracer config codeOriginForSpansEnabled', () => {


Is this name Node-specific or has it been standardized? Mostly asking because it exposes implementation details in the name.

watson added enhancement New feature or request semver-minor integration-fastify labels Jul 1, 2024

watson self-assigned this Jul 1, 2024

watson changed the title ~~[DEBUG-2503] Add experimental support for span origin~~ [DEBUG-2503] Add support for span code origin Jul 3, 2024

watson force-pushed the watson/span-origin branch 2 times, most recently from 4f6732a to cf52e47 Compare July 4, 2024 10:07

watson force-pushed the watson/span-origin branch from cf52e47 to 297bdfa Compare July 19, 2024 13:17

watson changed the title ~~[DEBUG-2503] Add support for span code origin~~ [DEBUG-2503] Add initial support for Code Origin for Spans (Fastify only) Oct 7, 2024

watson force-pushed the watson/span-origin branch from 297bdfa to 2ff305f Compare October 7, 2024 10:06

shatzi reviewed Oct 7, 2024

View reviewed changes

watson changed the title ~~[DEBUG-2503] Add initial support for Code Origin for Spans (Fastify only)~~ [DEBUG-2972] Add initial support for Code Origin for Spans (Fastify only) Oct 8, 2024

watson force-pushed the watson/span-origin branch from 2ff305f to bdbed4f Compare October 9, 2024 06:28

watson changed the title ~~[DEBUG-2972] Add initial support for Code Origin for Spans (Fastify only)~~ [DEBUG-2972] Add initial support for Code Origin for Spans (Fastify entry spans only) Oct 9, 2024

watson force-pushed the watson/span-origin branch from bdbed4f to 5c53936 Compare October 9, 2024 07:37

watson force-pushed the watson/span-origin branch from 5c53936 to 4d48155 Compare October 9, 2024 09:45

watson marked this pull request as ready for review October 9, 2024 10:10

watson requested review from a team as code owners October 9, 2024 10:10

watson requested a review from liashenko October 9, 2024 10:10

juan-fernandez reviewed Oct 9, 2024

View reviewed changes

packages/datadog-instrumentations/src/fastify.js Outdated Show resolved Hide resolved

packages/datadog-instrumentations/src/fastify.js Outdated Show resolved Hide resolved

Address review comments

fa9b265

watson requested review from juan-fernandez and shatzi October 9, 2024 12:41

juan-fernandez approved these changes Oct 9, 2024

View reviewed changes

rochdev requested changes Oct 9, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DEBUG-2972] Add initial support for Code Origin for Spans (Fastify entry spans only) #4449

[DEBUG-2972] Add initial support for Code Origin for Spans (Fastify entry spans only) #4449

watson commented Jul 1, 2024 •

edited

Loading

github-actions bot commented Jul 1, 2024 •

edited

Loading

pr-commenter bot commented Jul 1, 2024 •

edited

Loading

codecov bot commented Jul 1, 2024 •

edited

Loading

watson commented Jul 19, 2024

shatzi left a comment

shatzi Oct 7, 2024

watson Oct 8, 2024

juan-fernandez left a comment

juan-fernandez left a comment

rochdev Oct 9, 2024

rochdev Oct 9, 2024

rochdev Oct 9, 2024

rochdev Oct 9, 2024

rochdev Oct 9, 2024

rochdev Oct 9, 2024

[DEBUG-2972] Add initial support for Code Origin for Spans (Fastify entry spans only) #4449

Are you sure you want to change the base?

[DEBUG-2972] Add initial support for Code Origin for Spans (Fastify entry spans only) #4449

Conversation

watson commented Jul 1, 2024 • edited Loading

What does this PR do?

Motivation

Performance considerations

Scope

Note to reviewers

Tasks

github-actions bot commented Jul 1, 2024 • edited Loading

Overall package size

pr-commenter bot commented Jul 1, 2024 • edited Loading

Benchmarks

codecov bot commented Jul 1, 2024 • edited Loading

Codecov Report

watson commented Jul 19, 2024

shatzi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juan-fernandez left a comment

Choose a reason for hiding this comment

juan-fernandez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

watson commented Jul 1, 2024 •

edited

Loading

github-actions bot commented Jul 1, 2024 •

edited

Loading

pr-commenter bot commented Jul 1, 2024 •

edited

Loading

codecov bot commented Jul 1, 2024 •

edited

Loading