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

Fix gRPC instrumentation on recent kernels #150

Merged
merged 7 commits into from
May 9, 2023

Conversation

edeNFed
Copy link
Contributor

@edeNFed edeNFed commented May 8, 2023

In recent kernel versions, the eBPF verifier checks that a valid number of bytes (positive and not overflow) are being written/read from memory.
This PR adds additional bound checks so gRPC instrumentation now passes verification.

Tested on Ubuntu 20.02, x86, running in DigitalOcean.

@edeNFed edeNFed requested a review from a team May 8, 2023 16:47
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
include/alloc.h Outdated Show resolved Hide resolved
include/alloc.h Outdated Show resolved Hide resolved
@vreynolds
Copy link

I tested this with the example in #78 and it no longer errors!

This is the gRPC client span that's generated in the collector logs. It seems to be missing trace id and span id, and has a start time a few days in the past 😕 It could be an issue with the example app I'm using, but either way I think this fix addresses the error with loading the probe and this could be a separate issue.

ScopeSpans #0
ScopeSpans SchemaURL: 
InstrumentationScope google.golang.org/grpc 
Span #0
    Trace ID       : 
    Parent ID      : 
    ID             : 
    Name           : 
    Kind           : Client
    Start time     : 2023-05-04 19:01:53.589758682 +0000 UTC
    End time       : 2023-05-08 18:12:17.335586279 +0000 UTC
    Status code    : Unset
    Status message : 
Attributes:
     -> rpc.system: Str(grpc)
     -> rpc.service: Str()
     -> net.peer.ip: Str()
     -> net.peer.name: Str()

edeNFed and others added 2 commits May 8, 2023 22:27
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@edeNFed
Copy link
Contributor Author

edeNFed commented May 9, 2023

@vreynolds thanks for trying it out! I tested it with the emojivoto application and sent the traces to Jaeger which looked good (correct spans + context propagation

@edeNFed edeNFed merged commit 8e191d9 into open-telemetry:main May 9, 2023
MikeGoldsmith added a commit to honeycombio/opentelemetry-go-instrumentation that referenced this pull request May 9, 2023
can successfully run `make fixture-grpc`

some of the probe redaction can likely be removed once open-telemetry#150 is merged into this branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants