Skip to content

fix(injector): resolve .AST.Fun.Name crash on SelectorExpr in wrap-expression templates#819

Merged
eliottness merged 2 commits into
mainfrom
eliottness/fix-ast-fun-name-selector-expr
Apr 13, 2026
Merged

fix(injector): resolve .AST.Fun.Name crash on SelectorExpr in wrap-expression templates#819
eliottness merged 2 commits into
mainfrom
eliottness/fix-ast-fun-name-selector-expr

Conversation

@eliottness
Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a build crash (can't evaluate field Name in type dst.Expr) that occurs when a function-call join-point matches a qualified call like http.Get(...) and the advice template accesses {{ .AST.Fun.Name }}.

Root cause: proxyCallExpr.Fun() returns dst.Expr (interface). When the underlying node is a *dst.SelectorExpr (e.g., for http.Get), proxySelectorExpr had no Name field or method. Go's text/template could not resolve .Name, crashing with the error above. The same template works fine for bare/dot-imported calls (Get(...)) where Fun is a *dst.Ident with a promoted Name field.

Fix: Add a Name() string method to proxySelectorExpr in a hand-written companion file (dot_ast_extras.go) that returns Sel.Name. This makes {{ .AST.Fun.Name }} work transparently for both node types without any changes to dd-trace-go templates — fully backwards compatible.

Motivation

Customer escalation APMS-19232: any user calling http.Get(...), http.Head(...), http.Post(...), or http.PostForm(...) in a function with a context.Context parameter fails to build with orchestrion. The dd-trace-go net/http client aspect (Get|Head|Post|PostForm) triggers the crash.

Describe how you validated your changes

  • Added end-to-end injector regression test selector-expr-fun-name/ that exercises exactly the failure path: a function-call join-point on net/http.Get with input code that has a context.Context parameter, forcing the template branch that evaluates {{ .AST.Fun.Name }} on a *dst.SelectorExpr
  • All injector tests pass: go test ./internal/injector/... (14 packages, 51s)
  • golangci-lint clean

Additional Notes

The fix lives in dot_ast_extras.go, a hand-written companion to the auto-generated dot_ast.proxies.go. This separation keeps generated code untouched while allowing targeted extensions. A comment in the new file guides future maintainers to add similar extensions there.

The alternative (adding a new FuncName() template helper) was rejected because it would require dd-trace-go to depend on a minimum orchestrion version, breaking backwards compatibility.

…pression templates

When a function-call join-point matches a qualified call like http.Get(...),
the Fun field of the *dst.CallExpr is a *dst.SelectorExpr. Templates accessing
{{ .AST.Fun.Name }} crashed with "can't evaluate field Name in type dst.Expr"
because proxySelectorExpr had no Name field or method.

Add a Name() string method to proxySelectorExpr returning Sel.Name. This makes
{{ .AST.Fun.Name }} work transparently for both *dst.Ident (promoted field from
embedded *dst.Ident) and *dst.SelectorExpr (new method returning Sel.Name).

No dd-trace-go changes required — the existing template just starts working.

Constraint: Must not introduce new template API surface (no FuncName() helper)
that would force dd-trace-go to depend on a minimum orchestrion version
Rejected: Add FuncName() helper | breaks backwards compatibility between
dd-trace-go and orchestrion versions
Confidence: high
Scope-risk: narrow
@datadog-datadog-prod-us1-2
Copy link
Copy Markdown

datadog-datadog-prod-us1-2 Bot commented Apr 10, 2026

🎯 Code Coverage (details)
Patch Coverage: 0.00%
Overall Coverage: 67.89%

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: fa2c920 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!

@eliottness eliottness marked this pull request as ready for review April 10, 2026 12:37
@eliottness eliottness requested a review from a team as a code owner April 10, 2026 12:37
Comment thread internal/injector/testdata/injector/selector-expr-fun-name/config.yml Outdated
Co-authored-by: Eliott Bouhana <47679741+eliottness@users.noreply.github.com>
@eliottness
Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

ℹ️ 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".

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 10, 2026

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.66%. Comparing base (e061d12) to head (fa2c920).
⚠️ Report is 77 commits behind head on main.

Files with missing lines Patch % Lines
...rnal/injector/aspect/advice/code/dot_ast_extras.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #819      +/-   ##
==========================================
+ Coverage   65.72%   69.66%   +3.94%     
==========================================
  Files         113      117       +4     
  Lines        7926     6902    -1024     
==========================================
- Hits         5209     4808     -401     
+ Misses       2192     1545     -647     
- Partials      525      549      +24     
Components Coverage Δ
Generators 83.23% <ø> (+2.98%) ⬆️
Instruments ∅ <ø> (∅)
Go Driver 75.58% <65.38%> (-0.23%) ⬇️
Toolexec Driver 74.78% <100.00%> (+7.25%) ⬆️
Aspects 76.89% <75.00%> (+4.98%) ⬆️
Injector 77.19% <76.41%> (+4.39%) ⬆️
Job Server 68.38% <55.55%> (+2.46%) ⬆️
Other 69.66% <65.11%> (+3.94%) ⬆️
Files with missing lines Coverage Δ
...rnal/injector/aspect/advice/code/dot_ast_extras.go 0.00% <0.00%> (ø)

... and 108 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@eliottness eliottness enabled auto-merge April 13, 2026 12:12
@eliottness eliottness added this pull request to the merge queue Apr 13, 2026
Merged via the queue into main with commit df7c492 Apr 13, 2026
77 checks passed
@eliottness eliottness deleted the eliottness/fix-ast-fun-name-selector-expr branch April 13, 2026 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants