diff --git a/v3/newrelic/collector.go b/v3/newrelic/collector.go index 89e5ed594..12e0d50e9 100644 --- a/v3/newrelic/collector.go +++ b/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() diff --git a/v3/newrelic/collector_test.go b/v3/newrelic/collector_test.go index 746341c24..0dc7d076d 100644 --- a/v3/newrelic/collector_test.go +++ b/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) { diff --git a/v3/newrelic/internal_app.go b/v3/newrelic/internal_app.go index 47c36dcb1..42e170893 100644 --- a/v3/newrelic/internal_app.go +++ b/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, diff --git a/v3/newrelic/transport.go b/v3/newrelic/transport.go new file mode 100644 index 000000000..a3bd0b6b9 --- /dev/null +++ b/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, + } +) diff --git a/v3/newrelic/transport_1_12.go b/v3/newrelic/transport_1_12.go new file mode 100644 index 000000000..8b26dee4a --- /dev/null +++ b/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, + } +)