feat: remove grpc_opentracing interceptors, migrate to OTEL stats handler by ankurs · Pull Request #25 · go-coldbrew/interceptors

ankurs · 2026-03-28T01:22:23Z

Summary

Remove grpc_opentracing interceptors from the chain. gRPC tracing is now handled by otelgrpc.NewServerHandler()/NewClientHandler() stats handlers configured at the server/client level in core.

Changes

Remove grpc_opentracing.UnaryServerInterceptor from server chain
Remove grpc_opentracing.StreamServerInterceptor from server stream chain
Remove grpc_opentracing.StreamClientInterceptor from client stream chain
Remove tracing option filtering from DefaultClientInterceptors/DefaultClientStreamInterceptors
Deprecate GRPCClientInterceptor (returns no-op, kept for backwards compat)
Remove grpc-ecosystem/go-grpc-middleware/tracing/opentracing dependency

Depends on

feat: migrate from OpenTracing to native OpenTelemetry APIs tracing#20 (tracing package OTEL migration)
Core must add otelgrpc.NewServerHandler()/NewClientHandler() stats handlers

Test plan

go test -race ./... — passes
make lint — 0 issues

Summary by CodeRabbit

Release Notes

Deprecated
- gRPC client interceptor now deprecated and returns a no-op interceptor.
Chores
- Updated dependencies for error handling and tracing modules.
- Removed direct gRPC middleware dependency.
- Added OpenTelemetry framework dependencies.

…dler Remove grpc_opentracing unary/stream server/client interceptors from the interceptor chain. gRPC tracing is now handled by otelgrpc stats handlers configured at the server/client level in core. - Remove grpc-ecosystem/go-grpc-middleware/tracing/opentracing dependency - Deprecate GRPCClientInterceptor (now returns no-op) - Simplify option handling in DefaultClientInterceptors/StreamInterceptors

coderabbitai · 2026-03-28T01:22:30Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9b10b917-56ff-46ac-99bf-2b61f224d77a

📥 Commits

Reviewing files that changed from the base of the PR and between a7f0228 and 7ca1201.

⛔ Files ignored due to path filters (1)

