-
Notifications
You must be signed in to change notification settings - Fork 4.5k
stats/opentelemetry: separate out interceptors for tracing and metrics #8063
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
Merged
purnesh42H
merged 67 commits into
grpc:master
from
janardhanvissa:refactor-tracing-metrics
May 5, 2025
Merged
Changes from 3 commits
Commits
Show all changes
67 commits
Select commit
Hold shift + click to select a range
7ddbe46
replace dial with newclient
janardhankrishna-sai 0486355
Merge branch 'master' of https://github.com/janardhanvissa/grpc-go
janardhankrishna-sai 53fa9cd
Merge branch 'grpc:master' into master
janardhanvissa 4435b8a
Merge branch 'grpc:master' into master
janardhanvissa a413555
Merge branch 'grpc:master' into master
janardhanvissa 4e203c3
Merge branch 'grpc:master' into master
janardhanvissa e9ad552
Merge branch 'grpc:master' into master
janardhanvissa 71804b4
refactor tracing and metrics interceptors separately
janardhankrishna-sai 4b3bd26
adding opentelemetry tracing for client-server
janardhankrishna-sai 69df069
fixing vet issues
janardhankrishna-sai 770f430
reverting newclient changes and fixing vet issues
janardhankrishna-sai b97a3ca
reverting otel trace for client/server changes
janardhankrishna-sai c89f3c9
fixing vet issue
janardhankrishna-sai 3f07e48
renaming receiver name
janardhankrishna-sai 5e8a4a5
unused param fix
janardhankrishna-sai 76e422a
fixing vet issue
janardhankrishna-sai 8fa0b03
refactor client interceptor separately for traces and metrics
janardhankrishna-sai 1f41a49
moving tracing code to client_tracing.go file
janardhankrishna-sai d74c61d
revert previous commit
janardhankrishna-sai 170eef6
adding separate interceptors for traces and metrics of server
janardhankrishna-sai 7f5f539
separating HandleRPC interceptors of traces and metrics
janardhankrishna-sai 50999a0
updating client and server metrics
janardhankrishna-sai 68b8966
removing metrics code from tracingstatshandler and unused parameters
janardhankrishna-sai 8b56f0f
addressing review comments
janardhankrishna-sai ac6080f
addressing review comments
janardhankrishna-sai ed5506c
Merge branch 'master' into refactor-tracing-metrics
janardhanvissa be72377
updating go.sum
janardhankrishna-sai a5ed115
fixing linter issue
janardhankrishna-sai f243b43
addressing review comments for client
janardhankrishna-sai efb738f
addressing review comments for client
janardhankrishna-sai 7b97c65
Merge branch 'master' into refactor-tracing-metrics
janardhanvissa e8b0180
addressing review comments for server
janardhankrishna-sai f70274b
fixing vet issue
janardhankrishna-sai 5e607c5
updating newServerStatsHandler
janardhankrishna-sai a850532
updating var name
janardhankrishna-sai 0f39f8d
moving tracing code to a separate file and adding comments
janardhankrishna-sai 2a914e3
removing unused isMetricsEnabled()
janardhankrishna-sai 05b4278
addressing review comments and updating doc string
janardhankrishna-sai 105efe9
addressing review comments
janardhankrishna-sai 3d78d59
creating helper func for rpcinfo
janardhankrishna-sai 44fce2d
addressing review comments
janardhankrishna-sai e5f50b7
updating logger
janardhankrishna-sai ffcc0fa
Merge branch 'master' into refactor-tracing-metrics
janardhanvissa 67e5b24
update unaryInterceptor
janardhankrishna-sai 8cf7f15
revert doc string
janardhankrishna-sai 7949501
fixing vet issue
janardhankrishna-sai ad22a22
update clientStatsHandler unaryInterceptor
janardhankrishna-sai dd179fb
updating verbosity level and removing redundant code
janardhankrishna-sai 3201805
reverting logger.Info from logger.Error
janardhankrishna-sai a3915ef
Update server_metrics.go
janardhanvissa 1ff5159
addressing review comments
janardhankrishna-sai 4133dd3
updating the comment
janardhankrishna-sai f8e0cac
addressing review comments
janardhankrishna-sai 0b89cf6
fixing vet issue
janardhankrishna-sai 42c08a0
fixing linter issue
janardhankrishna-sai 5bb48b3
addressing review comments
janardhankrishna-sai fb8025d
addressing review comments
janardhankrishna-sai cc39d1a
addressing review comments
janardhankrishna-sai eb8321f
Initializes ri to &rpcInfo{} when it's nil, separating the initializa…
janardhankrishna-sai c858ed2
Merge branch 'master' into refactor-tracing-metrics
janardhanvissa 5c57489
fixing vet issue
janardhankrishna-sai cc12a84
addressing review comments
janardhankrishna-sai aae5419
Merge branch 'master' into refactor-tracing-metrics
janardhanvissa 18a9c18
fixing merge conflicts
janardhankrishna-sai 130ca22
addressing review comments
janardhankrishna-sai 397abf0
addressing review comments
janardhankrishna-sai f4b4fd6
update client_metrics.go
janardhankrishna-sai File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,18 +67,25 @@ | |
rm.registerMetrics(metrics, meter) | ||
} | ||
|
||
func (h *clientMetricsHandler) unaryInterceptor(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { | ||
// getOrCreateCallInfo returns the existing callInfo from context if present, | ||
// or creates and attaches a new one. | ||
func getOrCreateCallInfo(ctx context.Context, cc *grpc.ClientConn, method string, opts ...grpc.CallOption) (context.Context, *callInfo) { | ||
ci := getCallInfo(ctx) | ||
if ci == nil { | ||
if logger.V(2) { | ||
logger.Info("Creating new CallInfo since its not present in context in clientStatsHandler unaryInterceptor") | ||
logger.Info("Creating new CallInfo since its not present in context") | ||
} | ||
ci = &callInfo{ | ||
target: cc.CanonicalTarget(), | ||
method: determineMethod(method, opts...), | ||
} | ||
ctx = setCallInfo(ctx, ci) | ||
} | ||
return ctx, ci | ||
} | ||
|
||
func (h *clientMetricsHandler) unaryInterceptor(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { | ||
ctx, ci := getOrCreateCallInfo(ctx, cc, method, opts...) | ||
|
||
if h.options.MetricsOptions.pluginOption != nil { | ||
md := h.options.MetricsOptions.pluginOption.GetMetadata() | ||
|
@@ -108,17 +115,7 @@ | |
} | ||
|
||
func (h *clientMetricsHandler) streamInterceptor(ctx context.Context, desc *grpc.StreamDesc, cc *grpc.ClientConn, method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) { | ||
ci := getCallInfo(ctx) | ||
if ci == nil { | ||
if logger.V(2) { | ||
logger.Info("Creating new CallInfo since its not present in context in clientStatsHandler streamInterceptor") | ||
} | ||
ci = &callInfo{ | ||
target: cc.CanonicalTarget(), | ||
method: determineMethod(method, opts...), | ||
} | ||
ctx = setCallInfo(ctx, ci) | ||
} | ||
ctx, ci := getOrCreateCallInfo(ctx, cc, method, opts...) | ||
|
||
if h.options.MetricsOptions.pluginOption != nil { | ||
md := h.options.MetricsOptions.pluginOption.GetMetadata() | ||
|
@@ -156,6 +153,20 @@ | |
// HandleConn exists to satisfy stats.Handler. | ||
func (h *clientMetricsHandler) HandleConn(context.Context, stats.ConnStats) {} | ||
|
||
// getOrCreateRPCAttemptInfo retrieves or creates an rpc attemptInfo object | ||
// and ensures it is set in the context along with the rpcInfo. | ||
func getOrCreateRPCAttemptInfo(ctx context.Context) (context.Context, *attemptInfo) { | ||
ri := getRPCInfo(ctx) | ||
if ri == nil { | ||
ri = &rpcInfo{} | ||
} | ||
ai := ri.ai | ||
if ai == nil { | ||
ai = &attemptInfo{} | ||
} | ||
return setRPCInfo(ctx, &rpcInfo{ai: ai}), ai | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is adding it into the context a second time if it's already in there. I think you want something more like: ri := getRPCInfo(ctx)
if ri != nil {
return ctx, ri.ai
}
ri := &rpcInfo{ai: &attemptInfo{}}
return setRPCInfo(ctx, ri), ri.ai |
||
} | ||
|
||
// TagRPC implements per RPC attempt context management for metrics. | ||
func (h *clientMetricsHandler) TagRPC(ctx context.Context, info *stats.RPCTagInfo) context.Context { | ||
// Numerous stats handlers can be used for the same channel. The cluster | ||
|
@@ -174,16 +185,7 @@ | |
} | ||
ctx = istats.SetLabels(ctx, labels) | ||
} | ||
ri := getRPCInfo(ctx) | ||
if ri == nil { | ||
ri = &rpcInfo{} | ||
} | ||
var ai *attemptInfo | ||
if ri.ai == nil { | ||
ai = &attemptInfo{} | ||
} else { | ||
ai = ri.ai | ||
} | ||
ctx, ai := getOrCreateRPCAttemptInfo(ctx) | ||
ai.startTime = time.Now() | ||
ai.xdsLabels = labels.TelemetryLabels | ||
ai.method = removeLeadingSlash(info.FullMethodName) | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this not work?