-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
stats/opentelemetry: add trace event for name resolution delay #7992
base: master
Are you sure you want to change the base?
Conversation
c9eb817
to
d9e5843
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7992 +/- ##
==========================================
- Coverage 82.17% 82.16% -0.01%
==========================================
Files 381 384 +3
Lines 38539 38679 +140
==========================================
+ Hits 31668 31781 +113
- Misses 5564 5587 +23
- Partials 1307 1311 +4
|
522379d
to
fc16e79
Compare
…y_test to opentelemetry
fc16e79
to
af9b139
Compare
// nameResolutionStartTime track the start time since name resolution started. | ||
nameResolutionStartTime time.Time | ||
// nameResolutionInProgress indicate if name resolution is in progress. | ||
nameResolutionInProgress bool |
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.
I don't think we want these to be fields on the ClientConn
. These are supposed to be part of the RPC's lifecycle since that's what we're tracing.
@@ -674,13 +680,27 @@ func (cc *ClientConn) Connect() { | |||
// context expires. Returns nil unless the context expires first; otherwise | |||
// returns a status error based on the context. | |||
func (cc *ClientConn) waitForResolvedAddrs(ctx context.Context) error { | |||
// Set the start time for name resolution if it's not already set. | |||
cc.mu.Lock() |
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.
This is called on every RPC.
You're unconditionally setting this flag and timestamp at the start of every RPC. That can't be right.
This PR adds a bonus feature to grfc A72 to emit a trace event for when the initial RPCs block on the name resolver.
RELEASE NOTES: