Merge pull request #650 from newrelic/develop

Release 3.20.4

.github/workflows/ci.yaml

@@ -66,7 +66,6 @@ jobs:
             extratesting: go get -u github.com/gorilla/mux@master
           - go-version: 1.17.x
             dirs: v3/integrations/nrgraphgophers
-            extratesting: go get -u github.com/graph-gophers/graphql-go@master
           - go-version: 1.17.x
             dirs: v3/integrations/nrlogrus
             extratesting: go get -u github.com/sirupsen/logrus@master
@@ -127,7 +126,6 @@ jobs:
             # nrzap only supports the two most recent minor go releases
           - go-version: 1.17.x
             dirs: v3/integrations/nrzap
-            extratesting: go get -u go.uber.org/zap@master
           - go-version: 1.17.x
             dirs: v3/integrations/nrhttprouter
             extratesting: go get -u github.com/julienschmidt/httprouter@master
@@ -141,7 +139,7 @@ jobs:
             extratesting: go get -u github.com/graphql-go/graphql@master
           - go-version: 1.17.x
             dirs: v3/integrations/nrmssql
-            extratesting: go get -u github.com/denisenkom/go-mssqldb@master
+            extratesting: go get -u github.com/microsoft/go-mssqldb@main
 
     steps:
     - name: Install Go

CHANGELOG.md

@@ -1,3 +1,21 @@
+## 3.20.4
+
+### Fixed
+* nrmssql driver updated to use version maintained by Microsoft
+* bug where error messages were not truncated to the maximum size, and would get dropped if they were too large
+* bug where number of span events was hard coded to 1000, and config setting was being ignored
+
+### Added
+* improved performance of ignore error code checks in agent
+* HTTP error codes can be set as expected by adding them to ErrorCollector.ExpectStatusCodes in the config
+
+### 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.3
 
 Please note that the v2 go agent is no longer supported according to our EOL policy. 

CONTRIBUTING.md

@@ -37,4 +37,4 @@ For more information about CLAs, please check out Alex Russell’s excellent pos
 
 ## Slack
 
-We host a public Slack with a dedicated channel for contributors and maintainers of open source projects hosted by New Relic.  If you are contributing to this project, you're welcome to request access to the #oss-contributors channel in the newrelicusers.slack.com workspace.  To request access, see https://newrelicusers-signup.herokuapp.com/.
+We host a public Slack with a dedicated channel for contributors and maintainers of open source projects hosted by New Relic.  If you are contributing to this project, you're welcome to request access to the #oss-contributors channel in the newrelicusers.slack.com workspace.  To request access, please use this [link](https://join.slack.com/t/newrelicusers/shared_invite/zt-1ayj69rzm-~go~Eo1whIQGYnu3qi15ng).

v3/integrations/nrawssdk-v1/go.mod

@@ -7,6 +7,6 @@ go 1.7
 
 require (
 	// v1.15.0 is the first aws-sdk-go version with module support.
-	github.com/aws/aws-sdk-go v1.15.0
+	github.com/aws/aws-sdk-go v1.33.0
 	github.com/newrelic/go-agent/v3 v3.16.0
 )

v3/integrations/nrmssql/README.md

@@ -1,6 +1,6 @@
 # v3/integrations/nrmssql [![GoDoc](https://godoc.org/github.com/newrelic/go-agent/v3/integrations/nrmysql?status.svg)](https://godoc.org/github.com/newrelic/go-agent/v3/integrations/nrmysql)
 
