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

Adding net/http client instrumentor #50

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

oxeye-gal
Copy link
Contributor

Hi,

This PR attempts to add instrumentation for net/http client.

We do it by writing a fake tophash value (as calculating the correct one will be harder to achieve) to the first headers map bucket and writing the "traceparent" header key and value into memory.

This was tested with Go versions 1.12-1.19.

Our test environment consisted of instrumented Python application that sends a request to instrumented Go http server which in turn sends an http request to an additional instrumented Go http server.

The produced trace looks like the following:

image

We would love to get your feedback on this.
Thanks!

@edeNFed
Copy link
Contributor

edeNFed commented Feb 15, 2023

So sorry for the huge delay! I promise to finish looking on this by the end of this week

@vreynolds
Copy link

@oxeye-gal any interest in re-opening this in https://github.com/open-telemetry/opentelemetry-go-instrumentation ?

@oxeye-gal
Copy link
Contributor Author

Hi @vreynolds, sure thing i'll do it later this week.

@MikeGoldsmith
Copy link

Hey @oxeye-gal - If you would like help porting this to the OTel repo, let me know -- I'm happy to help however I can.

@vreynolds
Copy link

fwiw, I was able to see this work writing traceparent headers in a test app, with a small change to how link.UprobeOptions are initialized. Looks like in the OTel repo the offsets should be passed as Address option rather than Offset

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.

4 participants