Hybrid Tracing (DO NOT MERGE) by DarcyRaynerDD · Pull Request #1 · DataDog/datadog-lambda-js

DarcyRaynerDD · 2019-06-03T16:38:39Z

This contains the first approach for hybrid tracing using node.

How it works

We implement hybrid tracing by piggbacking on AWS Xray traces. In a lambda environment, AWS sets the environment variable _X_AMZN_TRACE_ID, which usually has a value formatted like this:

_X_AMZN_TRACE_ID="X-Amzn-Trace-Id: Root=1-aaaaaaaa-aaaaaaaaaaaaaaaaaaaaaaaa;Parent=aaaaaaaaaaaaaaaa;Sampled=1"

We convert this, (deterministically), into Datadog hybrid tracing headers, and attach to every outgoing request.

x-datadog-trace-id: 1111111111111111
x-datadog-parent-id: 1111111111111111
x-datadog-sampling-priority: 2

If the lambda received the Datadog hybrid tracing headers in a request, those values are used instead of the ones derived from xray. We also save those headers into a dummy xray subsegment, (so the crawler can read them).

…est headers

swfunc

I left a few optional notes mostly asking to clarify a few things with more comments. Overall it looks great. The tests are 💯.

swfunc · 2019-06-03T21:02:55Z

+function patchMethod(mod: typeof http | typeof https, method: "get" | "request", contextService: TraceContextService) {
+  shimmer.wrap(mod, method, (original) => {
+    const fn = (arg1: any, arg2: any, arg3: any) => {
+      const { options, callback } = normalizeArgs(arg1, arg2, arg3);


Could you add a comment here to clarify what's happening with the arg normalizing / why it's needed?

swfunc · 2019-06-03T21:03:25Z

+  }
+}
+
+function normalizeArgs(


A docstring here would be helpful

swfunc · 2019-06-03T21:05:10Z

+  return { options, callback };
+}
+
+function requestOptionsWithTraceContext(


do you think it would be more clear to rename this to something like getRequestOptionsWithTraceContext? I was a little confused at first thinking this was making the request

swfunc · 2019-06-03T21:30:49Z

@@ -0,0 +1,3 @@
+export { patchHttp } from "./patch-http";
+export { extractTraceContext as readTraceContext } from "./context";


what's the reason for renaming this?

Good catch, I think that was an accidental automatic refactoring.

swfunc · 2019-06-03T21:34:08Z

+ * Reads the trace context from either an incoming lambda event, or the process environment.
+ * @param event An incoming lambda event. This must have incoming trace headers in order to be read.
+ * @param env The process environment that may contain an xray trace id environment variable. This we be used
+ *  if the event doesn't contain trace headers.


could you add something here about how the _X_AMZN_TRACE_ID is set to be unique for each execution by X-Ray? I was a little confused at first thinking the env variable was set by the user for some reason

swfunc · 2019-06-03T21:37:34Z

+
+import { convertToAPMParentID, TraceContext } from "./context";
+
+export class TraceContextService {


Could you add a short comment here about the class?

swfunc · 2019-06-03T21:42:14Z

+    try {
+      addTraceContextToXray(trace);
+    } catch {
+      // This might fail if running in an environment where xray isn't set up, (like for local development).


do you think it would be helpful for customers if to add a log line here?

swfunc

thanks for the changes 👍

…804) * make integration tests run local instead of using real aws service * test(local): keep cold_start asserted in local integration snapshots Remove the cold_start value normalization from the local harness pipeline. The cold->warm transition (invoke #1 cold_start:true, invokes #2..N false) is deliberate coverage and is deterministic locally, since proactive initialization cannot happen unless SIMULATE_PROACTIVE_INIT=true. Snapshots regenerated with real cold_start values. proactive_initialization markers and the '(init: N ms)' END suffix remain stripped: they reflect platform scheduling, not code behavior. The real-AWS run_integration_tests.sh is intentionally untouched; proactive-init cold_start flakes there are handled by rerun. * ci: run local integration tests in GitHub Actions Adds a workflow that runs the RIE-based harness from integration_tests_local/ on every PR and main push: one job per Node major version (18/20/22/24), each building and testing both container variants against the real AWS Lambda base images. No AWS credentials needed. Uses linux/amd64 images on the x86_64 runners. * test(local): drop 'container-' prefix from snapshot file names Snapshot files are now named <variant>_node<runtime>[...] (e.g. esm_node18.log) instead of container-<variant>_node<runtime>[...]. The docker image tags and function names keep the container- prefix; only the snapshot paths changed. * Sign the datadog-lambda-js ci image (#809) * Pin serverless version (#810) * Remove use of Github secrets (#811) * Add Node26 runtime (#808) * Add Node26 runtime * update snapshots * fix integration test path * node version compatibilty * filtering snapshots * cleanup * add migration_parity.md doc * update * add node26 * Harden local RIE integration harness (Phase 0 PR 1) Prepare the docker/RIE golden harness to serve as the compatibility oracle for the dd-trace-js migration, before any migrated code runs against it. Oracle correctness: - normalize.sh: `run_id_filter` defaulted to 's/$/^/', which is not a no-op -- it appended a literal '^' to every output line, and all eight committed log goldens carried one. Setting RUN_ID would have flipped the filter and broken them all at once. Default is now a genuine pass-through, and the goldens are updated by that byte-provable transform only (strip the trailing '^', strip RIE's trailing REPORT tab); no golden was regenerated by running code. - normalize.sh: stop wiping span `meta`/`metrics` keys -- their stable keys and values are part of the compatibility contract. (The filter was already dead: the goldens are pretty-printed and the pattern could not cross newlines.) - Trailing-blank stripping is scoped to REPORT lines, so application output remains an exact-whitespace oracle. - run.sh: drop `diff -w` from the sorted log comparison. It hid whitespace-only drift, e.g. a migrated emitter serializing '{"a":1}' where today it emits '{"a": 1}'. The tab it existed for is now removed at the source. Reliability: - Pin the RIE binary to v1.36 with a per-platform SHA-256, re-hashed on every run so a stale cached download cannot survive. - Fail on missing snapshots unless UPDATE_SNAPSHOTS=true; validate RUNTIME_PARAM/VARIANT_PARAM/UPDATE_SNAPSHOTS; reject argv, which was silently ignored. - Capture the HTTP status alongside the body: `curl -s` exits 0 on a 5xx, so update mode could have recorded error pages as goldens. - Handle `diff` exit codes 0/1/>1 distinctly; a diff *error* previously read as success. - Replace fixed sleeps with polling: a TCP readiness probe (never a POST, which would consume the cold-start invocation) and a wait for both REPORT and INVOKE RTDONE per event, since RIE can emit RTDONE after REPORT. - Guard cleanup() against the blank array slots left by "${array[@]/$cid}". Node 26: - Add the leg to the harness and CI matrix. Node 26 ships to customers but has zero L2 coverage. - `public.ecr.aws/lambda/nodejs:26` does not exist yet (preview only), so pin the multi-arch tag 26-preview.2026.08.21.22 through a runtime->image-tag indirection applied only at the --build-arg; $node_version also feeds image tags, container names and snapshot paths, which must keep the bare major. - Node 26 is excluded from the default runtime list and runs in update mode in CI until PR 2 commits its snapshots. CI asserts it produces exactly 20 artifacts and that no committed snapshot is ever mutated. - Record the GA re-capture trigger: when AWS publishes the bare image, the Node 26 goldens must be re-captured from the pinned pre-migration ref. Node 18 and 20 legs are retained deliberately (C18); v5.x is the only dd-trace line serving those runtimes. Verified: all eight strict legs (18/20/22/24 x cjs/esm) pass byte-exact; the node26 update-mode leg produces exactly 20 artifacts; negative tests for missing snapshots and whitespace-only drift both fail as intended. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: share local integration goldens across runtimes and events The local harness committed one golden per (variant, runtime, event): 72 return values and 8 logs. All 72 return values were byte-identical, and the 8 logs held only one genuinely runtime-specific line each. 80 files carried 3 distinct expectations, and 80 files had to be re-read whenever one of them legitimately changed. Collapse them to 3 shared goldens covering all five runtimes and both variants: snapshots/return_values/default.json snapshots/logs/cjs.log snapshots/logs/esm.log Two changes make sharing safe rather than lossy: - Assert `runtime:nodejsNN.x` on every invocation with the major actually under test, then collapse it to `nodejsXX.x`. Without the assertion, sharing the golden would silently stop checking that the library reports the runtime it is running on. - Drop the runtime major from AWS_LAMBDA_FUNCTION_NAME. It propagates into service, resource, resource_names, functionname, function_arn, _dd.base_service and _dd.tags.process, so embedding the runtime there made every runtime's golden differ in ~100 lines of pure fixture naming, which buried the one line that was actually runtime-specific. Divergence is expressed by adding an override file, never by loosening a comparison: snapshots/logs/${variant}_node${major}.log snapshots/return_values/${variant}_node${major}_${event}.json An override wins for its leg alone, so a real behavioral difference appears as a new file in review, where widening a normalization filter to absorb it would appear as nothing. In update mode a leg that disagrees with an existing shared golden now fails instead of overwriting it; otherwise the last runtime to run would define the expectation for all of them. That agreement check uses the same sort-insensitive comparison as strict mode, since RIE's platform logging races application output. Node 26 consequently needs no snapshots of its own: its normalized output matches the shared goldens. That removes the update-mode CI override and the node26 baseline assertion, making all five workflow legs strict. The consolidation is auditable rather than a blind re-capture: reverse- substituting the runtime major and function name into the 3 shared files reproduces all 72 original return-value goldens byte-for-byte and all 8 original per-runtime logs with an identical set of lines. The only residual differences are the ordering of RIE's own platform lines (INVOKE RTDONE, START/END/REPORT), which is already compared order-insensitively. Also fixes a latent normalization hole this exposed: function_arn is normalized by a pattern whose character class excludes `_`, so names ending in `_node18` halted the match and the goldens recorded a partially normalized `"function_arn":"XXXX_node18"`. With the suffix gone the ARN normalizes completely. Verified with a full strict run: 5 runtimes x 2 variants, 110 assertions, no failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: unbreak SIMULATE_PROACTIVE_INIT under the RTDONE readiness poll The readiness poll waits for both a REPORT and an INVOKE RTDONE per invocation. RIE's managed-instances path — the one SIMULATE_PROACTIVE_INIT switches into to get eager init — emits REPORT but never RTDONE, so the poll could never be satisfied and the mode timed out at 9/0 instead of producing a usable diff. Verified directly against both RIE paths with a single invocation: default RIE REPORT=1 INVOKE_RTDONE=1 managed-instances REPORT=1 INVOKE_RTDONE=0 Require RTDONE only on the default path. The mode now runs to completion and surfaces exactly what it is meant to: cold_start flips to false on the first invocation, plus managed-instances platform noise (structured bootstrap logs and a different emulated region in ARNs). Also correct the proactive_initialization row in migration_parity.md, which claimed RIE cannot produce a proactively-initialized sandbox. It can: raw un-normalized logs from a managed-instances run with a 15s init->invoke gap contain "proactive_initialization":1, proactive_initialization:true and cold_start:false. The feature is unprotected because normalization drops those markers and the goldens come from immediate-invoke runs, not because the sandbox state is unreachable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: state plainly that SIMULATE_PROACTIVE_INIT is not a gate The section documented the mechanism in detail but never said what its status is, so a reader could reasonably assume it is part of the suite. It is not set in CI, not set in the default run, and nothing gates on it; a run with it is expected to differ from the goldens rather than match them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Rithika Narayan <93233069+rithikanarayan@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

DarcyRaynerDD and others added 17 commits May 29, 2019 17:01

Add basic dependencies for building

4be74d2

Setup basic tslint and prettier config

509f795

Add jest support

c9b634c

Update project configuration settings

be8547c

Add conversion functions for trace header

b3d45bd

Add function to parse trace header from environment

bcd3e9c

Add method to read trace id from incomming event

c7a5386

Add function to select correct context

340aa49

Add Github template and open source boilerplate

6d7008c

Implement http patching

de1690f

Add test to verify that an empty trace context won't be added to requ…

728ec96

…est headers

Merge branch 'master' of github.com:DataDog/dd-lambda-js

909d38f

Inject context from lambda wrapper

24e70d6

Add configuration options for datadog lambda wrapper

6e843b0

Add xray-sdk dependency

a04b849

Add datadog trace headers as metadata to xray

c4805b9

Capture local segment information when sending to lambda

293d0c8

swfunc reviewed Jun 3, 2019

View reviewed changes

Implement PR feedback

0565bc3

swfunc approved these changes Jun 4, 2019

View reviewed changes

DarcyRaynerDD closed this Jun 4, 2019

Navigation Menu

Repository navigation

Hybrid Tracing (DO NOT MERGE) - #1

Hybrid Tracing (DO NOT MERGE)#1
DarcyRaynerDD wants to merge 18 commits into
pr-branchfrom
master

DarcyRaynerDD commented Jun 3, 2019

Uh oh!

swfunc left a comment

Uh oh!

swfunc Jun 3, 2019

Uh oh!

swfunc Jun 3, 2019

Uh oh!

swfunc Jun 3, 2019

Uh oh!

swfunc Jun 3, 2019

Uh oh!

DarcyRaynerDD Jun 3, 2019

Uh oh!

swfunc Jun 3, 2019

Uh oh!

swfunc Jun 3, 2019

Uh oh!

swfunc Jun 3, 2019

Uh oh!

swfunc left a comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -0,0 +1,3 @@
		export { patchHttp } from "./patch-http";
		export { extractTraceContext as readTraceContext } from "./context";


		import { convertToAPMParentID, TraceContext } from "./context";

		export class TraceContextService {

Conversation

DarcyRaynerDD commented Jun 3, 2019

How it works

Uh oh!

swfunc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swfunc left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants