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

Consolidate HTTP server instrumentations to use single bpf probe implementation #249

Closed

Conversation

MikeGoldsmith
Copy link
Member

@MikeGoldsmith MikeGoldsmith commented Aug 7, 2023

Currently we have separate bpf probes for each http server instrumentation (net/http, gorilla mux & gin-gonic). The bpf code is identical across each implementation with only the go usage attaching the probe to different user space functions.

This PR does the following:

  • Create new root bpf directory and move all bpf headers into it
  • Move the net/http server bpf probe code to new bpf directory
  • Update generate Makefile target to also set the BPF_PROBES directory which instrumentations can referece
  • Update gorilla mux and gin-gonic instrumentations to use generic http service bpf probe, including:
    • Update event struct to add ParentSpanContext field
    • Add missing offset fields
    • Setting the parent span context when converting events
    • Updating expected test json structure to validate a parent span context is being set correctly
  • Remove gorilla mux and gin-gonic custom bpf probes

@MikeGoldsmith MikeGoldsmith requested a review from a team August 7, 2023 13:17
@MikeGoldsmith MikeGoldsmith changed the title Consolidate all http server instrumentations to use single bpf probe implementation Consolidate all HTTP server instrumentations to use single bpf probe implementation Aug 7, 2023
@MikeGoldsmith MikeGoldsmith changed the title Consolidate all HTTP server instrumentations to use single bpf probe implementation Consolidate HTTP server instrumentations to use single bpf probe implementation Aug 7, 2023
@MikeGoldsmith
Copy link
Member Author

Gin e2e test is still flakey - it works locally. Could someone else try and run to see if they something different?

@MrAlias
Copy link
Contributor

MrAlias commented Sep 13, 2023

Please rebase these changes into internal/pkg (changed in #282). The pkg directory is going to be removed in #302 given that package is no longer used.

@MikeGoldsmith
Copy link
Member Author

Closing as there's been lot of changes in the bpf code and project structurally which has meant this change isn't as needed as before.

@MikeGoldsmith MikeGoldsmith deleted the mike/http-reorg branch November 20, 2023 12:26
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.

2 participants