Fix grpc serverstream compat by dingsongjie · Pull Request #243 · apache/skywalking-go

dingsongjie · 2026-04-10T09:01:37Z

No description provided.

…mentation gRPC v1.69+ replaced transport.Stream with transport.ServerStream as the second argument of Server.handleStream, causing a runtime panic due to failed type assertion. Add dual witness points using WithArgType to match the correct stream type at compile time: transport.Stream for older versions and transport.ServerStream for v1.69+. Also fix trailing space bug in the original interceptor name. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

… set Previously, validateMethodInsMatch returned immediately after matching the receiver name, skipping all MethodFilters (WithArgType, WithResultType, etc). This caused WithArgType to be dead code for any method enhancement with a receiver, preventing correct method signature discrimination across framework versions. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

generateTypeNameByExp now serializes function types (e.g., func(context.Context, error)) and channel types (e.g., <-chan Delivery, chan<- int) instead of returning empty string. This enables WithArgType/WithResultType filters to correctly match these types. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

Adds v1.69.0, v1.72.0, v1.78.0 to the gRPC plugin integration test matrix to validate ServerStream compatibility. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

Copilot

Pull request overview

This PR improves gRPC plugin compatibility across gRPC versions by making method matching more precise (receiver + signature filters) and adding a dedicated interceptor/native struct for the newer transport.ServerStream handleStream signature.

Changes:

Fix Go agent method-matcher logic so receiver checks don’t short-circuit method filters (enables arg/result type filters to take effect).
Add gRPC handleStream instrumentation for both *transport.Stream (v1) and *transport.ServerStream (v2), including a new interceptor and native structure.
Extend type-name generation in core instrumentation (func/chan types) and add unit tests; expand gRPC scenario supported versions list.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tools/go-agent/instrument/plugins/instrument.go	Fix matcher evaluation so receiver check doesn’t skip method filters.
tools/go-agent/instrument/plugins/instrument_test.go	Add tests covering receiver/arg/result type filter combinations.
plugins/grpc/instrument.go	Split `handleStream` instrumentation into v1/v2 arg-type signatures and fix interceptor name string.
plugins/grpc/structures.go	Add native type mapping for `internal/transport.ServerStream`.
plugins/grpc/server_handleStream_v2_interceptor.go	New interceptor for the v2 `handleStream` signature.
plugins/grpc/server_handleStream_interceptor_test.go	Add tests for both v1 and v2 handleStream interceptors.
plugins/core/instrument/enhance.go	Add func/chan type stringification used by arg/result type matching.
plugins/core/instrument/enhance_test.go	Add unit tests for new type stringification behavior.
test/plugins/scenarios/grpc/plugin.yml	Declare additional supported gRPC versions for scenario testing.

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

Copilot · 2026-04-10T10:03:52Z

plugins/core/instrument/enhance.go

