Merge pull request #947 from go-agent/rabolofia/cloudflare-issues

Improvements for when collector timeouts are hit.

v3/newrelic/collector.go

@@ -74,6 +74,8 @@ type rpmResponse struct {
 	// should be used to avoid mismatch between statusCode and Err.
 	Err                      error
 	disconnectSecurityPolicy bool
+	// forceSaveHarvestData overrides the status code and forces a save of data
+	forceSaveHarvestData bool
 }
 
 func newRPMResponse(statusCode int) rpmResponse {
@@ -98,6 +100,9 @@ func (resp rpmResponse) IsRestartException() bool {
 // ShouldSaveHarvestData indicates that the agent should save the data and try
 // to send it in the next harvest.
 func (resp rpmResponse) ShouldSaveHarvestData() bool {
+	if resp.forceSaveHarvestData {
+		return true
+	}
 	switch resp.statusCode {
 	case 408, 429, 500, 503:
 		return true
@@ -165,7 +170,10 @@ func collectorRequestInternal(url string, cmd rpmCmd, cs rpmControls) rpmRespons
 
 	resp, err := cs.Client.Do(req)
 	if err != nil {
-		return rpmResponse{Err: err}
+		return rpmResponse{
+			forceSaveHarvestData: true,
+			Err:                  err,
+		}
 	}
 
 	defer resp.Body.Close()

v3/newrelic/collector_test.go

@@ -9,6 +9,7 @@ import (
 	"net/url"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/newrelic/go-agent/v3/internal"
 	"github.com/newrelic/go-agent/v3/internal/logger"
@@ -133,7 +134,32 @@ func TestCollectorBadRequest(t *testing.T) {
 	if nil == resp.Err {
 		t.Error("missing expected error")
 	}
+}
 
+func TestCollectorTimeout(t *testing.T) {
+	cmd := rpmCmd{
+		Name:              "cmd_name",
+		Collector:         "collector.com",
+		RunID:             "run_id",
+		Data:              nil,
+		RequestHeadersMap: map[string]string{"zip": "zap"},
+		MaxPayloadSize:    100,
+	}
+	cs := rpmControls{
+		License: "the_license",
+		Client: &http.Client{
+			Timeout: time.Nanosecond, // force a timeout
+		},
+		Logger: logger.ShimLogger{IsDebugEnabled: true},
+	}
+	u := "https://example.com"
+	resp := collectorRequestInternal(u, cmd, cs)
+	if nil == resp.Err {
+		t.Error("missing expected error")
+	}
+	if !resp.ShouldSaveHarvestData() {
+		t.Error("harvest data should be saved when timeout occurs")
+	}
 }
 
 func TestUrl(t *testing.T) {

v3/newrelic/internal_app.go

@@ -379,6 +379,10 @@ func (app *app) WaitForConnection(timeout time.Duration) error {
 }
 
 func newApp(c config) *app {
+	transport := c.Transport
+	if nil == transport {
+		transport = collectorDefaultTransport
+	}
 	app := &app{
 		Logger:         c.Logger,
 		config:         c,
@@ -396,7 +400,7 @@ func newApp(c config) *app {
 		rpmControls: rpmControls{
 			License: c.License,
 			Client: &http.Client{
-				Transport: c.Transport,
+				Transport: transport,
 				Timeout:   collectorTimeout,
 			},
 			Logger: c.Logger,

v3/newrelic/transport.go

@@ -0,0 +1,29 @@
+// +build go1.13
+
+package newrelic
+
+import (
+	"net"
+	"net/http"
+	"time"
+)
+
+var (
+	// collectorDefaultTransport is the http.Transport to be used with
+	// communication to the collector backend if a Transport is not set on the
+	// Config.
+	collectorDefaultTransport = &http.Transport{
+		Proxy: http.ProxyFromEnvironment,
+		DialContext: (&net.Dialer{
+			Timeout:   30 * time.Second,
+			KeepAlive: 30 * time.Second,
+			DualStack: true,
+		}).DialContext,
+		ForceAttemptHTTP2:     true, // added in go 1.13
+		MaxIdleConns:          100,
+		MaxIdleConnsPerHost:   100, // note: different from default global transport
+		IdleConnTimeout:       90 * time.Second,
+		TLSHandshakeTimeout:   10 * time.Second,
+		ExpectContinueTimeout: 1 * time.Second,
+	}
+)

v3/newrelic/transport_1_12.go

@@ -0,0 +1,28 @@
+// +build !go1.13
+
+package newrelic
+
+import (
+	"net"
+	"net/http"
+	"time"
+)
+
+var (
+	// collectorDefaultTransport is the http.Transport to be used with
+	// communication to the collector backend if a Transport is not set on the
+	// Config.
+	collectorDefaultTransport = &http.Transport{
+		Proxy: http.ProxyFromEnvironment,
+		DialContext: (&net.Dialer{
+			Timeout:   30 * time.Second,
+			KeepAlive: 30 * time.Second,
+			DualStack: true,
+		}).DialContext,
+		MaxIdleConns:          100,
+		MaxIdleConnsPerHost:   100, // note: different from default global transport
+		IdleConnTimeout:       90 * time.Second,
+		TLSHandshakeTimeout:   10 * time.Second,
+		ExpectContinueTimeout: 1 * time.Second,
+	}
+)