go.sum is excluded by !**/*.sum

📒 Files selected for processing (4)

README.md
go.mod
interceptors.go
interceptors_test.go

📝 Walkthrough

Walkthrough

The pull request deprecates the GRPCClientInterceptor function by converting it to a no-op passthrough interceptor and removes the dependency on grpc-ecosystem/go-grpc-middleware opentracing functionality. Dependencies are updated to use OpenTelemetry modules while removing direct reliance on opentracing, with corresponding documentation and test updates.

Changes

Cohort / File(s)	Summary
Documentation Updates `README.md`	Updated `GRPCClientInterceptor` signature from `grpc_opentracing.Option` to `interface{}`, added deprecation note stating tracing is now handled by `otelgrpc.NewClientHandler`, and adjusted anchor/link references for exported functions and types.
Dependency Management `go.mod`	Bumped `github.com/go-coldbrew/errors` to v0.2.6, replaced `github.com/go-coldbrew/tracing` with v0.2.0, removed direct dependency on `github.com/grpc-ecosystem/go-grpc-middleware` v1.4.0, pruned opentracing transitive dependency, and added OpenTelemetry modules (`go.opentelemetry.io/otel/*`) at v1.42.0.
Core Logic Changes `interceptors.go`	Removed `github.com/grpc-ecosystem/go-grpc-middleware/tracing/opentracing` dependency, deleted opentracing interceptor chains, changed `GRPCClientInterceptor` to return a no-op passthrough interceptor instead of wrapping `grpc_opentracing.UnaryClientInterceptor`, and updated documentation to mark function as deprecated.
Test Coverage `interceptors_test.go`	Added `TestGRPCClientInterceptorNoOp` test that verifies the interceptor correctly invokes the underlying invoker without errors.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hop along, old opentracing way!
We've found a newer path to play,
OTel now handles traces with care divine,
While deprecated interceptors gracefully decline,
Progress marches on, both sleek and spry! 🌿

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/otel-migration

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

Removes legacy OpenTracing-based gRPC interceptors from this library and aligns tracing responsibilities with OpenTelemetry otelgrpc stats handlers configured at the gRPC server/client level (in core), while keeping a deprecated shim for the old GRPCClientInterceptor API surface.

Changes:

Removed grpc_opentracing unary/stream interceptors from default server/client interceptor chains and stopped filtering opentracing options from DefaultClient*Interceptors.
Deprecated GRPCClientInterceptor and changed it to a no-op interceptor for compatibility.
Dropped the go-grpc-middleware/tracing/opentracing dependency and updated module metadata (go.mod/go.sum), including OTEL-related indirects.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

File	Description
interceptors.go	Removes OpenTracing interceptors from default chains; deprecates `GRPCClientInterceptor` as a no-op.
go.mod	Removes `github.com/grpc-ecosystem/go-grpc-middleware` v1 requirement and adds OTEL indirects.
go.sum	Cleans up sums after dependency removals/upgrades (incl. OTEL version bump).
README.md	Regenerates docs to reflect updated API signatures/line references.

Comments suppressed due to low confidence (1)

interceptors.go:316

DefaultClientStreamInterceptors still accepts defaultOpts ...interface{} but no longer uses it after removing the opentracing option filtering. This means callers can keep passing options that will now be silently ignored, which is easy to miss. Consider documenting that defaultOpts is intentionally ignored (or renaming the parameter to _), and/or planning to remove this parameter in the next major version.

func DefaultClientStreamInterceptors(defaultOpts ...interface{}) []grpc.StreamClientInterceptor {
	ints := []grpc.StreamClientInterceptor{}
	if len(streamClientInterceptors) > 0 {
		ints = append(ints, streamClientInterceptors...)
	}
	if useCBClientInterceptors {
		ints = append(ints,
			nrgrpc.StreamClientInterceptor,
			getClientMetrics().StreamClientInterceptor(),
		)
	}

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

Copilot · 2026-03-28T01:26:41Z

+func GRPCClientInterceptor(options ...interface{}) grpc.UnaryClientInterceptor {
+	return func(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error {
+		return invoker(ctx, method, req, reply, cc, opts...)
+	}


No test coverage was added for the new no-op behavior of GRPCClientInterceptor. Since this function is kept for compatibility, it’d be valuable to add a small unit test asserting the returned interceptor calls invoker exactly once and propagates its returned error unchanged (and does not mutate the context/opts).

Added in e7af476 — TestGRPCClientInterceptorNoOp verifies the returned interceptor calls the invoker exactly once and propagates no error.

Copilot

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.

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

Copilot

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

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

Copilot · 2026-03-28T07:55:59Z

+// Deprecated: GRPCClientInterceptor is no longer needed. gRPC tracing is now handled
+// by otelgrpc.NewClientHandler stats handler configured at the client level.
+// This function is retained for backwards compatibility but returns a no-op interceptor.
+func GRPCClientInterceptor(options ...interface{}) grpc.UnaryClientInterceptor {


GRPCClientInterceptor's options parameter is intentionally unused, and it currently shadows the imported options package name within this function scope. Consider making the variadic argument unnamed or _ (e.g., _ ...interface{}) to signal it is ignored and avoid identifier shadowing.

Suggested change

func GRPCClientInterceptor(options ...interface{}) grpc.UnaryClientInterceptor {

func GRPCClientInterceptor(_ ...interface{}) grpc.UnaryClientInterceptor {

Fixed in 2dd8f6f — changed to _ ...interface{} to signal the param is intentionally unused and avoid shadowing the options package.

Copilot AI review requested due to automatic review settings March 28, 2026 01:22

Copilot started reviewing on behalf of ankurs March 28, 2026 01:22 View session

ankurs mentioned this pull request Mar 28, 2026

feat: flip OpenTracing bridge default to false, use OTEL for HTTP middleware go-coldbrew/core#51

Merged

2 tasks

Copilot AI reviewed Mar 28, 2026

View reviewed changes

test: add no-op GRPCClientInterceptor test

e7af476

ankurs requested a review from Copilot March 28, 2026 05:26

Copilot started reviewing on behalf of ankurs March 28, 2026 05:26 View session

Copilot AI reviewed Mar 28, 2026

View reviewed changes

ankurs requested a review from Copilot March 28, 2026 07:52

Copilot started reviewing on behalf of ankurs March 28, 2026 07:52 View session

Copilot AI reviewed Mar 28, 2026

View reviewed changes

ankurs added 2 commits March 28, 2026 16:12

fix: use blank identifier for unused deprecated options param

2dd8f6f

chore: bump tracing v0.2.0, errors v0.2.6

7ca1201

ankurs merged commit 0a629bd into main Mar 28, 2026
5 of 6 checks passed

ankurs deleted the feat/otel-migration branch March 28, 2026 09:11

This was referenced Apr 4, 2026

chore: bump errors to v0.2.13, apply go fix #34

Merged

feat: add rate limiting interceptor with pluggable limiter #39

Merged

coderabbitai Bot mentioned this pull request Apr 17, 2026

feat: add Executor hook for pluggable resilience #45

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: remove grpc_opentracing interceptors, migrate to OTEL stats handler#25

feat: remove grpc_opentracing interceptors, migrate to OTEL stats handler#25
ankurs merged 4 commits intomainfrom
feat/otel-migration

ankurs commented Mar 28, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Mar 28, 2026 •

edited

Loading

Review failed

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

ankurs Mar 28, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

ankurs Mar 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	func GRPCClientInterceptor(options ...interface{}) grpc.UnaryClientInterceptor {
	func GRPCClientInterceptor(_ ...interface{}) grpc.UnaryClientInterceptor {

Conversation

ankurs commented Mar 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Depends on

Test plan

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

ankurs Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

ankurs Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ankurs commented Mar 28, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 28, 2026 •

edited

Loading