Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add instrgen implementation #3108

Merged
merged 15 commits into from Mar 24, 2023

Conversation

pdelewski
Copy link
Member

No description provided.

@pdelewski pdelewski requested review from a team and MrAlias as code owners December 19, 2022 14:24
@pdelewski pdelewski force-pushed the adding-instrgen-implementation branch 5 times, most recently from ea147d9 to 2f7f181 Compare December 22, 2022 11:34
@pdelewski pdelewski force-pushed the adding-instrgen-implementation branch 14 times, most recently from 0039758 to 24451d9 Compare December 29, 2022 13:55
@pdelewski pdelewski force-pushed the adding-instrgen-implementation branch from da9aa79 to ee021aa Compare February 6, 2023 09:23
@pdelewski pdelewski force-pushed the adding-instrgen-implementation branch from 5686e6a to ad2bef0 Compare February 6, 2023 09:55
@pdelewski pdelewski force-pushed the adding-instrgen-implementation branch from ad2bef0 to 40b94be Compare February 6, 2023 10:02
@MrAlias
Copy link
Contributor

MrAlias commented Feb 21, 2023

SIG meeting: plan is to add a readme to this.

@pdelewski pdelewski force-pushed the adding-instrgen-implementation branch from 76c9f73 to 4bd3b5d Compare February 27, 2023 14:26
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

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

This looks good enough to start with. I've found a fair amount of issues, but I don't think any of them fundamentally block this from being merged as a starting point.

Thanks for getting this all together @pdelewski! 🎉

@pdelewski pdelewski force-pushed the adding-instrgen-implementation branch from be359f2 to c40b6de Compare March 23, 2023 13:19
Copy link
Member

@dineshg13 dineshg13 left a comment

Choose a reason for hiding this comment

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

This is a good start. I was able to instrument a sample HTTP server using this. @pdelewski Thanks for the contribution.

@pdelewski
Copy link
Member Author

@MrAlias Seems that we can merge it.

@MrAlias MrAlias added the area: instrgen Related to the instrgen package label Mar 24, 2023
@MrAlias
Copy link
Contributor

MrAlias commented Mar 24, 2023

Merging as this has 2 reviews from the @open-telemetry/go-instrumentation-approvers group.

@MrAlias MrAlias merged commit e65f090 into open-telemetry:main Mar 24, 2023
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: instrgen Related to the instrgen package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants