chore(test): add grcp integration test #135

pdelewski · 2025-11-06T21:57:38Z

No description provided.

kakkoyun

Looking good 🚀

Copilot

Pull Request Overview

This PR adds gRPC server instrumentation support to the OpenTelemetry compile-time instrumentation project. It enables hooking into gRPC server initialization to add telemetry capabilities.

Key changes:

Adds gRPC server hook configuration and implementation
Implements a Shutdown RPC method for graceful server termination in tests
Adds end-to-end test coverage for the gRPC instrumentation hook

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tool/data/grpc.yaml	Defines hook configuration for gRPC server's Serve method
test/e2e/grpc_test.go	Adds e2e test to verify gRPC server hook invocation
pkg/instrumentation/grpc/server_hook.go	Implements the BeforeServe hook for gRPC server instrumentation
pkg/instrumentation/grpc/go.mod	Adds Go module definition for the gRPC instrumentation package
demo/grpc/server/pb/greeter_grpc.pb.go	Generated gRPC code including new Shutdown RPC method
demo/grpc/server/pb/greeter.pb.go	Generated protobuf code for Shutdown request/reply messages
demo/grpc/server/main.go	Implements Shutdown RPC handler and adds startup logging
demo/grpc/server/greeter.proto	Adds Shutdown RPC definition to the service
demo/grpc/server/go.sum	Updates dependency checksums
demo/grpc/client/main.go	Adds shutdown flag to invoke server shutdown
demo/grpc/README.md	Documents the new Shutdown RPC method and usage

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-07T11:49:48Z

test/e2e/grpc_test.go

+			if strings.Contains(line, readyMsg) {
+				close(readyChan)
+			}


The readyChan channel could be closed multiple times if the "server started" message appears more than once in the server output, which would cause a panic. Consider using a sync.Once or checking if the channel is already closed before attempting to close it again.

Example fix:

var readyOnce sync.Once // ... in the goroutine: if strings.Contains(line, readyMsg) { readyOnce.Do(func() { close(readyChan) }) }

pdelewski · 2025-11-09T18:04:38Z

Right now, the example rule isn’t working, and BeforeServe isn’t being called. I’ll investigate the root cause.

❌ TestGrpc (13.81s)
      infra.go:44: 
          	Error Trace:	/home/runner/work/opentelemetry-go-compile-instrumentation/opentelemetry-go-compile-instrumentation/test/app/infra.go:44
          	            				/home/runner/work/opentelemetry-go-compile-instrumentation/opentelemetry-go-compile-instrumentation/test/e2e/grpc_test.go:63
          	Error:      	Received unexpected error:
          	            	exit status 1
          	Test:       	TestGrpc

pdelewski · 2025-11-10T20:43:59Z

According to debug.log rule is matched:

msg="Match func rule" phase=go rule=server_hook2 dep="{google.golang.org/[email protected]: [/Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/backoff.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/balancer_wrapper.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/call.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/clientconn.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/codec.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/dialoptions.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/doc.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/interceptor.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/picker_wrapper.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/pickfirst.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/preloader.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/resolver_wrapper.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/rpc_util.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/server.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/service_config.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/shared_buffer_pool.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/stream.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/stream_interfaces.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/trace.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/trace_withtrace.go /Users/pdelewski/go/pkg/mod/google.golang.org/[email protected]/version.go]}"

But it's not applied, e.g code is not injected. Below place where we return with an error

pdelewski · 2025-11-10T22:35:22Z

Going deeper, it seems that there is signature mismatch

Hi @y1yang0 — there’s a gap in my understanding here. I’m trying to hook into grpc.(*Server).Serve

My current hook is:

//go:linkname BeforeServe google.golang.org/grpc.BeforeServe
func BeforeServe(ictx inst.HookContext, _ interface{}, lis net.Listener) {
    fmt.Println("BeforeServe")
    // TODO: Implement the real server hook logic here
}

According to my understanding the first two parameters in this case should be ictx inst.HookContext, _ interface{}, however it does not work. Just wanted to confirm if that's correct.

y1yang0 · 2025-11-11T03:18:47Z

@pdelewski I see. The issue was that ast.FindFuncDecl was identifying the wrong target function, which led to a signature mismatch. I will submit a PR to fix this. Apologies for the mistake.

y1yang0 · 2025-11-11T03:49:59Z

test/e2e/grpc_test.go

