Skip to content
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

Bugfix #115

Merged
merged 2 commits into from
Jun 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion strategy/sampling/centralized.go
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ func (ss *CentralizedStrategy) refreshTargets() (err error) {
logger.Infof("Refreshing sampling rules out-of-band.")

go func() {
if err = ss.refreshManifest(); err != nil {
if err := ss.refreshManifest(); err != nil {
logger.Debugf("Error occurred refreshing sampling rules out-of-band. %v", err)
}
}()
Expand Down
25 changes: 20 additions & 5 deletions xray/httptrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ import (
)

// HTTPSubsegments is a set of context in different HTTP operation.
// Note: from ClientTrace godoc
// Functions may be called concurrently from different goroutines
//
// HTTPSubsegments must operate as though all functions on it can be called in
// different goroutines and must protect against races
type HTTPSubsegments struct {
opCtx context.Context
connCtx context.Context
Expand All @@ -39,6 +44,8 @@ func NewHTTPSubsegments(opCtx context.Context) *HTTPSubsegments {
// GetConn begins a connect subsegment if the HTTP operation
// subsegment is still in progress.
func (xt *HTTPSubsegments) GetConn(hostPort string) {
xt.mu.Lock()
defer xt.mu.Unlock()
if GetSegment(xt.opCtx).safeInProgress() {
xt.connCtx, _ = BeginSubsegment(xt.opCtx, "connect")
}
Expand All @@ -60,6 +67,8 @@ func (xt *HTTPSubsegments) DNSStart(info httptrace.DNSStartInfo) {
// and whether or not the call was coalesced is added as
// metadata to the dns subsegment.
func (xt *HTTPSubsegments) DNSDone(info httptrace.DNSDoneInfo) {
xt.mu.Lock()
defer xt.mu.Unlock()
if xt.dnsCtx != nil && GetSegment(xt.opCtx).safeInProgress() {
metadata := make(map[string]interface{})
metadata["addresses"] = info.Addrs
Expand All @@ -85,6 +94,8 @@ func (xt *HTTPSubsegments) ConnectStart(network, addr string) {
// (if any). Information about the network over which the dial
// was made is added as metadata to the subsegment.
func (xt *HTTPSubsegments) ConnectDone(network, addr string, err error) {
xt.mu.Lock()
defer xt.mu.Unlock()
if xt.connectCtx != nil && GetSegment(xt.opCtx).safeInProgress() {
metadata := make(map[string]interface{})
metadata["network"] = network
Expand All @@ -97,6 +108,8 @@ func (xt *HTTPSubsegments) ConnectDone(network, addr string, err error) {
// TLSHandshakeStart begins a tls subsegment if the HTTP operation
// subsegment is still in progress.
func (xt *HTTPSubsegments) TLSHandshakeStart() {
xt.mu.Lock()
defer xt.mu.Unlock()
if GetSegment(xt.opCtx).safeInProgress() && xt.connCtx != nil {
xt.tlsCtx, _ = BeginSubsegment(xt.connCtx, "tls")
}
Expand All @@ -107,6 +120,8 @@ func (xt *HTTPSubsegments) TLSHandshakeStart() {
// error value(if any). Information about the tls connection
// is added as metadata to the subsegment.
func (xt *HTTPSubsegments) TLSHandshakeDone(connState tls.ConnectionState, err error) {
xt.mu.Lock()
defer xt.mu.Unlock()
if xt.tlsCtx != nil && GetSegment(xt.opCtx).safeInProgress() {
metadata := make(map[string]interface{})
metadata["did_resume"] = connState.DidResume
Expand All @@ -125,14 +140,14 @@ func (xt *HTTPSubsegments) TLSHandshakeDone(connState tls.ConnectionState, err e
// metadata to the subsegment. If the connection is marked as reused,
// the connect subsegment is deleted.
func (xt *HTTPSubsegments) GotConn(info *httptrace.GotConnInfo, err error) {
xt.mu.Lock()
defer xt.mu.Unlock()
if xt.connCtx != nil && GetSegment(xt.opCtx).safeInProgress() { // GetConn may not have been called (client_test.TestBadRoundTrip)
if info != nil {
if info.Reused {
GetSegment(xt.opCtx).RemoveSubsegment(GetSegment(xt.connCtx))
xt.mu.Lock()
// Remove the connCtx context since it is no longer needed.
xt.connCtx = nil
xt.mu.Unlock()
} else {
metadata := make(map[string]interface{})
metadata["reused"] = info.Reused
Expand All @@ -159,12 +174,12 @@ func (xt *HTTPSubsegments) GotConn(info *httptrace.GotConnInfo, err error) {
// subsegment is still in progress, passing the error value
// (if any). The response subsegment is then begun.
func (xt *HTTPSubsegments) WroteRequest(info httptrace.WroteRequestInfo) {
xt.mu.Lock()
defer xt.mu.Unlock()
if xt.reqCtx != nil && GetSegment(xt.opCtx).InProgress {
GetSegment(xt.reqCtx).Close(info.Err)
resCtx, _ := BeginSubsegment(xt.opCtx, "response")
xt.mu.Lock()
xt.responseCtx = resCtx
xt.mu.Unlock()
}

// In case the GotConn http trace handler wasn't called,
Expand All @@ -180,8 +195,8 @@ func (xt *HTTPSubsegments) WroteRequest(info httptrace.WroteRequestInfo) {
// operation subsegment is still in progress.
func (xt *HTTPSubsegments) GotFirstResponseByte() {
xt.mu.Lock()
defer xt.mu.Unlock()
resCtx := xt.responseCtx
xt.mu.Unlock()
if resCtx != nil && GetSegment(xt.opCtx).InProgress {
GetSegment(resCtx).Close(nil)
}
Expand Down