Skip to content

feat: orchestrion diff#631

Merged
kakkoyun merged 19 commits into
mainfrom
eliottness/work-to-diff
Jun 5, 2025
Merged

feat: orchestrion diff#631
kakkoyun merged 19 commits into
mainfrom
eliottness/work-to-diff

Conversation

@eliottness

@eliottness eliottness commented May 27, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor 
NAME:
   orchestrion diff - Generates a diff between a nominal and orchestrion-instrumented build using a go work directory that can be obtained running `orchestrion go build -work -a`. This is incompatible with coverage related flags.

USAGE:
   orchestrion diff [command options] [arguments...]

OPTIONS:
   --files         Only show file paths created by orchestrion instead of diff output (default: false)
   --filter value  Filter the diff to a regex matched on the package or file paths from the build.
   --package       Print package names instead of printing the diff (default: false)
   --debug         Also print synthetic and tracer weaved packages (default: false)
   --help, -h      show help

Closes #571

@eliottness
eliottness requested a review from a team as a code owner May 27, 2025 07:55
datadog-datadog-prod-us1[bot]
datadog-datadog-prod-us1 Bot reviewed May 27, 2025
View reviewed changes
Comment thread internal/report/report.go Outdated
@github-actions github-actions Bot added the conventional-commit/feat Feature work label May 27, 2025
@eliottness
eliottness marked this pull request as draft May 27, 2025 08:36
eliottness and others added 14 commits June 4, 2025 17:10
@eliottness
feat(cmd): orchestrion diff
a4c99e6 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness
fix misconception about original file placement in fs
7350c11 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness
move code to the report package
6570c2c 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness
wip
0a89990 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness
test with sourcegraph lib
bad9bc6 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness
test with the diff binary
8dbb11a 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness
reduce the diff
0708c1f 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness
support --package
1f71aee 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness
better error reporting
befa827 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness
add special case for synthetic path
ad69a4f 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness
rework report to use fs.FS when possible
f230665 
Signed-off-by: Eliott Bouhana <[email protected]>
@RomainMuller @eliottness
chore: update generated files
5c931ba
@eliottness
write a small doc and filter weaving special cases
a2fb8db 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness
order output
d296497 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness
eliottness force-pushed the eliottness/work-to-diff branch from 56a2a56 to d296497 Compare June 4, 2025 15:13
eliottness added 2 commits June 4, 2025 17:18
@eliottness
fix license header issu
162f042 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness
fix doc phrasing
b655039 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness
eliottness marked this pull request as ready for review June 4, 2025 15:20
kakkoyun
kakkoyun approved these changes Jun 5, 2025
View reviewed changes

@kakkoyun kakkoyun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉

I see some windows test failures because of the escaping issues.

Comment thread internal/report/report.go
Comment thread internal/report/report.go
var errs []error

// Check if the diff command is available, exit early if not
if _, err := exec.LookPath("diff"); err != nil {

@kakkoyun kakkoyun Jun 5, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe in the subsequent PRs, we can consider git diff facilities. e.g. I use fancy git diff setup maybe it is possible to use whatever is set for git.

@eliottness eliottness Jun 5, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would require to make sure all the options I currently pass are supported. We can create an issue about this for future planning 👍

Comment thread internal/report/report.go Outdated
}

// When adding options, always make sure this is supported on all platforms.
cmd := exec.Command("diff",

@kakkoyun kakkoyun Jun 5, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume our experiments with Go diff packages didn't go well?

@eliottness eliottness Jun 5, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried basically all of them and even combining them but none of them have context window diffs which is a necessity when stdlib files makes multiple thousands lines long

@eliottness
eliottness added this pull request to the merge queue Jun 5, 2025
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 5, 2025
@kakkoyun
kakkoyun added this pull request to the merge queue Jun 5, 2025
Merged via the queue into main with commit b3088d9 Jun 5, 2025
57 checks passed
@kakkoyun
kakkoyun deleted the eliottness/work-to-diff branch June 5, 2025 18:57
@codecov

codecov Bot commented Jun 5, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 28.37838% with 159 lines in your changes missing coverage. Please review.

Project coverage is 67.45%. Comparing base (f5bd095) to head (93f1e6b).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
internal/report/report.go 23.41% 116 Missing and 5 partials ⚠️
internal/cmd/diff.go 0.00% 34 Missing ⚠️
internal/injector/parse/mappedfilename.go 81.81% 1 Missing and 1 partial ⚠️
internal/injector/parse/parser.go 50.00% 0 Missing and 1 partial ⚠️
internal/toolexec/aspect/oncompile.go 87.50% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #631      +/-   ##
==========================================
- Coverage   68.63%   67.45%   -1.19%     
==========================================
  Files         109      112       +3     
  Lines        7336     7715     +379     
==========================================
+ Hits         5035     5204     +169     
- Misses       1812     2001     +189     
- Partials      489      510      +21
Components Coverage Δ
Generators 80.24% <ø> (ø)
Instruments ∅ <ø> (∅)
Go Driver 76.51% <ø> (ø)
Toolexec Driver 70.83% <94.11%> (ø)
Aspects 76.91% <ø> (-0.72%) ⬇️
Injector 76.51% <76.92%> (-0.36%) ⬇️
Job Server 65.91% <ø> (ø)
Other 67.45% <28.37%> (-1.19%) ⬇️
Files with missing lines Coverage Δ
internal/toolexec/aspect/oncompile-main.go 65.55% <100.00%> (ø)
internal/toolexec/aspect/specialcase.go 100.00% <100.00%> (ø)
main.go 27.27% <ø> (ø)
internal/injector/parse/parser.go 70.37% <50.00%> (ø)
internal/toolexec/aspect/oncompile.go 71.11% <87.50%> (-1.62%) ⬇️
internal/injector/parse/mappedfilename.go 80.30% <81.81%> (+1.96%) ⬆️
internal/cmd/diff.go 0.00% <0.00%> (ø)
internal/report/report.go 23.41% <23.41%> (ø)

... and 3 files with indirect coverage changes

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@datadog-datadog-prod-us1 datadog-datadog-prod-us1[bot] datadog-datadog-prod-us1[bot] left review comments
@RomainMuller RomainMuller RomainMuller approved these changes
@kakkoyun kakkoyun kakkoyun approved these changes

Assignees

No one assigned

Labels

conventional-commit/feat Feature work

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Print instrumented code

3 participants

@eliottness @RomainMuller @kakkoyun