Skip to content

fix(profiling): require --time-hint for flamegraph and profile-types trace scoping - #878

Merged
platinummonkey merged 3 commits into
DataDog:mainfrom
AlexJF:fix/profiling-trace-time-hint
Oct 2, 2026
Merged

platinummonkey merged 3 commits into
DataDog:mainfrom
AlexJF:fix/profiling-trace-time-hint

Conversation

@AlexJF

@AlexJF AlexJF commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes --trace-id scoping for pup profiling explore flamegraph and pup profiling profile-types list.

The backend's traceContext requires traceId, spanId and timeHint (epoch seconds) together, and rejects a null timeHint with a 400. Since #819, pup sent timeHint: null and didn't require --span-id, so both commands failed whenever they were scoped by trace.

  • Both commands now build traceContext via the shared trace_context_json helper from feat(profiling): add continuous profiler callgraph exploration #876. The helper requires --trace-id, --span-id and --time-hint together and converts --time-hint (Unix timestamp, RFC3339, relative time, etc.) to epoch seconds.
  • New --time-hint flag on both commands (src/main.rs).
  • Tests: the profile-types trace test now asserts timeHint, and there's a new flamegraph trace-context request-body test. The helper's own tests are in feat(profiling): add continuous profiler callgraph exploration #876.
  • docs/EXAMPLES.md: the profile-types trace example passes --time-hint.
  • docs/EXAMPLES.md: a note in the Continuous Profiler intro that pup profiling calls unstable /api/unstable/profiling/pup/... endpoints with a reduced support guarantee (latest pup release, plus 30 days for older versions).
  • docs/COMMANDS.md: a matching sub-bullet under the profiling domain entry, linking to the EXAMPLES.md note.

Motivation

Found while adding call graph (#876) and timeline (#877) exploration. Both have the same traceContext contract, and #876 applies this fix for call graph.

Additional Notes

Checklist

  • The code change follows the project conventions (see CONTRIBUTING.md)
  • Tests have been added/updated (if applicable)
  • Documentation has been updated (if applicable)
  • All CI checks pass
  • Code coverage is maintained or improved

Related Issues

Fixes trace scoping from #819.

Plan
# Fix: require --time-hint for profiling trace scoping (flamegraph, profile-types)

Spun off from PROF-16039 (timeline exploration). Full cross-repo plan:
/home/bits/.claude/plans/lets-work-on-https-datadoghq-atlassian-n-nested-dragon.md

## Context

prof-viz-java's `TraceContext` record marks `traceId`, `spanId` and `timeHint` as `@NotEmpty`, and
`FlameGraphExplorationRequest`/`ProfileTypesRequest` validate it (`@Valid`). pup (since #819) sent
`timeHint: null` and didn't require `--span-id`, so `pup profiling explore flamegraph --trace-id`
and `pup profiling profile-types list --trace-id` were rejected with a 400. The backend parses
`timeHint` as epoch seconds (or millis), matching the MCP tool contract.

The same fix landed for callgraph in #876, which introduced the shared `trace_context_json`
helper (validates the three flags together, converts `--time-hint` from any pup time format to
epoch seconds). This branch was stacked on #876 to reuse that helper; #876 is now merged and the
branch is rebased onto upstream/main (commits 8e36e95, 369eb6a; 40 profiling tests pass).

## Changes

- `src/commands/profiling.rs`: `profile_types_list` and `explore_flamegraph` take `time_hint` and
  build `traceContext` via `trace_context_json`.
- `src/main.rs`: `--time-hint` on `profiling profile-types list` and `profiling explore flamegraph`.
- Tests: profile-types trace-context test now asserts `timeHint`; new flamegraph trace-context
  body test; existing call sites updated.
- `docs/EXAMPLES.md`: profile-types trace example passes `--time-hint`; note in the Continuous
  Profiler intro that `pup profiling` uses unstable endpoints with a reduced support guarantee
  (latest pup release + 30 days for older versions); matching sub-bullet under the `profiling`
  entry in `docs/COMMANDS.md` linking to it.

## Status

- [x] Worktree `.claude/worktrees/fix-profiling-trace-time-hint`, branch
      `fix/profiling-trace-time-hint` stacked on `feat/callgraph-exploration`
- [x] Implementation + tests + docs
- [x] fmt / clippy / `cargo test profiling::` — 40 pass
- [x] Commit + push to `origin`, draft PR into DataDog/pup:main

- Rebased onto upstream/main after #877 (timeline) merged; resolved the expected COMMANDS.md
  conflict (kept `flamegraph/callgraph/timeline`, added support-window sub-bullet below). 53
  profiling tests pass.
Prompts
# Prompts log — profiling trace time-hint fix (spun off from PROF-16039)

1. (PROF-16039 context) "Lets work on https://datadoghq.atlassian.net/browse/PROF-16039, following
   similar patterns to https://github.com/ddoghq/profiling-backend/pull/9070 and
   https://github.com/ddoghq/dd-source/pull/102914"
2. "Can't we sign with my personal SSH key, open a PR against my fork AlexJF/pup and open a draft PR
   from there to upstream?"
3. "try again"
4. Asked whether to open a separate fix PR for the same timeHint bug in merged flamegraph /
   profile-types trace scoping: "yes do"
5. "On the pup PR fixing timehints, lets also add a small comment to the docs explaining that the profiling ops are using unstable endpoints with a reduced support guarantee: latest pup version + 30 days of support for older versions."
6. "Hmm just the timeline PUP PR is showing 3 commits, including the callgraph one, despite me having already merged it" (same rebase applied here)
7. "Hmm actually for the docs support window mention EXAMPLES.md is the best place. I suppose we can leave it there but I think this warrants a note in COMMANDS.md as well?"
8. "This needs a rebasing on top of latest main to fix conflicts"

🤖 Generated with Claude Code

@AlexJF
AlexJF force-pushed the fix/profiling-trace-time-hint branch from 65ec9ae to 369eb6a Compare October 2, 2026 14:08
AlexJF and others added 3 commits October 2, 2026 14:24
…trace scoping

The backend's traceContext requires traceId, spanId and timeHint (epoch
seconds) together and rejects a null timeHint, so `--trace-id` scoping on
`explore flamegraph` and `profile-types list` always failed validation.

- Build traceContext via the shared trace_context_json helper, which
  validates the three flags together and converts --time-hint to epoch
  seconds
- Add --time-hint to `profiling explore flamegraph` and
  `profiling profile-types list`
- Assert timeHint in the profile-types trace test; add a flamegraph
  trace-context request body test

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`pup profiling` calls unstable /api/unstable/profiling/pup endpoints, which
are supported for the latest pup release plus 30 days for older versions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@AlexJF
AlexJF force-pushed the fix/profiling-trace-time-hint branch from c7e191f to 8cf8b73 Compare October 2, 2026 14:30
@AlexJF
AlexJF marked this pull request as ready for review October 2, 2026 14:43
@AlexJF
AlexJF requested a review from a team as a code owner October 2, 2026 14:43
@platinummonkey
platinummonkey removed the request for review from a team October 2, 2026 14:46
@platinummonkey
platinummonkey merged commit c36042c into DataDog:main Oct 2, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants