feat(v2fix): expand analyzer coverage and harden suggested fix generation#4393
feat(v2fix): expand analyzer coverage and harden suggested fix generation#4393
Conversation
…improve context handling This is needed to fix the race condition that exists on concurrent evaluations.
…fixes This update introduces functionality to generate and update golden files based on suggested fixes from the analyzer. It processes diagnostics, collects edits, and formats the output accordingly, ensuring proper handling of single and multiple messages. This enhancement improves the testing workflow for the v2fix package. The warning-only golden files are generated for consistency/documentation but aren't validated by analysistest.RunWithSuggestedFixes. This is expected behavior from the analysistest package - it only compares when there are actual text edits to apply.
The tests require V1Usage to satisfy the KnownChange interface, and adding Clone() to the test helper makes that contract explicit so test scaffolding stays aligned with production interface changes without altering runtime behavior.
This updates probe context plumbing to capture type expressions, import literals, and helper logic for aliases, composite types, and v1-path detection so analysis is more accurate (including ChildOf option extraction) and fixes can target precise source spans without misidentifying non‑v1 symbols.
This extends the v1 import rewrite to recognize contrib subpaths and translate them to the new contrib/.../v2 path while still handling core imports, ensuring fixes produce correct module paths for both patterns.
Adds the AppSecLoginEvents known change plus wiring in main and tests so v2fix can remap TrackUserLogin(Success|Failure)Event to the new TrackUserLogin(Success|Failure) APIs and keep the golden fixtures in sync; this keeps the AppSec migration path covered and avoids false positives against the renamed helpers.
Adds the DeprecatedWithPrioritySampling KnownChange plus new golden fixture/output to cover tracer.WithPrioritySampling so the checker can emit the “has been removed” warning (priority sampling is now default) without attempting any code changes, keeping migrations focused on the new semantics.
Adds the DeprecatedWithHTTPRoundTripper KnownChange, the associated golden fixture inputs/outputs, and registers it so v2fix can detect tracer.WithHTTPRoundTripper calls and emit the “has been removed; use WithHTTPClient instead” warning; this keeps the migration checker aware of the removed option instead of letting outdated code pass silently.
Added a falsepositive fixture and regression test that runs the WithServiceName, TraceID, WithDogstatsdAddress, and sampling rule rewrites against locally defined functions/types so that the checker proves it only flags dd-trace-go v1 symbols, preventing the migration tool from warning about unrelated code that happens to share names.
Adds the ChildOfStartChild known change along with its probe/test wiring so v2fix can rewrite tracer.StartSpan(..., tracer.ChildOf(...)) into the new parent.StartChild(...) pattern, keeping option plumbing and test coverage aligned with the migration guidance.
Added composite-type cases plus the N constant to the ddtracetypes stage fixtures and taught DDTraceTypes to preserve pointer/slice/array prefixes, skip fixes when array lengths can’t be rendered, and exclude SpanContext even when wrapped so the migration checker can diagnose the expanded scenarios without corrupting the source representation.
Added a RateRule exercise to the samplingrules fixture and refreshed its golden output to show the expected tracer.Rule{…} literal, while updating DeprecatedSamplingRules to require the v1 package path, keep argument-length guards, and qualify the emitted Rule literal with the package prefix so the fixer now safely rewrites every deprecated constructor (including RateRule) into the new struct form without touching non-dd-trace-go symbols.
… generation Use the stored type expression string to keep the original qualifier/alias in struct pointer fixes, fall back to derived types when missing, guard against non-selector calls, and reuse the package prefix helper while tightening argument checks so generated edits are safer and more accurate.
darccio
added
the
AI Assisted
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files🚀 New features to boost your workflow:
|
BenchmarksBenchmark execution time: 2026-02-12 18:58:20 Comparing candidate commit 3c92d46 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 155 metrics, 9 unstable metrics.
|
| @@ -43,6 +43,9 @@ type KnownChange interface { | |||
|
|
|||
There was a problem hiding this comment.
I think we should split the known_change.go into several files and have a package to collect them.
There was a problem hiding this comment.
Doing it in a separate PR.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7bd0cfc776
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Refactor the v2fix tool to replace instances of tracer.StartSpan with parent.StartChild when using ChildOf, ensuring compliance with the new API pattern. This change enhances the migration process by aligning with the updated tracing methods while maintaining option handling and test coverage.
…for improved sorting
This change standardizes the naming convention for type extraction functions in the v2fix tool, improving code clarity and consistency.
|
@codex review |
|
View all feedbacks in Devflow UI.
This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
The expected merge time in
Tests failed on this commit 019b653:
What to do next?
|
|
/merge |
|
View all feedbacks in Devflow UI.
This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
The expected merge time in
Tests failed on this commit 8da6860:
What to do next?
|
|
/merge |
|
View all feedbacks in Devflow UI.
This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
dario.castane@datadoghq.com unqueued this merge request |
gh-worker-dd-devflow-36fce6
bot
added
mergequeue-status: waiting
and removed
mergequeue-status: rejected
labels
|
/remove |
|
View all feedbacks in Devflow UI.
|
gh-worker-dd-devflow-36fce6
bot
added
mergequeue-status: removed
and removed
mergequeue-status: waiting
labels
3 participants
What does this PR do?
KnownChangebehaviorMotivation
We want to broaden automated migration support for
dd-trace-gov2 by capturing more v1 API changes and ensuring suggested fixes are precise, stable, and safe. The added rewrites, warnings, and import mappings reduce manual migration effort, while the improved probe/type handling and stronger tests prevent false positives and regressions.This has been implemented while testing the automatic campaigner for internal migration to deprecate v1.
Reviewer's Checklist
./scripts/lint.shlocally.Unsure? Have a question? Request a review!