+	"github.com/stretchr/testify/require"
+)
+
+func waitUntilGrpcReady(t *testing.T, serverApp *exec.Cmd, outputPipe io.ReadCloser) func() string {


This looks almost identical to waitUntilReady in http_test.go. Perhaps we could extract this into a shared app.go, like app.WaitUntilReady (or app.WaitReady)?

pdelewski · 2025-11-11T09:22:40Z

@kakkoyun Why it was closed? #147 contains a fix that is needed to grpc work correctly

pdelewski · 2025-11-11T09:28:43Z

It seems that still, there is some problem

pdelewski · 2025-11-11T09:35:37Z

It's better now, as it seems that instrumentation is there:

func (s *Server) Serve(lis net.Listener) (_unnamedRetVal0 error) {
        //line <generated>:1  
        if hookContext802697909, skip802697909 := OtelBeforeTrampoline_Serve802697909(&s, &lis); skip802697909 {
                OtelAfterTrampoline_Serve802697909(hookContext802697909, &_unnamedRetVal0)
                return _unnamedRetVal0
        } else

However tests is failing. I need to investigate further.

pdelewski · 2025-11-11T09:38:51Z

@pdelewski I see. The issue was that ast.FindFuncDecl was identifying the wrong target function, which led to a signature mismatch. I will submit a PR to fix this. Apologies for the mistake.

@y1yang0 Thanks for the fix. I was not sure if that was due to wrong rule or bug in the instrumentation itself.

kakkoyun · 2025-11-11T12:35:01Z

@kakkoyun Why it was closed? #147 contains a fix that is needed to grpc work correctly

I think it happened because I merged the PR (#147)? I didn't close it deliberately.

It says "fixes" in the description. And apparently GH doesn't care if it is an issue or PR, it closes it.

pdelewski · 2025-11-11T13:00:52Z

@kakkoyun Why it was closed? #147 contains a fix that is needed to grpc work correctly

I think it happened because I merged the PR (#147)? I didn't close it deliberately.

It says "fixes" in the description. And apparently GH doesn't care if it is an issue or PR, it closes it.

Ok, thank you

kakkoyun · 2025-11-12T08:32:59Z

@pdelewski Do you need any help with this?

pdelewski · 2025-11-12T08:56:08Z

@pdelewski Do you need any help with this?

@kakkoyun I haven’t had much time on this yet. I’ll take a closer look today. If I don’t find the root cause, I’d appreciate a hand.

chore(test): add grcp integration test

e6aa94d

github-actions bot added the conventional-commit/chore Something that needs to be taken care of but not very appealing label Nov 6, 2025

kakkoyun requested a review from Copilot November 7, 2025 11:46

kakkoyun approved these changes Nov 7, 2025

View reviewed changes

Copilot AI reviewed Nov 7, 2025

View reviewed changes

y1yang0 mentioned this pull request Nov 10, 2025

beta version: Implementation tasks #29

Open

36 tasks

y1yang0 mentioned this pull request Nov 11, 2025

fix: ast.FindFuncDecl finds wrong target func #147

Merged

y1yang0 reviewed Nov 11, 2025

View reviewed changes

kakkoyun closed this in #147 Nov 11, 2025

pdelewski reopened this Nov 11, 2025

Merge branch 'main' into grpc-e2e-test

95d6b35

Merge branch 'main' into grpc-e2e-test

c1f2717

fix: removing go:linkname

016892e

pdelewski marked this pull request as ready for review November 12, 2025 10:04

pdelewski requested a review from a team as a code owner November 12, 2025 10:04

pdelewski merged commit d67b38c into open-telemetry:main Nov 12, 2025
26 checks passed

chore(test): add grcp integration test #135

chore(test): add grcp integration test #135

Uh oh!

Conversation

pdelewski commented Nov 6, 2025

Uh oh!

kakkoyun left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

pdelewski commented Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pdelewski commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pdelewski commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

y1yang0 commented Nov 11, 2025

Uh oh!

y1yang0 Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

pdelewski commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pdelewski commented Nov 11, 2025

Uh oh!

pdelewski commented Nov 11, 2025

Uh oh!

pdelewski commented Nov 11, 2025

Uh oh!

kakkoyun commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pdelewski commented Nov 11, 2025

Uh oh!

kakkoyun commented Nov 12, 2025

Uh oh!

pdelewski commented Nov 12, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pdelewski commented Nov 9, 2025 •

edited

Loading

pdelewski commented Nov 10, 2025 •

edited

Loading

pdelewski commented Nov 10, 2025 •

edited

Loading

pdelewski commented Nov 11, 2025 •

edited

Loading

kakkoyun commented Nov 11, 2025 •

edited

Loading