Merge branch 'master' into aws-session

.golangci.yaml

@@ -0,0 +1,35 @@
+run:
+  deadline: 3m
+
+linters:
+  disable-all: true
+  enable:
+    - deadcode
+    - depguard
+    - dupl
+    - errcheck
+    - gochecknoinits
+    - gocritic
+    - gocyclo
+    - gofmt
+    - goimports
+    - golint
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - interfacer
+    - maligned
+    - megacheck
+    - misspell
+    - nakedret
+    - prealloc
+    - scopelint
+    - staticcheck
+    - structcheck
+    - stylecheck
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - varcheck

.travis.yml

@@ -7,7 +7,7 @@ sudo: required
 go:
   - "1.9"
   - "1.10"
-  - "1.11" 
+  - "1.11"
   - "1.12"
   - "tip"
 
@@ -41,9 +41,18 @@ install:
     else
       go get;
     fi
+  # Only bother running lints on the latest version.  Some of the linters fail to install on older versions of go.
+  - if [ $TRAVIS_GO_VERSION == "1.12" ]; then
+      go get github.com/golangci/golangci-lint/cmd/golangci-lint@v1.16.0;
+    fi
 
 script:
   - make test-with-race
+  # We are in the process of resolving all the lint warnings for x-ray-sdk-go.  Still run the linter to expose these
+  # and resolve to clean these up.
+  - if [ $TRAVIS_GO_VERSION == "1.12" ]; then
+      make golangci-lint || true;
+    fi
 
 matrix:
   allow_failures:

CHANGELOG.md

