Skip to content

Type-check trace_context.rb (remove Steepfile ignore) - #6125

Closed
p-datadog wants to merge 1 commit into
type-trace-span-idfrom
type-trace-context-typecheck
Closed

Type-check trace_context.rb (remove Steepfile ignore)#6125
p-datadog wants to merge 1 commit into
type-trace-span-idfrom
type-trace-context-typecheck

Conversation

@p-datadog

Copy link
Copy Markdown
Member

What does this PR do?

Removes the Steepfile ignore for lib/datadog/tracing/distributed/trace_context.rb
(the W3C trace-context propagator) and resolves all 32 resulting steep errors so the
file is type-checked. rake steep:check is clean.

Stacked on #6124 (branch type-trace-span-id): that PR types TraceDigest#trace_id
as Integer?, which is what forces the one id-related fix here
(build_traceparent guarding a nil trace_id).

Signature changes:

  • Declare @fetcher / @traceparent_key / @tracestate_key and concrete
    initialize param types in trace_context.rbs.
  • Declare the Distributed constants TAG_DD_PARENT_ID, DD_PARENT_ID_DEFAULT,
    TID, TAG_TID in sig/datadog/tracing/metadata/ext.rbs (present in the .rb,
    missing from the .rbs).
  • build_traceparent returns String?; extract_tracestate returns the 6-tuple
    uniformly (drops the String | alternative).

Code changes (type-narrowing; no behavior change for well-formed input):

  • Guard the extract_traceparent tuple before destructuring so trace_id /
    parent_id / sampled / trace_flags are non-nil Integer.
  • Extract version[0] / version[1] into locals with a nil check (guaranteed
    non-nil by the preceding version.size == 2).
  • extract_tracestate returns [tracestate, nil, nil, nil, nil, nil] instead of a
    bare String when there is no dd= entry (destructures identically at the call site).
  • Annotate the extract_datadog_fields accumulators and narrow the t.-tag
    key / value.

Two behavior refinements (covered by the existing 478 distributed specs, all green):

  • A malformed t. tracestate tag with no value is skipped instead of raising and
    discarding the entire tracestate for that propagator.
  • A digest with a nil trace_id produces no traceparent header, without logging an
    error (previously it raised and was swallowed upstream — same header output).

Motivation:

trace_context.rb was ignored by steep purely due to accumulated type debt; the
signatures were otherwise already written. Un-ignoring it removes a checking gap on a
security/correctness-sensitive propagator.

Change log entry

None.

Additional Notes:

Signature/type-checking change plus small type-narrowing refactors. No public API change.

How to test the change?

bundle exec rake steep:check (clean) and bundle exec rspec spec/datadog/tracing/distributed/
(478 examples, 0 failures, 1 pre-existing pending).

Removes the Steepfile ignore for the W3C trace-context propagator and resolves
all resulting steep errors:

- Declare @fetcher/@traceparent_key/@tracestate_key in the RBS.
- Declare the Distributed constants TAG_DD_PARENT_ID/DD_PARENT_ID_DEFAULT/TID/
  TAG_TID in metadata/ext.rbs.
- Guard the extract_traceparent tuple before destructuring so span/trace ids and
  flags are non-nil Integer.
- Guard build_traceparent when digest.trace_id is nil (Integer? from #6124).
- Normalize extract_tracestate to always return the 6-tuple.
- Annotate the extract_datadog_fields accumulators and narrow t.-tag key/value.

Behavior refinements (all specs pass): a malformed `t.` tag with no value is
skipped instead of raising and discarding the whole tracestate; a digest with
nil trace_id produces no traceparent header without logging an error.
@p-datadog p-datadog added the AI Generated Largely based on code generated by an AI or LLM. This label is the same across all dd-trace-* repos label Jul 29, 2026
@dd-octo-sts dd-octo-sts Bot added the tracing label Jul 29, 2026
@pr-commenter

pr-commenter Bot commented Jul 29, 2026

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-07-29 22:02:05

Comparing candidate commit 789fa03 in PR branch type-trace-context-typecheck with baseline commit 86fd9b2 in branch type-trace-span-id.

📊 Benchmarking dashboard

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

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

Unstable benchmarks

These benchmarks have a confidence interval too wide to call a change; treat them as noise rather than signal.

scenario:tracing - trace.to_digest - Continue

  • unstable throughput [-1182.657op/s; +1804.888op/s] or [-4.042%; +6.169%]

@p-datadog p-datadog closed this Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI Generated Largely based on code generated by an AI or LLM. This label is the same across all dd-trace-* repos tracing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants