feat(injector): Add return-implements function join point by kakkoyun · Pull Request #602 · DataDog/orchestrion

kakkoyun · 2025-04-09T11:12:32Z

feat(injector/join): add 'return-implements' function join point

Adds the return-implements: "" selector to the function join point. This allows matching functions where at least one return value implements the specified interface, respecting Go's structural typing.
Includes a test case (testdata/injector/return-implements) covering standard library and custom types.

--

Fixes #538

ref: #537
xref: DataDog/dd-trace-go#3168
xref: LANGPLAT-159

kakkoyun · 2025-04-09T11:16:42Z

refactor(injector): Refactor TypeName handling and migrate to typed package #615 : 2 dependent PRs (#616 , #633 )
feat(injector): Add return-implements function join point #602 👈 (View in Graphite)
feat(injector): Add ability to find function results implementing an interface #587
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

kakkoyun · 2025-04-09T11:22:25Z

PTAL @korECM

eliottness

Do you mind adding tests for generic interfaces and generic interface implementations ?

kakkoyun · 2025-04-24T14:21:28Z

Do you mind adding tests for generic interfaces and generic interface implementations ?

What do you exactly mean by generic interfaces? Could you give me an example?

eliottness · 2025-04-28T07:36:38Z

@kakkoyun I mean making sure the new join-point interact correctly with concrete types having a generic component and interfaces declared in the join-point being able to be generic.

Concretely simply adding tests in internal/injector/testdata/injector/result-implements-join/config.yml would be enough for me

kakkoyun · 2025-05-26T14:15:19Z

@kakkoyun I mean making sure the new join-point interact correctly with concrete types having a generic component and interfaces declared in the join-point being able to be generic.

Concretely simply adding tests in internal/injector/testdata/injector/result-implements-join/config.yml would be enough for me

It was a good call! Handled as part of 9721aaa

RomainMuller

I think there are some issues with the current implementation of the "implies imported" heuristics.

Adds a new FunctionOption, 'return-implements', to the function join point selector. This allows matching functions where at least one return value's type implements a specified interface. The implementation leverages type information passed via the AspectContext during join point matching. It utilizes helpers from the 'typed' package for resolving interface names and performing type implementation checks (`types.Implements`). The `packageMayMatch` and `fileMayMatch` hints for this option return `may.Unknown` to correctly handle Go's structural typing, ensuring that types implementing an interface without explicitly importing its package are still considered. A comprehensive test case (`testdata/injector/return-implements`) is included, covering various scenarios like explicit interface returns, concrete type implementations (including custom types), multiple return values, and non-matching cases. Signed-off-by: Kemal Akkoyun <[email protected]>

Signed-off-by: Kemal Akkoyun <[email protected]>

…n value interface matching This update adds the `finalResultImplements` FunctionOption, enabling the matching of functions where the final return value implements a specified interface. The implementation includes methods for determining package and file match types, as well as evaluating the final return value against the target interface. Additionally, the configuration YAML has been updated with test cases demonstrating the new functionality, including scenarios that illustrate the distinction between `result-implements` and `final-result-implements`. Signed-off-by: Kemal Akkoyun <[email protected]>

…in points This commit extends the result-implements and final-result-implements join points to properly handle generic types. Previously, functions returning generic types that implement interfaces (e.g., GenericBuffer[string] implementing io.Reader) were not being instrumented correctly. The fix ensures that the type checker properly resolves generic type instantiations, allowing the join points to correctly identify when generic types implement the specified interfaces. Signed-off-by: Kemal Akkoyun <[email protected]>

Signed-off-by: Kemal Akkoyun <[email protected]>

codecov · 2025-06-05T15:08:22Z

Codecov Report

Attention: Patch coverage is 56.72515% with 74 lines in your changes missing coverage. Please review.

Project coverage is 66.32%. Comparing base (983120d) to head (cda266d).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/injector/aspect/join/function.go	57.30%	30 Missing and 8 partials ⚠️
internal/injector/typed/typecheck.go	58.82%	19 Missing and 9 partials ⚠️
internal/injector/aspect/context/context.go	0.00%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #602      +/-   ##
==========================================
- Coverage   70.14%   66.32%   -3.83%     
==========================================
  Files         109      110       +1     
  Lines        7517     7682     +165     
==========================================
- Hits         5273     5095     -178     
- Misses       1718     2056     +338     
- Partials      526      531       +5

Components	Coverage Δ
Generators	`80.24% <ø> (ø)`
Instruments	`∅ <ø> (∅)`
Go Driver	`75.81% <ø> (-0.70%)`	⬇️
Toolexec Driver	`67.37% <ø> (-3.46%)`	⬇️
Aspects	`71.76% <54.90%> (-5.16%)`	⬇️
Injector	`72.49% <56.72%> (-3.98%)`	⬇️
Job Server	`65.91% <ø> (-3.32%)`	⬇️
Other	`66.32% <56.72%> (-3.83%)`	⬇️

Files with missing lines	Coverage Δ
internal/injector/aspect/advice/code/dot.go	`79.06% <ø> (ø)`
...ternal/injector/aspect/advice/code/dot_function.go	`56.94% <100.00%> (-6.49%)`	⬇️
internal/injector/check.go	`71.79% <100.00%> (+0.74%)`	⬆️
internal/injector/aspect/context/context.go	`72.79% <0.00%> (-21.74%)`	⬇️
internal/injector/typed/typecheck.go	`58.82% <58.82%> (ø)`
internal/injector/aspect/join/function.go	`66.66% <57.30%> (-6.87%)`	⬇️