+func generateFuncTypeName(n *dst.FuncType) string {
+	result := "func("
+	if n.Params != nil {
+		for i, field := range n.Params.List {
+			if i > 0 {
+				result += ", "
+			}
+			if len(field.Names) > 0 {
+				for j, name := range field.Names {
+					if j > 0 {
+						result += ", "
+					}
+					result += name.Name + " " + generateTypeNameByExp(field.Type)
+				}
+			} else {
+				result += generateTypeNameByExp(field.Type)
+			}
+		}
+	}
+	result += ")"
+	if n.Results != nil && len(n.Results.List) > 0 {
+		if len(n.Results.List) == 1 && len(n.Results.List[0].Names) == 0 {
+			result += " " + generateTypeNameByExp(n.Results.List[0].Type)
+		} else {
+			result += " ("
+			for i, field := range n.Results.List {
+				if i > 0 {
+					result += ", "
+				}
+				if len(field.Names) > 0 {
+					for j, name := range field.Names {
+						if j > 0 {
+							result += ", "
+						}
+						result += name.Name + " " + generateTypeNameByExp(field.Type)
+					}
+				} else {
+					result += generateTypeNameByExp(field.Type)
+				}


generateFuncTypeName currently includes parameter/result variable names when present (e.g., it will produce func(ctx context.Context) error). This will break WithArgType/WithResultType matching for func-typed args in real code where func signatures commonly have named params, since matchers throughout the repo use name-less strings like func(context.Context, *primitive.SendResult, error). Consider generating func type names using only types (ignore field.Names) so matching is stable and independent of source-level naming.

Suggested change

func generateFuncTypeName(n *dst.FuncType) string {

result := "func("

if n.Params != nil {

for i, field := range n.Params.List {

if i > 0 {

result += ", "

}

if len(field.Names) > 0 {

for j, name := range field.Names {

if j > 0 {

result += ", "

}

result += name.Name + " " + generateTypeNameByExp(field.Type)

}

} else {

result += generateTypeNameByExp(field.Type)

}

}

}

result += ")"

if n.Results != nil && len(n.Results.List) > 0 {

if len(n.Results.List) == 1 && len(n.Results.List[0].Names) == 0 {

result += " " + generateTypeNameByExp(n.Results.List[0].Type)

} else {

result += " ("

for i, field := range n.Results.List {

if i > 0 {

result += ", "

}

if len(field.Names) > 0 {

for j, name := range field.Names {

if j > 0 {

result += ", "

}

result += name.Name + " " + generateTypeNameByExp(field.Type)

}

} else {

result += generateTypeNameByExp(field.Type)

}

func generateFieldTypeNames(fields []*dst.Field) []string {

var result []string

for _, field := range fields {

typeName := generateTypeNameByExp(field.Type)

if len(field.Names) > 0 {

for range field.Names {

result = append(result, typeName)

}

} else {

result = append(result, typeName)

}

}

return result

}

func generateFuncTypeName(n *dst.FuncType) string {

result := "func("

if n.Params != nil {

params := generateFieldTypeNames(n.Params.List)

for i, param := range params {

if i > 0 {

result += ", "

}

result += param

}

}

result += ")"

if n.Results != nil && len(n.Results.List) > 0 {

results := generateFieldTypeNames(n.Results.List)

if len(results) == 1 {

result += " " + results[0]

} else if len(results) > 1 {

result += " ("

for i, res := range results {

if i > 0 {

result += ", "

}

result += res

Copilot · 2026-04-10T10:03:52Z

plugins/core/instrument/enhance_test.go

+				}},
+			},
+			"func(context.Context, *primitive.SendResult, error)",
+		},


The new func-type generation tests don’t cover func signatures with named parameters/results (which are common in callback types). Adding at least one case with named params (e.g., func(ctx context.Context, err error) expected to stringify without names) would help prevent regressions once func type name generation is made name-insensitive.

Suggested change

},

},

{

"func with named params and named result",

&dst.FuncType{

Params: &dst.FieldList{List: []*dst.Field{

{

Names: []*dst.Ident{dst.NewIdent("ctx")},

Type: &dst.SelectorExpr{X: dst.NewIdent("context"), Sel: dst.NewIdent("Context")},

},

{

Names: []*dst.Ident{dst.NewIdent("err")},

Type: dst.NewIdent("error"),

},

}},

Results: &dst.FieldList{List: []*dst.Field{

{

Names: []*dst.Ident{dst.NewIdent("sendErr")},

Type: dst.NewIdent("error"),

},

}},

},

"func(context.Context, error) error",

},

mrproliu · 2026-04-10T10:05:13Z

Thanks, is this a new feature to support some gRPC type? Please fix the CI and update the CHANGES.md and support-plugins.md

…WithResultType matching

dingsongjie · 2026-04-10T10:33:24Z

Thanks, is this a new feature to support some gRPC type? Please fix the CI and update the CHANGES.md and support-plugins.md

Fix gRPC plugin panic on v1.69+ by supporting transport.ServerStream type

…ibility

mrproliu · 2026-04-10T10:37:06Z

Thanks, is this a new feature to support some gRPC type? Please fix the CI and update the CHANGES.md and support-plugins.md

Fix gRPC plugin panic on v1.69+ by supporting transport.ServerStream type

Got it, please fix the comment(also Copilot review).

…rule

dingsongjie and others added 4 commits April 10, 2026 14:05

wu-sheng requested review from Copilot and mrproliu April 10, 2026 09:59

wu-sheng added this to the 0.7.0 milestone Apr 10, 2026

Copilot started reviewing on behalf of wu-sheng April 10, 2026 10:00 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

fix: generateFuncTypeName ignores field names for stable WithArgType/…

822eba1

…WithResultType matching

docs: update CHANGES.md and support-plugins.md for gRPC v1.69+ compat…

ae68d85

…ibility

dingsongjie force-pushed the fix-grpc-serverstream-compat branch from d43e9e3 to ae68d85 Compare April 10, 2026 10:37

fix: split TestGenerateTypeNameByExp_FuncType to satisfy funlen lint …

6089e06

…rule

dingsongjie force-pushed the fix-grpc-serverstream-compat branch from 1c965ac to 6089e06 Compare April 13, 2026 01:15

Merge branch 'apache:main' into fix-grpc-serverstream-compat

2d34ba9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix grpc serverstream compat#243

Fix grpc serverstream compat#243
dingsongjie wants to merge 8 commits intoapache:mainfrom
dingsongjie:fix-grpc-serverstream-compat

dingsongjie commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

mrproliu commented Apr 10, 2026

Uh oh!

dingsongjie commented Apr 10, 2026

Uh oh!

mrproliu commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

-		},
+		},
+		{
+			"func with named params and named result",
+			&dst.FuncType{
+				Params: &dst.FieldList{List: []*dst.Field{
+					{
+						Names: []*dst.Ident{dst.NewIdent("ctx")},
+						Type:  &dst.SelectorExpr{X: dst.NewIdent("context"), Sel: dst.NewIdent("Context")},
+					},
+					{
+						Names: []*dst.Ident{dst.NewIdent("err")},
+						Type:  dst.NewIdent("error"),
+					},
+				}},
+				Results: &dst.FieldList{List: []*dst.Field{
+					{
+						Names: []*dst.Ident{dst.NewIdent("sendErr")},
+						Type:  dst.NewIdent("error"),
+					},
+				}},
+			},
+			"func(context.Context, error) error",
+		},

Conversation

dingsongjie commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

mrproliu commented Apr 10, 2026

Uh oh!

dingsongjie commented Apr 10, 2026

Uh oh!

mrproliu commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants