Merge pull request #616 from newrelic/develop

3.20.2 Release

CHANGELOG.md

@@ -1,3 +1,20 @@
+## 3.20.2
+
+### Added
+* New `NoticeExpectedError()` method allows you to capture errors that you are expecting to handle, without triggering alerts
+
+### Fixed
+* More defensive harvest cycle code that will avoid crashing even in the event of a panic.
+* Update `nats-server` version to avoid known zip-slip exploit
+* Update `labstack/echo` version to mitigate known open redirect exploit
+
+### Support Statement
+New Relic recommends that you upgrade the agent regularly to ensure that you’re getting the latest features and performance benefits. Additionally, older releases will no longer be supported when they reach end-of-life.
+
+We also recommend using the latest version of the Go language. At minimum, you should at least be using no version of Go older than what is supported by the Go team themselves.
+
+See the [Go Agent EOL Policy](https://docs.newrelic.com/docs/apm/agents/go-agent/get-started/go-agent-eol-policy/) for details about supported versions of the Go Agent and third-party components.
+
 ## 3.20.1
 
 ### Added

CONTRIBUTING.md

@@ -24,7 +24,7 @@ Before submitting an Issue, please search for similar ones in the
 
 ## Pull Requests
 
-Pull requests must pass all automated tests and must be reviewed by at least one maintaining engineer before being merged.
+Pull requests must pass all automated tests and must be reviewed by at least one maintaining engineer before being merged. Please contribute all pull requests against the `develop` branch, which is where we stage changes ahead of a release and run our most complete suite of tests.
 
 When contributing a new integration package, please follow the [Writing a New Integration Package](https://github.com/newrelic/go-agent/wiki/Writing-a-New-Integration-Package) wiki page.
 

README.md

@@ -1,5 +1,4 @@
-
-[![Community Plus header](https://github.com/newrelic/opensource-website/raw/master/src/images/categories/Community_Plus.png)](https://opensource.newrelic.com/oss-category/#community-plus)
+[![Community Plus header](https://github.com/newrelic/opensource-website/raw/main/src/images/categories/Community_Plus.png)](https://opensource.newrelic.com/oss-category/#community-plus)
 
 # New Relic Go Agent [![GoDoc](https://godoc.org/github.com/newrelic/go-agent?status.svg)](https://godoc.org/github.com/newrelic/go-agent/v3/newrelic/) [![Go Report Card](https://goreportcard.com/badge/github.com/newrelic/go-agent)](https://goreportcard.com/report/github.com/newrelic/go-agent)
 

v3/examples/server/main.go

@@ -31,6 +31,13 @@ func noticeError(w http.ResponseWriter, r *http.Request) {
 	txn.NoticeError(errors.New("my error message"))
 }
 
+func noticeExpectedError(w http.ResponseWriter, r *http.Request) {
+	io.WriteString(w, "noticing an error")
+
+	txn := newrelic.FromContext(r.Context())
+	txn.NoticeExpectedError(errors.New("my expected error message"))
+}
+
 func noticeErrorWithAttributes(w http.ResponseWriter, r *http.Request) {
 	io.WriteString(w, "noticing an error")
 
@@ -273,6 +280,7 @@ func main() {
 	http.HandleFunc(newrelic.WrapHandleFunc(app, "/", index))
 	http.HandleFunc(newrelic.WrapHandleFunc(app, "/version", versionHandler))
 	http.HandleFunc(newrelic.WrapHandleFunc(app, "/notice_error", noticeError))
+	http.HandleFunc(newrelic.WrapHandleFunc(app, "/notice_expected_error", noticeExpectedError))
 	http.HandleFunc(newrelic.WrapHandleFunc(app, "/notice_error_with_attributes", noticeErrorWithAttributes))
 	http.HandleFunc(newrelic.WrapHandleFunc(app, "/custom_event", customEvent))
 	http.HandleFunc(newrelic.WrapHandleFunc(app, "/set_name", setName))

v3/integrations/nrecho-v4/go.mod

@@ -5,6 +5,6 @@ module github.com/newrelic/go-agent/v3/integrations/nrecho-v4
 go 1.17
 
 require (
-	github.com/labstack/echo/v4 v4.5.0
+	github.com/labstack/echo/v4 v4.9.0
 	github.com/newrelic/go-agent/v3 v3.18.2
 )

v3/integrations/nrstan/test/go.mod

@@ -6,9 +6,9 @@ module github.com/newrelic/go-agent/v3/integrations/nrstan/test
 go 1.13
 
 require (
-	github.com/nats-io/nats-streaming-server v0.24.1
-	github.com/nats-io/stan.go v0.10.2
-	github.com/newrelic/go-agent/v3 v3.4.0
+	github.com/nats-io/nats-streaming-server v0.24.3
+	github.com/nats-io/stan.go v0.10.3
+	github.com/newrelic/go-agent/v3 v3.18.2
 	github.com/newrelic/go-agent/v3/integrations/nrstan v0.0.0
 )
 

v3/internal/jsonx/encode_test.go

@@ -5,6 +5,7 @@ package jsonx
 
 import (
 	"bytes"
+	"fmt"
 	"math"
 	"testing"
 )
@@ -28,6 +29,30 @@ func TestAppendFloat(t *testing.T) {
 	}
 }
 
+func TestAppendFloat32(t *testing.T) {
+	buf := &bytes.Buffer{}
+
+	err := AppendFloat32(buf, float32(math.NaN()))
+	if err == nil {
+		t.Error("AppendFloat(NaN) should return an error")
+	}
+
+	err = AppendFloat32(buf, float32(math.Inf(1)))
+	if err == nil {
+		t.Error("AppendFloat(+Inf) should return an error")
+	}
+
+	err = AppendFloat32(buf, float32(math.Inf(-1)))
+	if err == nil {
+		t.Error("AppendFloat(-Inf) should return an error")
+	}
+
+	err = AppendFloat32(buf, float32(12.5))
+	if err != nil {
+		t.Error("AppendFloat(12.5) should not return an error")
+	}
+}
+
 func TestAppendFloats(t *testing.T) {
 	buf := &bytes.Buffer{}
 
@@ -166,6 +191,15 @@ var encodeStringTests = []struct {
 	{"\\", `"\\"`},
 	{`"`, `"\""`},
 	{"the\u2028quick\t\nbrown\u2029fox", `"the\u2028quick\t\nbrown\u2029fox"`},
+
+	//extra edge cases
+	{string([]byte{237, 159, 193}), `"\ufffd\ufffd\ufffd"`},                            // invalid utf8
+	{string([]byte{55, 237, 159, 193, 55}), `"7\ufffd\ufffd\ufffd7"`},                  // invalid utf8 surrounded by valid utf8
+	{`abcdefghijklmnopqrstuvwxyz1234567890`, `"abcdefghijklmnopqrstuvwxyz1234567890"`}, // alphanumeric
+	{"'", `"'"`},
+	{``, `""`},
+	{`\`, `"\\"`},
+	{fmt.Sprintf("%c", rune(65533)), fmt.Sprintf("\"%c\"", rune(65533))}, // invalid rune utf8 symbol (valid utf8)
 }
 
 func TestAppendString(t *testing.T) {
@@ -181,6 +215,41 @@ func TestAppendString(t *testing.T) {
 	}
 }
 
+func TestAppendStringArray(t *testing.T) {
+	buf := &bytes.Buffer{}
+
+	var encodeStringArrayTests = []struct {
+		in  []string
+		out string
+	}{
+		{
+			in: []string{
+				"hi",
+				"foo",
+			},
+			out: `["hi","foo"]`,
+		},
+		{
+			in: []string{
+				"foo",
+			},
+			out: `["foo"]`,
+		},
+		{
+			in:  []string{},
+			out: `[]`,
+		},
+	}
+
+	for _, tt := range encodeStringArrayTests {
+		buf.Reset()
+
+		AppendStringArray(buf, tt.in...)
+		if got := buf.String(); got != tt.out {
+			t.Errorf("AppendString(%q) = %#q, want %#q", tt.in, got, tt.out)
+		}
+	}
+}
 func BenchmarkAppendString(b *testing.B) {
 	buf := &bytes.Buffer{}
 

v3/newrelic/attributes_from_internal.go

@@ -275,10 +275,8 @@ func (attr agentAttributes) Add(id string, stringVal string, otherVal interface{
 	}
 }
 
-//
 // Remove is used to remove agent attributes.
 // It is not an error if the attribute wasn't present to begin with.
-//
 func (attr agentAttributes) Remove(id string) {
 	if _, ok := attr[id]; ok {
 		delete(attr, id)
@@ -453,14 +451,14 @@ func writeAttributeValueJSON(w *jsonFieldsWriter, key string, val interface{}) {
 }
 
 func agentAttributesJSON(a *attributes, buf *bytes.Buffer, d destinationSet) {
-	if nil == a {
+	if a == nil {
 		buf.WriteString("{}")
 		return
 	}
 	w := jsonFieldsWriter{buf: buf}
 	buf.WriteByte('{')
 	for id, val := range a.Agent {
-		if 0 != a.config.agentDests[id]&d {
+		if a.config.agentDests[id]&d != 0 {
 			if val.stringVal != "" {
 				w.stringField(id, val.stringVal)
 			} else {
@@ -478,12 +476,12 @@ func userAttributesJSON(a *attributes, buf *bytes.Buffer, d destinationSet, extr
 		w := jsonFieldsWriter{buf: buf}
 		for key, val := range extraAttributes {
 			outputDest := applyAttributeConfig(a.config, key, d)
-			if 0 != outputDest&d {
+			if outputDest&d != 0 {
 				writeAttributeValueJSON(&w, key, val)
 			}
 		}
 		for name, atr := range a.user {
-			if 0 != atr.dests&d {
+			if atr.dests&d != 0 {
 				if _, found := extraAttributes[name]; found {
 					continue
 				}

v3/newrelic/errors_from_internal.go

@@ -61,6 +61,7 @@ type errorData struct {
 	Msg             string
 	Klass           string
 	SpanID          string
+	Expect          bool
 }
 
 // txnError combines error data with information about a transaction.  txnError is used for
@@ -113,7 +114,7 @@ func (h *tracedError) WriteJSON(buf *bytes.Buffer) {
 	buf.WriteByte(',')
 	buf.WriteString(`"intrinsics"`)
 	buf.WriteByte(':')
-	intrinsicsJSON(&h.txnEvent, buf)
+	intrinsicsJSON(&h.txnEvent, buf, h.errorData.Expect)
 	if nil != h.Stack {
 		buf.WriteByte(',')
 		buf.WriteString(`"stack_trace"`)
@@ -152,7 +153,7 @@ func mergeTxnErrors(errors *harvestErrors, errs txnErrors, txnEvent txnEvent) {
 }
 
 func (errors harvestErrors) Data(agentRunID string, harvestStart time.Time) ([]byte, error) {
-	if 0 == len(errors) {
+	if len(errors) == 0 {
 		return nil, nil
 	}
 	estimate := 1024 * len(errors)

v3/newrelic/harvest.go

@@ -327,12 +327,16 @@ func createTxnMetrics(args *txnData, metrics *metricTable) {
 	}
 
 	// Error Metrics
-	if args.HasErrors() {
+	if args.NoticeErrors() {
 		metrics.addSingleCount(errorsRollupMetric.all, forced)
 		metrics.addSingleCount(errorsRollupMetric.webOrOther(args.IsWeb), forced)
 		metrics.addSingleCount(errorsPrefix+args.FinalName, forced)
 	}
 
+	if args.HasExpectedErrors() {
+		metrics.addSingleCount(expectedErrorsRollupMetric.all, forced)
+	}
+
 	// Queueing Metrics
 	if args.Queuing > 0 {
 		metrics.addDuration(queueMetric, "", args.Queuing, args.Queuing, forced)

v3/newrelic/harvest_test.go

@@ -771,6 +771,7 @@ func TestCreateTxnMetrics(t *testing.T) {
 	webName := "WebTransaction/zip/zap"
 	backgroundName := "OtherTransaction/zip/zap"
 	args := &txnData{}
+	args.noticeErrors = true
 	args.Duration = 123 * time.Second
 	args.TotalTime = 150 * time.Second
 	args.ApdexThreshold = 2 * time.Second
@@ -803,6 +804,7 @@ func TestCreateTxnMetrics(t *testing.T) {
 	args.FinalName = webName
 	args.IsWeb = true
 	args.Errors = nil
+	args.noticeErrors = false
 	args.Zone = apdexTolerating
 	metrics = newMetricTable(100, time.Now())
 	createTxnMetrics(args, metrics)
@@ -821,6 +823,7 @@ func TestCreateTxnMetrics(t *testing.T) {
 	args.FinalName = backgroundName
 	args.IsWeb = false
 	args.Errors = txnErrors
+	args.noticeErrors = true
 	args.Zone = apdexNone
 	metrics = newMetricTable(100, time.Now())
 	createTxnMetrics(args, metrics)
@@ -838,9 +841,32 @@ func TestCreateTxnMetrics(t *testing.T) {
 		{Name: "ErrorsByCaller/Unknown/Unknown/Unknown/Unknown/allOther", Scope: "", Forced: false, Data: []float64{1, 0, 0, 0, 0, 0}},
 	})
 
+	// Verify expected errors metrics
+	args.FinalName = backgroundName
+	args.IsWeb = false
+	args.Errors = txnErrors
+	args.noticeErrors = false
+	args.expectedErrors = true
+	args.Zone = apdexNone
+	metrics = newMetricTable(100, time.Now())
+	createTxnMetrics(args, metrics)
+	expectMetrics(t, metrics, []internal.WantMetric{
+		{Name: backgroundName, Scope: "", Forced: true, Data: []float64{1, 123, 0, 123, 123, 123 * 123}},
+		{Name: backgroundRollup, Scope: "", Forced: true, Data: []float64{1, 123, 0, 123, 123, 123 * 123}},
+		{Name: "OtherTransactionTotalTime", Scope: "", Forced: true, Data: []float64{1, 150, 150, 150, 150, 150 * 150}},
+		{Name: "OtherTransactionTotalTime/zip/zap", Scope: "", Forced: false, Data: []float64{1, 150, 150, 150, 150, 150 * 150}},
+		{Name: "ErrorsExpected/all", Scope: "", Forced: true, Data: []float64{1, 0, 0, 0, 0, 0}},
+		{Name: "DurationByCaller/Unknown/Unknown/Unknown/Unknown/all", Scope: "", Forced: false, Data: []float64{1, 123, 123, 123, 123, 123 * 123}},
+		{Name: "DurationByCaller/Unknown/Unknown/Unknown/Unknown/allOther", Scope: "", Forced: false, Data: []float64{1, 123, 123, 123, 123, 123 * 123}},
+		{Name: "ErrorsByCaller/Unknown/Unknown/Unknown/Unknown/all", Scope: "", Forced: false, Data: []float64{1, 0, 0, 0, 0, 0}},
+		{Name: "ErrorsByCaller/Unknown/Unknown/Unknown/Unknown/allOther", Scope: "", Forced: false, Data: []float64{1, 0, 0, 0, 0, 0}},
+	})
+
 	args.FinalName = backgroundName
 	args.IsWeb = false
 	args.Errors = nil
+	args.noticeErrors = false
+	args.expectedErrors = false
 	args.Zone = apdexNone
 	metrics = newMetricTable(100, time.Now())
 	createTxnMetrics(args, metrics)
@@ -889,6 +915,7 @@ func TestCreateTxnMetricsOldCAT(t *testing.T) {
 	args.FinalName = webName
 	args.IsWeb = true
 	args.Errors = txnErrors
+	args.noticeErrors = true
 	args.Zone = apdexTolerating
 	metrics := newMetricTable(100, time.Now())
 	createTxnMetrics(args, metrics)
@@ -908,6 +935,7 @@ func TestCreateTxnMetricsOldCAT(t *testing.T) {
 	args.FinalName = webName
 	args.IsWeb = true
 	args.Errors = nil
+	args.noticeErrors = false
 	args.Zone = apdexTolerating
 	metrics = newMetricTable(100, time.Now())
 	createTxnMetrics(args, metrics)
@@ -924,6 +952,7 @@ func TestCreateTxnMetricsOldCAT(t *testing.T) {
 	args.FinalName = backgroundName
 	args.IsWeb = false
 	args.Errors = txnErrors
+	args.noticeErrors = true
 	args.Zone = apdexNone
 	metrics = newMetricTable(100, time.Now())
 	createTxnMetrics(args, metrics)
@@ -940,6 +969,7 @@ func TestCreateTxnMetricsOldCAT(t *testing.T) {
 	args.FinalName = backgroundName
 	args.IsWeb = false
 	args.Errors = nil
+	args.noticeErrors = false
 	args.Zone = apdexNone
 	metrics = newMetricTable(100, time.Now())
 	createTxnMetrics(args, metrics)

v3/newrelic/internal_app.go

@@ -71,16 +71,30 @@ func (app *app) doHarvest(h *harvest, harvestStart time.Time, run *appRun) {
 	payloads := h.Payloads(app.config.DistributedTracer.Enabled)
 	for _, p := range payloads {
 		cmd := p.EndpointMethod()
+		var data []byte
+
+		defer func() {
+			if r := recover(); r != nil {
+				app.Warn("panic occured when creating harvest data", map[string]interface{}{
+					"cmd":   cmd,
+					"panic": r,
+				})
+
+				// make sure the loop continues
+				data = nil
+			}
+		}()
+
 		data, err := p.Data(run.Reply.RunID.String(), harvestStart)
 
-		if nil != err {
+		if err != nil {
 			app.Warn("unable to create harvest data", map[string]interface{}{
 				"cmd":   cmd,
 				"error": err.Error(),
 			})
 			continue
 		}
-		if nil == data {
+		if data == nil {
 			continue
 		}
 
@@ -103,7 +117,7 @@ func (app *app) doHarvest(h *harvest, harvestStart time.Time, run *appRun) {
 			return
 		}
 
-		if nil != resp.Err {
+		if resp.Err != nil {
 			app.Warn("harvest failure", map[string]interface{}{
 				"cmd":         cmd,
 				"error":       resp.Err.Error(),