... and 25 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

korECM · 2025-07-18T04:03:32Z

@kakkoyun
Hi! I've confirmed that this has been deployed in v1.5.0.

However, after looking at the orchestrion.yml file, it seems that ResultOfType is still being used. This makes me think that while the join point support has been added, it's not actually capturing errors yet.

Could you please confirm if my understanding is correct?

kakkoyun · 2025-07-18T05:40:34Z

Hey 👋 Yes, we haven't utilizing the new improvements yet.

Feel free to test it out and contribute the changes in your mind.

Otherwise, we are working on a related issue and we will handle it as part of that.

cc @hannahkm

kakkoyun requested a review from a team as a code owner April 9, 2025 11:12

github-actions Bot added the conventional-commit/feat Feature work label Apr 9, 2025

kakkoyun changed the base branch from main to kakkoyun/check-error-types April 9, 2025 11:15

kakkoyun mentioned this pull request Apr 9, 2025

feat(injector): Add ability to find function results implementing an interface #587

Merged

kakkoyun force-pushed the kakkoyun/check-return-error-types-join-point branch from 56aec54 to ee796e1 Compare April 9, 2025 11:18

kakkoyun requested review from RomainMuller, darccio and eliottness April 9, 2025 11:21

kakkoyun force-pushed the kakkoyun/check-error-types branch from 17d27eb to df69d0c Compare April 11, 2025 11:54

kakkoyun force-pushed the kakkoyun/check-return-error-types-join-point branch 2 times, most recently from e37581f to f054729 Compare April 14, 2025 16:34

eliottness requested changes Apr 17, 2025

View reviewed changes

eliottness removed the request for review from RomainMuller April 30, 2025 07:30

kakkoyun force-pushed the kakkoyun/check-error-types branch from 01ea546 to fd5e99c Compare May 26, 2025 10:47

kakkoyun force-pushed the kakkoyun/check-return-error-types-join-point branch from 5af6ebb to afe939a Compare May 26, 2025 10:52

kakkoyun force-pushed the kakkoyun/check-error-types branch from fd5e99c to ff5b86b Compare May 26, 2025 14:11

kakkoyun force-pushed the kakkoyun/check-return-error-types-join-point branch from cd7519c to 9721aaa Compare May 26, 2025 14:14

kakkoyun requested a review from eliottness May 26, 2025 14:15

kakkoyun force-pushed the kakkoyun/check-return-error-types-join-point branch from 9721aaa to f7476cf Compare May 26, 2025 14:19

kakkoyun mentioned this pull request May 27, 2025

feat(injector/typed): add interface-based type system with parser #633

Closed

eliottness approved these changes May 27, 2025

View reviewed changes

kakkoyun mentioned this pull request May 27, 2025

feat(injector/typed): Enhance type checking with additional generic type test cases #634

Merged

RomainMuller requested changes Jun 3, 2025

View reviewed changes

kakkoyun force-pushed the kakkoyun/check-error-types branch from ff5b86b to 81f002c Compare June 5, 2025 10:21

kakkoyun force-pushed the kakkoyun/check-return-error-types-join-point branch from f7476cf to bd35383 Compare June 5, 2025 10:42

kakkoyun requested a review from RomainMuller June 5, 2025 10:44

kakkoyun force-pushed the kakkoyun/check-return-error-types-join-point branch from bd35383 to cbef457 Compare June 5, 2025 11:00

kakkoyun changed the base branch from kakkoyun/check-error-types to graphite-base/602 June 5, 2025 12:59

RomainMuller approved these changes Jun 5, 2025

View reviewed changes

kakkoyun added 5 commits June 5, 2025 16:21

chore: Address review comments

39905e4

Signed-off-by: Kemal Akkoyun <[email protected]>

chore: Address review comments

cda266d

Signed-off-by: Kemal Akkoyun <[email protected]>

kakkoyun force-pushed the kakkoyun/check-return-error-types-join-point branch from cbef457 to cda266d Compare June 5, 2025 14:25

kakkoyun force-pushed the graphite-base/602 branch from 81f002c to 983120d Compare June 5, 2025 14:25

kakkoyun changed the base branch from graphite-base/602 to main June 5, 2025 14:25

kakkoyun enabled auto-merge June 5, 2025 14:26

kakkoyun added this pull request to the merge queue Jun 5, 2025

Merged via the queue into main with commit 121b15f Jun 5, 2025
54 of 67 checks passed

kakkoyun deleted the kakkoyun/check-return-error-types-join-point branch June 5, 2025 15:08

Uh oh!

Conversation

kakkoyun commented Apr 9, 2025 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kakkoyun commented Apr 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kakkoyun commented Apr 9, 2025

Uh oh!

eliottness left a comment

Choose a reason for hiding this comment

Uh oh!

kakkoyun commented Apr 24, 2025

Uh oh!

eliottness commented Apr 28, 2025

Uh oh!

kakkoyun commented May 26, 2025

Uh oh!

RomainMuller left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jun 5, 2025

Codecov Report

Uh oh!

korECM commented Jul 18, 2025

Uh oh!

kakkoyun commented Jul 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kakkoyun commented Apr 9, 2025 •

edited by atlassian Bot

Loading

kakkoyun commented Apr 9, 2025 •

edited

Loading