Skip to content

Delete advice transformation code#17471

Merged
laurit merged 5 commits intoopen-telemetry:mainfrom
laurit:advice-transform
Apr 7, 2026
Merged

Delete advice transformation code#17471
laurit merged 5 commits intoopen-telemetry:mainfrom
laurit:advice-transform

Conversation

@laurit
Copy link
Copy Markdown
Contributor

@laurit laurit commented Apr 3, 2026

None of our advices seem to require transformation any more.

@laurit laurit changed the title Disable transforming advice Delete advice transformation code Apr 3, 2026
@laurit laurit marked this pull request as ready for review April 3, 2026 17:49
@laurit laurit requested a review from a team as a code owner April 3, 2026 17:49
Copy link
Copy Markdown
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

🎉🎉🎉

Image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This instrumentation aims to represent an existing inlined instrumentation extension.

The difference is that now this is no longer modified at runtime when indy is enabled but always reused as-is (so always inlined).

If we want to avoid regressions for existing inlined instrumentation in extensions we should not modify it and keep the readOnly = false in the arguments.

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.

It still uses inlined advice but not by using @AssignReturned it can also be used as non-inlined without advice transformation (currently the only transform indy instrumentation uses is changing the value of inlined attribute). So I'd argue that it still tests inlined advice.

@laurit laurit merged commit 2641b97 into open-telemetry:main Apr 7, 2026
93 checks passed
@laurit laurit deleted the advice-transform branch April 7, 2026 16:21
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.

3 participants