-Package `nrmssql` instruments github.com/denisenkom/go-mssqldb.
+Package `nrmssql` instruments github.com/microsoft/go-mssqldb.
 
 ```go
 import "github.com/newrelic/go-agent/v3/integrations/nrmssql"

v3/integrations/nrmssql/go.mod

@@ -3,18 +3,18 @@ module github.com/newrelic/go-agent/v3/integrations/nrmssql
 go 1.17
 
 require (
-	github.com/denisenkom/go-mssqldb v0.12.2
+	github.com/microsoft/go-mssqldb v0.19.0
 	github.com/newrelic/go-agent/v3 v3.16.1
 )
 
 require (
 	github.com/golang-sql/civil v0.0.0-20190719163853-cb61b32ac6fe // indirect
 	github.com/golang-sql/sqlexp v0.1.0 // indirect
 	github.com/golang/protobuf v1.4.3 // indirect
-	golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897 // indirect
-	golang.org/x/net v0.0.0-20210610132358-84b48f89b13b // indirect
-	golang.org/x/sys v0.0.0-20210423082822-04245dca01da // indirect
-	golang.org/x/text v0.3.6 // indirect
+	golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d // indirect
+	golang.org/x/net v0.0.0-20220425223048-2871e0cb64e4 // indirect
+	golang.org/x/sys v0.0.0-20220224120231-95c6836cb0e7 // indirect
+	golang.org/x/text v0.3.7 // indirect
 	google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013 // indirect
 	google.golang.org/grpc v1.39.0 // indirect
 	google.golang.org/protobuf v1.25.0 // indirect

v3/integrations/nrmssql/nrmssql.go

@@ -1,9 +1,10 @@
 // Copyright 2020 New Relic Corporation. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
+//go:build go1.10
 // +build go1.10
 
-// Package nrmssql instruments github.com/denisenkom/go-mssqldb.
+// Package nrmssql instruments github.com/microsoft/go-mssqldb.
 //
 // Use this package to instrument your MSSQL calls without having to manually
 // create DatastoreSegments.  This is done in a two step process:
@@ -13,7 +14,7 @@
 // If your code is using sql.Open like this:
 //
 //	import (
-//		_ "github.com/denisenkom/go-mssqldb"
+//		_ "github.com/microsoft/go-mssqldb"
 //	)
 //
 //	func main() {
@@ -47,10 +48,11 @@ package nrmssql
 import (
 	"database/sql"
 	"fmt"
-	"github.com/denisenkom/go-mssqldb/msdsn"
-	"github.com/newrelic/go-agent/v3/internal"
 
-	"github.com/denisenkom/go-mssqldb"
+	mssql "github.com/microsoft/go-mssqldb"
+	"github.com/microsoft/go-mssqldb/msdsn"
+
+	"github.com/newrelic/go-agent/v3/internal"
 	"github.com/newrelic/go-agent/v3/newrelic"
 	"github.com/newrelic/go-agent/v3/newrelic/sqlparse"
 )
@@ -71,7 +73,7 @@ func init() {
 }
 
 func parseDSN(s *newrelic.DatastoreSegment, dsn string) {
-	cfg, _, err := msdsn.Parse(dsn)
+	cfg, err := msdsn.Parse(dsn)
 	if nil != err {
 		return
 	}

v3/internal/connect_reply.go

@@ -100,6 +100,7 @@ type ConnectReply struct {
 		TransactionTracerStackTraceThreshold *float64    `json:"transaction_tracer.stack_trace_threshold"`
 		ErrorCollectorEnabled                *bool       `json:"error_collector.enabled"`
 		ErrorCollectorIgnoreStatusCodes      []int       `json:"error_collector.ignore_status_codes"`
+		ErrorCollectorExpectStatusCodes      []int       `json:"error_collector.expected_status_codes"`
 		CrossApplicationTracerEnabled        *bool       `json:"cross_application_tracer.enabled"`
 	} `json:"agent_config"`
 

v3/internal/limits.go

@@ -25,4 +25,7 @@ const (
 	// MaxErrorEvents is the maximum number of Error Events that can be captured
 	// per 60-second harvest cycle
 	MaxErrorEvents = 100
+	// MaxSpanEvents is the maximum number of Spans Events that can be captured
+	// per 60-second harvest cycle
+	MaxSpanEvents = 1000
 )

v3/newrelic/app_run.go

@@ -6,6 +6,7 @@ package newrelic
 import (
 	"encoding/json"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/newrelic/go-agent/v3/internal"
@@ -35,6 +36,11 @@ type appRun struct {
 	// flexible harvest periods.  This field is created once at appRun
 	// creation.
 	harvestConfig harvestConfig
+
+	// Error code caches for faster lookups O(1)
+	ignoreErrorCodesCache map[int]bool
+	expectErrorCodesCache map[int]bool
+	mu                    sync.RWMutex
 }
 
 const (
@@ -43,27 +49,29 @@ const (
 
 func newAppRun(config config, reply *internal.ConnectReply) *appRun {
 	run := &appRun{
-		Reply:           reply,
-		AttributeConfig: createAttributeConfig(config, reply.SecurityPolicies.AttributesInclude.Enabled()),
-		Config:          config,
-		rulesCache:      newRulesCache(txnNameCacheLimit),
+		Reply:                 reply,
+		AttributeConfig:       createAttributeConfig(config, reply.SecurityPolicies.AttributesInclude.Enabled()),
+		Config:                config,
+		rulesCache:            newRulesCache(txnNameCacheLimit),
+		ignoreErrorCodesCache: make(map[int]bool),
+		expectErrorCodesCache: make(map[int]bool),
 	}
 
 	// Overwrite local settings with any server-side-config settings
 	// present. NOTE!  This requires that the Config provided to this
 	// function is a value and not a pointer: We do not want to change the
 	// input Config with values particular to this connection.
 
-	if v := run.Reply.ServerSideConfig.TransactionTracerEnabled; nil != v {
+	if v := run.Reply.ServerSideConfig.TransactionTracerEnabled; v != nil {
 		run.Config.TransactionTracer.Enabled = *v
 	}
-	if v := run.Reply.ServerSideConfig.ErrorCollectorEnabled; nil != v {
+	if v := run.Reply.ServerSideConfig.ErrorCollectorEnabled; v != nil {
 		run.Config.ErrorCollector.Enabled = *v
 	}
-	if v := run.Reply.ServerSideConfig.CrossApplicationTracerEnabled; nil != v {
+	if v := run.Reply.ServerSideConfig.CrossApplicationTracerEnabled; v != nil {
 		run.Config.CrossApplicationTracer.Enabled = *v
 	}
-	if v := run.Reply.ServerSideConfig.TransactionTracerThreshold; nil != v {
+	if v := run.Reply.ServerSideConfig.TransactionTracerThreshold; v != nil {
 		switch val := v.(type) {
 		case float64:
 			run.Config.TransactionTracer.Threshold.IsApdexFailing = false
@@ -74,12 +82,30 @@ func newAppRun(config config, reply *internal.ConnectReply) *appRun {
 			}
 		}
 	}
-	if v := run.Reply.ServerSideConfig.TransactionTracerStackTraceThreshold; nil != v {
+	if v := run.Reply.ServerSideConfig.TransactionTracerStackTraceThreshold; v != nil {
 		run.Config.TransactionTracer.Segments.StackTraceThreshold = internal.FloatSecondsToDuration(*v)
 	}
-	if v := run.Reply.ServerSideConfig.ErrorCollectorIgnoreStatusCodes; nil != v {
+	if v := run.Reply.ServerSideConfig.ErrorCollectorIgnoreStatusCodes; v != nil {
 		run.Config.ErrorCollector.IgnoreStatusCodes = v
 	}
+	if run.Config.ErrorCollector.IgnoreStatusCodes != nil {
+		run.mu.Lock()
+		for _, errorCode := range run.Config.ErrorCollector.IgnoreStatusCodes {
+			run.ignoreErrorCodesCache[errorCode] = true
+		}
+		run.mu.Unlock()
+	}
+
+	if v := run.Reply.ServerSideConfig.ErrorCollectorExpectStatusCodes; v != nil {
+		run.Config.ErrorCollector.ExpectStatusCodes = v
+	}
+	if run.Config.ErrorCollector.IgnoreStatusCodes != nil {
+		run.mu.Lock()
+		for _, errorCode := range run.Config.ErrorCollector.ExpectStatusCodes {
+			run.expectErrorCodesCache[errorCode] = true
+		}
+		run.mu.Unlock()
+	}
 
 	if !run.Reply.CollectErrorEvents {
 		run.Config.ErrorCollector.CaptureEvents = false
@@ -171,12 +197,15 @@ func (run *appRun) responseCodeIsError(code int) bool {
 	if code < 400 && code >= 100 {
 		return false
 	}
-	for _, ignoreCode := range run.Config.ErrorCollector.IgnoreStatusCodes {
-		if code == ignoreCode {
-			return false
-		}
-	}
-	return true
+	run.mu.RLock()
+	defer run.mu.RUnlock()
+	return !run.ignoreErrorCodesCache[code]
+}
+
+func (run *appRun) responseCodeIsExpected(code int) bool {
+	run.mu.RLock()
+	defer run.mu.RUnlock()
+	return run.expectErrorCodesCache[code]
 }
 
 func (run *appRun) txnTraceThreshold(apdexThreshold time.Duration) time.Duration {
@@ -219,7 +248,7 @@ func (run *appRun) LoggingConfig() (config loggingConfig) {
 // which will be the default or the user's configured size (if any), but
 // may be capped to the maximum allowed by the collector.
 func (run *appRun) MaxSpanEvents() int {
-	return run.limit(run.Config.DistributedTracer.ReservoirLimit, run.ptrSpanEvents)
+	return run.limit(internal.MaxSpanEvents, run.ptrSpanEvents)
 }
 
 func (run *appRun) limit(dflt int, field func() *uint) int {
@@ -253,11 +282,11 @@ func (run *appRun) ReportPeriods() map[harvestTypes]time.Duration {
 }
 
 func (run *appRun) createTransactionName(input string, isWeb bool) string {
-	if name := run.rulesCache.find(input, isWeb); "" != name {
+	if name := run.rulesCache.find(input, isWeb); name != "" {
 		return name
 	}
 	name := internal.CreateFullTxnName(input, run.Reply, isWeb)
-	if "" != name {
+	if name != "" {
 		// Note that we  don't cache situations where the rules say
 		// ignore.  It would increase complication (we would need to
 		// disambiguate not-found vs ignore).  Also, the ignore code

v3/newrelic/app_run_test.go

@@ -42,7 +42,41 @@ func TestResponseCodeIsError(t *testing.T) {
 				tc.Code, tc.IsError, is)
 		}
 	}
+}
+
+func TestResponseCodeIsExpected(t *testing.T) {
+	cfg := config{Config: defaultConfig()}
+	cfg.ErrorCollector.ExpectStatusCodes = []int{400, 503, 504}
+	run := newAppRun(cfg, internal.ConnectReplyDefaults())
 
+	for _, tc := range []struct {
+		Code    int
+		IsError bool
+	}{
+		{Code: 0, IsError: false}, // gRPC
+		{Code: 1, IsError: false}, // gRPC
+		{Code: 400, IsError: true},
+		{Code: 404, IsError: false},
+		{Code: 503, IsError: true},
+		{Code: 504, IsError: true},
+	} {
+		if is := run.responseCodeIsExpected(tc.Code); is != tc.IsError {
+			t.Errorf("responseCodeIsError for %d, wanted=%v got=%v",
+				tc.Code, tc.IsError, is)
+		}
+	}
+}
+
+func BenchmarkResponseCodeIsExpectedHit(b *testing.B) {
+	cfg := config{Config: defaultConfig()}
+	cfg.ErrorCollector.ExpectStatusCodes = []int{400, 503, 504}
+	run := newAppRun(cfg, internal.ConnectReplyDefaults())
+
+	b.ResetTimer()
+	b.ReportAllocs()
+	for n := 0; n < b.N; n++ {
+		run.responseCodeIsExpected(400)
+	}
 }
 
 func TestCrossAppTracingEnabled(t *testing.T) {
@@ -187,7 +221,7 @@ func TestZeroReportPeriod(t *testing.T) {
 		maxCustomEvents: internal.MaxCustomEvents,
 		maxLogEvents:    internal.MaxLogEvents,
 		maxErrorEvents:  internal.MaxErrorEvents,
-		maxSpanEvents:   defaultMaxSpanEvents,
+		maxSpanEvents:   internal.MaxSpanEvents,
 		periods: map[harvestTypes]time.Duration{
 			harvestTypesAll: 60 * time.Second,
 			0:               60 * time.Second,
@@ -331,7 +365,7 @@ func TestConfigurableTxnEvents_withCollResponse(t *testing.T) {
 	}
 	result := newAppRun(config{Config: defaultConfig()}, h).MaxTxnEvents()
 	if result != 15 {
-		t.Error(fmt.Sprintf("Unexpected max number of txn events, expected %d but got %d", 15, result))
+		t.Errorf("Unexpected max number of txn events, expected %d but got %d", 15, result)
 	}
 }
 
@@ -350,7 +384,7 @@ func TestConfigurableTxnEvents_notInCollResponse(t *testing.T) {
 	cfg.TransactionEvents.MaxSamplesStored = expected
 	result := newAppRun(cfg, reply).MaxTxnEvents()
 	if result != expected {
-		t.Error(fmt.Sprintf("Unexpected max number of txn events, expected %d but got %d", expected, result))
+		t.Errorf("Unexpected max number of txn events, expected %d but got %d", expected, result)
 	}
 }
 
@@ -368,7 +402,7 @@ func TestConfigurableTxnEvents_configMoreThanMax(t *testing.T) {
 	cfg.TransactionEvents.MaxSamplesStored = internal.MaxTxnEvents + 100
 	result := newAppRun(cfg, h).MaxTxnEvents()
 	if result != internal.MaxTxnEvents {
-		t.Error(fmt.Sprintf("Unexpected max number of txn events, expected %d but got %d", internal.MaxTxnEvents, result))
+		t.Errorf(fmt.Sprintf("Unexpected max number of txn events, expected %d but got %d", internal.MaxTxnEvents, result))
 	}
 }
 

v3/newrelic/attributes_from_internal.go

@@ -339,6 +339,13 @@ func truncateStringValueIfLong(val string) string {
 	return val
 }
 
+func truncateStringMessageIfLong(message string) string {
+	if len(message) > errorEventMessageLengthLimit {
+		return stringLengthByteLimit(message, errorEventMessageLengthLimit)
+	}
+	return message
+}
+
 // validateUserAttribute validates a user attribute.
 func validateUserAttribute(key string, val interface{}) (interface{}, error) {
 	if str, ok := val.(string); ok {