@@ -1,3 +1,21 @@
+Release v1.0.0-rc.12 (2019-06-11)
+================================
+### SDK Breaking Changes
+* Updates `sampling.NewProxy` method to be private. [PR #93](https://github.com/aws/aws-xray-sdk-go/pull/93)
+
+### SDK Enhancements
+* Fixes a bug for failing to close in-progress `connect` subsegments in some cases. [PR #102](https://github.com/aws/aws-xray-sdk-go/pull/102)
+* Fixes data races condition. [PR #103](https://github.com/aws/aws-xray-sdk-go/pull/103)
+* Fixes a nil pointer issue. [PR #109](https://github.com/aws/aws-xray-sdk-go/pull/109)
+* Refactors `newGlobalConfig` to avoid initializing log. [PR #96](https://github.com/aws/aws-xray-sdk-go/pull/96)
+* Adds `-race` to travis test script. [PR #104](https://github.com/aws/aws-xray-sdk-go/pull/104)
+* Fixes data race condition for parallel http client request. [PR #100](https://github.com/aws/aws-xray-sdk-go/pull/100)
+* Adds support for `tx.Prepare`. [PR #95](https://github.com/aws/aws-xray-sdk-go/pull/95)
+* Fixes race bugs with `ClientTrace`. [PR #115](https://github.com/aws/aws-xray-sdk-go/pull/115)
+* Updates lock abstraction for `defaultLogger`. [PR #113](https://github.com/aws/aws-xray-sdk-go/pull/113)
+* Adds `golangci-lint` into travis CI. [PR #114](https://github.com/aws/aws-xray-sdk-go/pull/114)
+* Fixes uncaught error on SQL url parse. [PR #121](https://github.com/aws/aws-xray-sdk-go/pull/121)
+
 Release v1.0.0-rc.11 (2019-03-15)
 ================================
 ### SDK Breaking Changes

CONTRIBUTORS.md

@@ -3,6 +3,7 @@ The following people have contributed to the AWS X-Ray SDK for Go's design and/o
 * Anssi Alaranta
 * Bilal Khan
 * Christopher Radek
+* Jacob Rickerd
 * James Bowman
 * Lulu Zhao
 * Muir Manders

go.mod

@@ -3,11 +3,9 @@ module github.com/aws/aws-xray-sdk-go
 require (
 	github.com/DATA-DOG/go-sqlmock v1.2.0
 	github.com/aws/aws-sdk-go v1.17.12
-	github.com/davecgh/go-spew v0.0.0-20160907170601-6d212800a42e
-	github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af
+	github.com/davecgh/go-spew v0.0.0-20160907170601-6d212800a42e // indirect
 	github.com/pkg/errors v0.8.1
-	github.com/pmezard/go-difflib v0.0.0-20151028094244-d8ed2627bdf0
+	github.com/pmezard/go-difflib v0.0.0-20151028094244-d8ed2627bdf0 // indirect
 	github.com/stretchr/testify v1.1.4
-	golang.org/x/net v0.0.0-20190301231341-16b79f2e4e95
-	golang.org/x/text v0.0.0-20190306152657-5d731a35f486
+	golang.org/x/net v0.0.0-20190311183353-d8887717615a
 )

go.sum

@@ -12,8 +12,9 @@ github.com/pmezard/go-difflib v0.0.0-20151028094244-d8ed2627bdf0 h1:GD+A8+e+wFkq
 github.com/pmezard/go-difflib v0.0.0-20151028094244-d8ed2627bdf0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stretchr/testify v1.1.4 h1:ToftOQTytwshuOSj6bDSolVUa3GINfJP/fg3OkkOzQQ=
 github.com/stretchr/testify v1.1.4/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
-golang.org/x/net v0.0.0-20190301231341-16b79f2e4e95 h1:fY7Dsw114eJN4boqzVSbpVHO6rTdhq6/GnXeu+PKnzU=
-golang.org/x/net v0.0.0-20190301231341-16b79f2e4e95/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
-golang.org/x/text v0.0.0-20190306152657-5d731a35f486 h1:XzEEnDs8NtiSU8gJvzuDjv+Kam+0nPN9pjOA3oZlxIY=
-golang.org/x/text v0.0.0-20190306152657-5d731a35f486/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
-golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
+golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
+golang.org/x/net v0.0.0-20190311183353-d8887717615a h1:oWX7TPOiFAMXLq8o0ikBYfCJVlRHBcsciT5bXOrH628=
+golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
+golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
+golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=
+golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=

internal/logger/logger.go

@@ -15,7 +15,7 @@ import (
 	"github.com/aws/aws-xray-sdk-go/xraylog"
 )
 
-// This internal pacakge hides the actual logging functions from the user.
+// This internal package hides the actual logging functions from the user.
 
 // The Logger instance used by xray to log. Set via xray.SetLogger().
 var Logger xraylog.Logger = xraylog.NewDefaultLogger(os.Stdout, xraylog.LogLevelInfo)

makefile

@@ -19,3 +19,6 @@ test-with-race: test
 
 fmt:
 	go fmt `go list ./... | grep -v vendor`
+
+golangci-lint:
+	golangci-lint run

strategy/sampling/centralized.go

@@ -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)
 			}
 		}()

strategy/sampling/centralized_test.go

@@ -966,7 +966,7 @@ func TestRefreshManifestRuleAddition(t *testing.T) {
 	assert.Equal(t, r2, ss.manifest.Rules[1])
 	assert.Equal(t, r3, ss.manifest.Rules[2])
 
-	// Assert on size of manfiest
+	// Assert on size of manifest
 	assert.Equal(t, 3, len(ss.manifest.Rules))
 	assert.Equal(t, 3, len(ss.manifest.Index))
 

xray/aws.go

@@ -88,19 +88,6 @@ var xRayBeforeSignHandler = request.NamedHandler{
 	},
 }
 
-var xRayAfterSignHandler = request.NamedHandler{
-	Name: "XRayAfterSignHandler",
-	Fn: func(r *request.Request) {
-		endSubsegment(r)
-	},
-}
-
-var xRayBeforeSendHandler = request.NamedHandler{
-	Name: "XRayBeforeSendHandler",
-	Fn: func(r *request.Request) {
-	},
-}
-
 var xRayAfterSendHandler = request.NamedHandler{
 	Name: "XRayAfterSendHandler",
 	Fn: func(r *request.Request) {
@@ -299,8 +286,7 @@ func (j *jsonMap) data() interface{} {
 }
 
 func (j *jsonMap) search(keys ...string) *jsonMap {
-	var object interface{}
-	object = j.data()
+	object := j.data()
 
 	for target := 0; target < len(keys); target++ {
 		if mmap, ok := object.(map[string]interface{}); ok {

xray/config.go

@@ -24,7 +24,7 @@ import (
 )
 
 // SDKVersion records the current X-Ray Go SDK version.
-const SDKVersion = "1.0.0-rc.11"
+const SDKVersion = "1.0.0-rc.12"
 
 // SDKType records which X-Ray SDK customer uses.
 const SDKType = "X-Ray for Go"

xray/config_test.go

@@ -72,7 +72,7 @@ func (te *TestEmitter) Emit(seg *Segment) {}
 func (te *TestEmitter) RefreshEmitterWithAddress(raddr *net.UDPAddr) {}
 
 func (cms *TestContextMissingStrategy) ContextMissing(v interface{}) {
-	fmt.Sprintf("Test ContextMissing Strategy %v", v)
+	fmt.Printf("Test ContextMissing Strategy %v\n", v)
 }
 
 func stashEnv() []string {

xray/default_emitter_test.go

@@ -11,10 +11,11 @@ package xray
 import (
 	"encoding/json"
 	"fmt"
-	"github.com/stretchr/testify/assert"
 	"math/rand"
 	"testing"
 	"time"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestNoNeedStreamingStrategy(t *testing.T) {

xray/default_streaming_strategy_test.go

@@ -9,8 +9,9 @@
 package xray
 
 import (
-	"github.com/stretchr/testify/assert"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestDefaultStreamingStrategyMaxSegmentSize(t *testing.T) {

xray/httptrace.go

@@ -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
@@ -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")
 	}
@@ -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
@@ -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
@@ -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")
 	}
@@ -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
@@ -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
@@ -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,
@@ -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)
 	}

xray/lambda_test.go

@@ -2,8 +2,9 @@ package xray
 
 import (
 	"context"
-	"github.com/stretchr/testify/assert"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestLambdaSegmentEmit(t *testing.T) {

xray/segment.go

@@ -162,7 +162,7 @@ func BeginSubsegment(ctx context.Context, name string) (context.Context, *Segmen
 		name = name[:200]
 	}
 
-	parent := &Segment{}
+	var parent *Segment
 	// first time to create facade segment
 	if getTraceHeaderFromContext(ctx) != nil && GetSegment(ctx) == nil {
 		_, parent = newFacadeSegment(ctx)
@@ -445,10 +445,8 @@ func (seg *Segment) AddError(err error) error {
 	return nil
 }
 
-func (seg *Segment) addError(err error) error {
+func (seg *Segment) addError(err error) {
 	seg.Fault = true
 	seg.GetCause().WorkingDirectory, _ = os.Getwd()
 	seg.GetCause().Exceptions = append(seg.GetCause().Exceptions, seg.ParentSegment.GetConfiguration().ExceptionFormattingStrategy.ExceptionFromError(err))
-
-	return nil
 }

xray/segment_test.go

@@ -39,7 +39,7 @@ func TestSubsegmentDataRace(t *testing.T) {
 		go func() {
 			defer wg.Done()
 			ctx, seg := BeginSubsegment(ctx, "TestSubsegment1")
-			ctx, seg2 := BeginSubsegment(ctx, "TestSubsegment2")
+			_, seg2 := BeginSubsegment(ctx, "TestSubsegment2")
 			seg2.Close(nil)
 			seg.Close(nil)
 		}()