From e90e22776826cbaf59d30fdd22e14dfe4a71844f Mon Sep 17 00:00:00 2001 From: Mirac Kara Date: Thu, 2 Jun 2022 13:16:21 -0700 Subject: [PATCH 01/15] bump go version for nrechov4 test --- .github/workflows/ci.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index e74a51f5e..a32f3220b 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -124,7 +124,8 @@ jobs: dirs: v3/integrations/nrecho-v3 # Test against the latest v3 Echo: extratesting: go get -u github.com/labstack/echo@v3 - - go-version: 1.15.x + - go-version: 1.17.x + # go/new/http no longer stable under go 1.17 dirs: v3/integrations/nrecho-v4 extratesting: go get -u github.com/labstack/echo@master - go-version: 1.15.x From e48133cd8d55b6c32bbf13b8aa6f738e6e37cb06 Mon Sep 17 00:00:00 2001 From: Mirac Kara Date: Thu, 2 Jun 2022 14:02:22 -0700 Subject: [PATCH 02/15] test commit --- v3/integrations/nrecho-v4/nrecho_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/v3/integrations/nrecho-v4/nrecho_test.go b/v3/integrations/nrecho-v4/nrecho_test.go index 3e847110f..47ca8f0f2 100644 --- a/v3/integrations/nrecho-v4/nrecho_test.go +++ b/v3/integrations/nrecho-v4/nrecho_test.go @@ -20,6 +20,7 @@ func TestBasicRoute(t *testing.T) { e := echo.New() e.Use(Middleware(app.Application)) e.GET("/hello", func(c echo.Context) error { + return c.Blob(http.StatusOK, "text/html", []byte("Hello, World!")) }) From 04f6fe88344c6e2e505d361acdadd172e5cafacb Mon Sep 17 00:00:00 2001 From: Mirac Kara Date: Thu, 2 Jun 2022 14:04:00 -0700 Subject: [PATCH 03/15] Test Commit --- v3/integrations/nrecho-v4/nrecho_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/v3/integrations/nrecho-v4/nrecho_test.go b/v3/integrations/nrecho-v4/nrecho_test.go index 47ca8f0f2..a300213dc 100644 --- a/v3/integrations/nrecho-v4/nrecho_test.go +++ b/v3/integrations/nrecho-v4/nrecho_test.go @@ -18,6 +18,7 @@ func TestBasicRoute(t *testing.T) { app := integrationsupport.NewBasicTestApp() e := echo.New() + e.Use(Middleware(app.Application)) e.GET("/hello", func(c echo.Context) error { From d12eab739cd471e334022347dddcfe790b324ce5 Mon Sep 17 00:00:00 2001 From: Mirac Kara Date: Fri, 3 Jun 2022 12:59:10 -0700 Subject: [PATCH 04/15] Added go mod tidying if available in go version --- .github/workflows/ci.yaml | 2 +- build-script.sh | 7 +++++++ v3/integrations/nrecho-v4/nrecho_test.go | 9 +++------ 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index a32f3220b..b1994cc95 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -124,8 +124,8 @@ jobs: dirs: v3/integrations/nrecho-v3 # Test against the latest v3 Echo: extratesting: go get -u github.com/labstack/echo@v3 + # go/new/http no longer stable under go 1.17.x - go-version: 1.17.x - # go/new/http no longer stable under go 1.17 dirs: v3/integrations/nrecho-v4 extratesting: go get -u github.com/labstack/echo@master - go-version: 1.15.x diff --git a/build-script.sh b/build-script.sh index 364678bd5..80db9a5ab 100755 --- a/build-script.sh +++ b/build-script.sh @@ -35,6 +35,10 @@ pin_go_dependency() { fi } +go_mod_tidy() { + go mod tidy +} + IFS="," for dir in $DIRS; do cd "$pwd/$dir" @@ -43,6 +47,9 @@ for dir in $DIRS; do go mod edit -replace github.com/newrelic/go-agent/v3="$pwd"/v3 fi + # Do tidy if we can + go_mod_tidy || true + pin_go_dependency "$PIN" # avoid testing v3 code when testing v2 newrelic package diff --git a/v3/integrations/nrecho-v4/nrecho_test.go b/v3/integrations/nrecho-v4/nrecho_test.go index a300213dc..eb2eaf151 100644 --- a/v3/integrations/nrecho-v4/nrecho_test.go +++ b/v3/integrations/nrecho-v4/nrecho_test.go @@ -5,23 +5,20 @@ package nrecho import ( "errors" - "net/http" - "net/http/httptest" - "testing" - "github.com/labstack/echo/v4" "github.com/newrelic/go-agent/v3/internal" "github.com/newrelic/go-agent/v3/internal/integrationsupport" + "net/http" + "net/http/httptest" + "testing" ) func TestBasicRoute(t *testing.T) { app := integrationsupport.NewBasicTestApp() e := echo.New() - e.Use(Middleware(app.Application)) e.GET("/hello", func(c echo.Context) error { - return c.Blob(http.StatusOK, "text/html", []byte("Hello, World!")) }) From a95c5e4fb07c2270008702d14db9607931344833 Mon Sep 17 00:00:00 2001 From: Mirac Kara Date: Fri, 3 Jun 2022 13:17:14 -0700 Subject: [PATCH 05/15] Bumped v3 dependancy number --- v3/integrations/nrecho-v4/go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/v3/integrations/nrecho-v4/go.mod b/v3/integrations/nrecho-v4/go.mod index 3363a2217..0c17bd911 100644 --- a/v3/integrations/nrecho-v4/go.mod +++ b/v3/integrations/nrecho-v4/go.mod @@ -6,5 +6,5 @@ go 1.12 require ( github.com/labstack/echo/v4 v4.5.0 - github.com/newrelic/go-agent/v3 v3.15.0 + github.com/newrelic/go-agent/v3 v3.16.1 ) From f799a242fd087d5cb970773535a531c5f4ddf73b Mon Sep 17 00:00:00 2001 From: Mirac Kara Date: Mon, 6 Jun 2022 12:49:59 -0700 Subject: [PATCH 06/15] Fixed nrecho-v4 tests --- v3/integrations/nrecho-v4/go.mod | 2 + v3/integrations/nrecho-v4/nrecho_test.go | 67 +++++++++++++++++------- 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/v3/integrations/nrecho-v4/go.mod b/v3/integrations/nrecho-v4/go.mod index 0c17bd911..07f9e7fdd 100644 --- a/v3/integrations/nrecho-v4/go.mod +++ b/v3/integrations/nrecho-v4/go.mod @@ -4,6 +4,8 @@ module github.com/newrelic/go-agent/v3/integrations/nrecho-v4 // https://github.com/labstack/echo/blob/master/go.mod go 1.12 +replace github.com/newrelic/go-agent/v3 v3.16.1 => /Users/mkara/Desktop/go-agent/v3 + require ( github.com/labstack/echo/v4 v4.5.0 github.com/newrelic/go-agent/v3 v3.16.1 diff --git a/v3/integrations/nrecho-v4/nrecho_test.go b/v3/integrations/nrecho-v4/nrecho_test.go index eb2eaf151..c8b8cf074 100644 --- a/v3/integrations/nrecho-v4/nrecho_test.go +++ b/v3/integrations/nrecho-v4/nrecho_test.go @@ -33,13 +33,19 @@ func TestBasicRoute(t *testing.T) { t.Error("wrong response body", respBody) } app.ExpectTxnMetrics(t, internal.WantTxn{ - Name: "GET /hello", - IsWeb: true, + Name: "GET /hello", + IsWeb: true, + UnknownCaller: true, }) + app.ExpectTxnEvents(t, []internal.WantEvent{{ Intrinsics: map[string]interface{}{ "name": "WebTransaction/Go/GET /hello", "nr.apdexPerfZone": "S", + "sampled": false, + "guid": "*", + "traceId": "*", + "priority": "*", }, AgentAttributes: map[string]interface{}{ "httpResponseCode": "200", @@ -93,9 +99,11 @@ func TestTransactionContext(t *testing.T) { t.Error("wrong response body", respBody) } app.ExpectTxnMetrics(t, internal.WantTxn{ - Name: "GET /hello", - IsWeb: true, - NumErrors: 1, + Name: "GET /hello", + IsWeb: true, + NumErrors: 1, + UnknownCaller: true, + ErrorByCaller: true, }) } @@ -113,8 +121,9 @@ func TestNotFoundHandler(t *testing.T) { e.ServeHTTP(response, req) app.ExpectTxnMetrics(t, internal.WantTxn{ - Name: "NotFoundHandler", - IsWeb: true, + Name: "NotFoundHandler", + IsWeb: true, + UnknownCaller: true, }) } @@ -135,9 +144,11 @@ func TestMethodNotAllowedHandler(t *testing.T) { e.ServeHTTP(response, req) app.ExpectTxnMetrics(t, internal.WantTxn{ - Name: "MethodNotAllowedHandler", - IsWeb: true, - NumErrors: 1, + Name: "MethodNotAllowedHandler", + IsWeb: true, + NumErrors: 1, + UnknownCaller: true, + ErrorByCaller: true, }) } @@ -158,14 +169,20 @@ func TestReturnsHTTPError(t *testing.T) { e.ServeHTTP(response, req) app.ExpectTxnMetrics(t, internal.WantTxn{ - Name: "GET /hello", - IsWeb: true, - NumErrors: 1, + Name: "GET /hello", + IsWeb: true, + NumErrors: 1, + UnknownCaller: true, + ErrorByCaller: true, }) app.ExpectTxnEvents(t, []internal.WantEvent{{ Intrinsics: map[string]interface{}{ "name": "WebTransaction/Go/GET /hello", "nr.apdexPerfZone": "F", + "sampled": false, + "guid": "*", + "traceId": "*", + "priority": "*", }, AgentAttributes: map[string]interface{}{ "httpResponseCode": "418", @@ -194,14 +211,20 @@ func TestReturnsError(t *testing.T) { e.ServeHTTP(response, req) app.ExpectTxnMetrics(t, internal.WantTxn{ - Name: "GET /hello", - IsWeb: true, - NumErrors: 1, + Name: "GET /hello", + IsWeb: true, + NumErrors: 1, + UnknownCaller: true, + ErrorByCaller: true, }) app.ExpectTxnEvents(t, []internal.WantEvent{{ Intrinsics: map[string]interface{}{ "name": "WebTransaction/Go/GET /hello", "nr.apdexPerfZone": "F", + "sampled": false, + "guid": "*", + "traceId": "*", + "priority": "*", }, AgentAttributes: map[string]interface{}{ "httpResponseCode": "500", @@ -230,14 +253,20 @@ func TestResponseCode(t *testing.T) { e.ServeHTTP(response, req) app.ExpectTxnMetrics(t, internal.WantTxn{ - Name: "GET /hello", - IsWeb: true, - NumErrors: 1, + Name: "GET /hello", + IsWeb: true, + NumErrors: 1, + UnknownCaller: true, + ErrorByCaller: true, }) app.ExpectTxnEvents(t, []internal.WantEvent{{ Intrinsics: map[string]interface{}{ "name": "WebTransaction/Go/GET /hello", "nr.apdexPerfZone": "F", + "sampled": false, + "guid": "*", + "traceId": "*", + "priority": "*", }, AgentAttributes: map[string]interface{}{ "httpResponseCode": "418", From c24bac8338ad34df223888a72d301c00ee6b1ccc Mon Sep 17 00:00:00 2001 From: Mirac Kara Date: Mon, 6 Jun 2022 13:01:31 -0700 Subject: [PATCH 07/15] go mod file changes --- v3/integrations/nrecho-v4/go.mod | 21 ++++++++++++++++++--- v3/integrations/nrecho-v4/nrecho_test.go | 7 ++++--- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/v3/integrations/nrecho-v4/go.mod b/v3/integrations/nrecho-v4/go.mod index 07f9e7fdd..2023c8b94 100644 --- a/v3/integrations/nrecho-v4/go.mod +++ b/v3/integrations/nrecho-v4/go.mod @@ -1,12 +1,27 @@ module github.com/newrelic/go-agent/v3/integrations/nrecho-v4 -// As of Dec 2019, the echo go.mod file uses 1.12: +// As of Jun 2022, the echo go.mod file uses 1.17: // https://github.com/labstack/echo/blob/master/go.mod -go 1.12 +go 1.17 -replace github.com/newrelic/go-agent/v3 v3.16.1 => /Users/mkara/Desktop/go-agent/v3 require ( github.com/labstack/echo/v4 v4.5.0 github.com/newrelic/go-agent/v3 v3.16.1 ) + +require ( + github.com/golang/protobuf v1.4.3 // indirect + github.com/labstack/gommon v0.3.0 // indirect + github.com/mattn/go-colorable v0.1.8 // indirect + github.com/mattn/go-isatty v0.0.12 // indirect + github.com/valyala/bytebufferpool v1.0.0 // indirect + github.com/valyala/fasttemplate v1.2.1 // indirect + golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2 // indirect + golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 // indirect + golang.org/x/sys v0.0.0-20210403161142-5e06dd20ab57 // indirect + golang.org/x/text v0.3.6 // 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 +) diff --git a/v3/integrations/nrecho-v4/nrecho_test.go b/v3/integrations/nrecho-v4/nrecho_test.go index c8b8cf074..0093d2360 100644 --- a/v3/integrations/nrecho-v4/nrecho_test.go +++ b/v3/integrations/nrecho-v4/nrecho_test.go @@ -43,9 +43,10 @@ func TestBasicRoute(t *testing.T) { "name": "WebTransaction/Go/GET /hello", "nr.apdexPerfZone": "S", "sampled": false, - "guid": "*", - "traceId": "*", - "priority": "*", + // Note: "*" is a wildcard value + "guid": "*", + "traceId": "*", + "priority": "*", }, AgentAttributes: map[string]interface{}{ "httpResponseCode": "200", From 908cb1af7cf04437daf7205e1a2340247c2afbfb Mon Sep 17 00:00:00 2001 From: Mirac Kara Date: Wed, 8 Jun 2022 12:11:13 -0700 Subject: [PATCH 08/15] Added changes to WantTxn to support distributed tracing testing --- v3/internal/expect.go | 8 +++++--- v3/newrelic/expect_implementation.go | 19 ++++++++++++++++++- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/v3/internal/expect.go b/v3/internal/expect.go index d06367ffa..3dff6e475 100644 --- a/v3/internal/expect.go +++ b/v3/internal/expect.go @@ -101,9 +101,11 @@ func HarvestTesting(app interface{}, replyfn func(*ConnectReply)) { // WantTxn provides the expectation parameters to ExpectTxnMetrics. type WantTxn struct { - Name string - IsWeb bool - NumErrors int + Name string + IsWeb bool + NumErrors int + UnknownCaller bool + ErrorByCaller bool } // Expect exposes methods that allow for testing whether the correct data was diff --git a/v3/newrelic/expect_implementation.go b/v3/newrelic/expect_implementation.go index 82a9607b1..5edcc3033 100644 --- a/v3/newrelic/expect_implementation.go +++ b/v3/newrelic/expect_implementation.go @@ -56,6 +56,23 @@ func expectTxnMetrics(t internal.Validator, mt *metricTable, want internal.WantT {Name: "Apdex", Scope: "", Forced: true, Data: nil}, {Name: "Apdex/Go/" + want.Name, Scope: "", Forced: false, Data: nil}, } + if want.UnknownCaller { + metrics = append(metrics, + internal.WantMetric{Name: "DurationByCaller/Unknown/Unknown/Unknown/Unknown/all", Scope: "", Forced: false, Data: nil}, + ) + metrics = append(metrics, + internal.WantMetric{Name: "DurationByCaller/Unknown/Unknown/Unknown/Unknown/allWeb", Scope: "", Forced: false, Data: nil}, + ) + } + if want.ErrorByCaller { + metrics = append(metrics, + internal.WantMetric{Name: "ErrorsByCaller/Unknown/Unknown/Unknown/Unknown/allWeb", Scope: "", Forced: false, Data: nil}, + ) + metrics = append(metrics, + internal.WantMetric{Name: "ErrorsByCaller/Unknown/Unknown/Unknown/Unknown/all", Scope: "", Forced: false, Data: nil}, + ) + } + } else { scope = "OtherTransaction/Go/" + want.Name allWebOther = "allOther" @@ -148,7 +165,7 @@ func expectAttributes(v internal.Validator, exists map[string]interface{}, expec v.Error("expected attribute not found: ", key) continue } - if val == internal.MatchAnything { + if val == internal.MatchAnything || val == "*" { continue } v1 := fmt.Sprint(found) From effaaa88f4d075887fae5a7ceccb9e797ee82328 Mon Sep 17 00:00:00 2001 From: Mirac Kara Date: Wed, 8 Jun 2022 12:28:23 -0700 Subject: [PATCH 09/15] Specified echo version --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index b1994cc95..3afc46ec2 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -127,7 +127,7 @@ jobs: # go/new/http no longer stable under go 1.17.x - go-version: 1.17.x dirs: v3/integrations/nrecho-v4 - extratesting: go get -u github.com/labstack/echo@master + extratesting: go get -u github.com/labstack/echo/v4@master - go-version: 1.15.x dirs: v3/integrations/nrelasticsearch-v7 extratesting: go get -u github.com/elastic/go-elasticsearch/v7@7.x From 2bd9ccb8d8282f2830975ad3823cfd769e81b229 Mon Sep 17 00:00:00 2001 From: Steve Willoughby Date: Sun, 26 Jun 2022 18:13:24 -0700 Subject: [PATCH 10/15] changed DT header JSON marshalling to be faster --- v3/go.mod | 1 + v3/internal/jsonx/encode.go | 15 ++++++++ v3/newrelic/distributed_tracing.go | 57 +++++++++++++++++++++++++----- v3/newrelic/json_object_writer.go | 5 +++ 4 files changed, 70 insertions(+), 8 deletions(-) diff --git a/v3/go.mod b/v3/go.mod index 92bc2eef5..94744cbac 100644 --- a/v3/go.mod +++ b/v3/go.mod @@ -4,5 +4,6 @@ go 1.7 require ( github.com/golang/protobuf v1.4.3 + github.com/pkg/profile v1.6.0 // indirect google.golang.org/grpc v1.39.0 ) diff --git a/v3/internal/jsonx/encode.go b/v3/internal/jsonx/encode.go index 6495829f7..b72558801 100644 --- a/v3/internal/jsonx/encode.go +++ b/v3/internal/jsonx/encode.go @@ -118,6 +118,21 @@ func AppendFloat(buf *bytes.Buffer, x float64) error { return nil } +func AppendFloat32(buf *bytes.Buffer, x float32) error { + var scratch [64]byte + x64 := float64(x) + + if math.IsInf(x64, 0) || math.IsNaN(x64) { + return &json.UnsupportedValueError{ + Value: reflect.ValueOf(x64), + Str: strconv.FormatFloat(x64, 'g', -1, 32), + } + } + + buf.Write(strconv.AppendFloat(scratch[:0], x64, 'g', -1, 32)) + return nil +} + // AppendFloatArray appends an array of numeric literals to buf. func AppendFloatArray(buf *bytes.Buffer, a ...float64) error { buf.WriteByte('[') diff --git a/v3/newrelic/distributed_tracing.go b/v3/newrelic/distributed_tracing.go index 42e6ddb13..f5e59b3e5 100644 --- a/v3/newrelic/distributed_tracing.go +++ b/v3/newrelic/distributed_tracing.go @@ -4,6 +4,7 @@ package newrelic import ( + "bytes" "encoding/base64" "encoding/json" "errors" @@ -15,6 +16,7 @@ import ( "time" "github.com/newrelic/go-agent/v3/internal" + "github.com/newrelic/go-agent/v3/internal/jsonx" ) type distTraceVersion [2]int @@ -22,6 +24,10 @@ type distTraceVersion [2]int func (v distTraceVersion) major() int { return v[0] } func (v distTraceVersion) minor() int { return v[1] } +func (v distTraceVersion) WriteJSON(buf *bytes.Buffer) { + jsonx.AppendIntArray(buf, int64(v[0]), int64(v[1])) +} + const ( // callerTypeApp is the Type field's value for outbound payloads. callerTypeApp = "App" @@ -57,6 +63,10 @@ func (tm timestampMillis) MarshalJSON() ([]byte, error) { return json.Marshal(timeToUnixMilliseconds(tm.Time())) } +func (tm timestampMillis) WriteJSON(buf *bytes.Buffer) { + jsonx.AppendUint(buf, timeToUnixMilliseconds(tm.Time())) +} + func (tm timestampMillis) Time() time.Time { return time.Time(tm) } func (tm *timestampMillis) Set(t time.Time) { *tm = timestampMillis(t) } @@ -86,6 +96,34 @@ type payload struct { OriginalTraceState string `json:"-"` } +func (p payload) WriteJSON(buf *bytes.Buffer) { + buf.WriteByte('{') + w := jsonFieldsWriter{buf: buf} + w.stringField("ty", p.Type) + w.stringField("ap", p.App) + w.stringField("ac", p.Account) + if p.TransactionID != "" { + w.stringField("tx", p.TransactionID) + } + if p.ID != "" { + w.stringField("id", p.ID) + } + w.stringField("tr", p.TracedID) + w.float32Field("pr", float32(p.Priority)) + + if p.Sampled == nil { + w.addKey("sa") + w.buf.WriteString("null") + } else { + w.boolField("sa", *p.Sampled) + } + w.writerField("ti", p.Timestamp) + if p.TrustedAccountKey != "" { + w.stringField("tk", p.TrustedAccountKey) + } + buf.WriteByte('}') +} + type payloadCaller struct { TransportType string Type string @@ -142,14 +180,17 @@ func (p payload) text(v distTraceVersion) []byte { if p.TrustedAccountKey == p.Account { p.TrustedAccountKey = "" } - js, _ := json.Marshal(struct { - Version distTraceVersion `json:"v"` - Data payload `json:"d"` - }{ - Version: v, - Data: p, - }) - return js + + var js bytes.Buffer + w := jsonFieldsWriter{ + buf: &js, + } + js.WriteByte('{') + w.writerField("v", v) + w.writerField("d", p) + js.WriteByte('}') + + return js.Bytes() } // NRText implements newrelic.DistributedTracePayload. diff --git a/v3/newrelic/json_object_writer.go b/v3/newrelic/json_object_writer.go index 3472c83d6..9bae24b88 100644 --- a/v3/newrelic/json_object_writer.go +++ b/v3/newrelic/json_object_writer.go @@ -44,6 +44,11 @@ func (w *jsonFieldsWriter) floatField(key string, val float64) { jsonx.AppendFloat(w.buf, val) } +func (w *jsonFieldsWriter) float32Field(key string, val float32) { + w.addKey(key) + jsonx.AppendFloat32(w.buf, val) +} + func (w *jsonFieldsWriter) boolField(key string, val bool) { w.addKey(key) if val { From e59734ce2d02714688b6cf6da3fe090d7297a7ee Mon Sep 17 00:00:00 2001 From: Mirac Kara Date: Mon, 27 Jun 2022 13:54:16 -0500 Subject: [PATCH 11/15] Changed naming conventions --- v3/integrations/nrgrpc/nrgrpc_server.go | 18 +++++++-------- v3/integrations/nrgrpc/nrgrpc_server_test.go | 24 ++++++++++---------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/v3/integrations/nrgrpc/nrgrpc_server.go b/v3/integrations/nrgrpc/nrgrpc_server.go index fe5954cb2..6b28c84f0 100644 --- a/v3/integrations/nrgrpc/nrgrpc_server.go +++ b/v3/integrations/nrgrpc/nrgrpc_server.go @@ -198,9 +198,9 @@ func ErrorInterceptorStatusHandler(ctx context.Context, txn *newrelic.Transactio Message: s.Message(), Class: "gRPC Status: " + s.Code().String(), }) - txn.AddAttribute("GrpcStatusLevel", "error") - txn.AddAttribute("GrpcStatusMessage", s.Message()) - txn.AddAttribute("GrpcStatusCode", s.Code().String()) + txn.AddAttribute("grpcStatusLevel", "error") + txn.AddAttribute("grpcStatusMessage", s.Message()) + txn.AddAttribute("grpcStatusCode", s.Code().String()) } // @@ -212,9 +212,9 @@ func ErrorInterceptorStatusHandler(ctx context.Context, txn *newrelic.Transactio // func WarningInterceptorStatusHandler(ctx context.Context, txn *newrelic.Transaction, s *status.Status) { txn.SetWebResponse(nil).WriteHeader(int(codes.OK)) - txn.AddAttribute("GrpcStatusLevel", "warning") - txn.AddAttribute("GrpcStatusMessage", s.Message()) - txn.AddAttribute("GrpcStatusCode", s.Code().String()) + txn.AddAttribute("grpcStatusLevel", "warning") + txn.AddAttribute("grpcStatusMessage", s.Message()) + txn.AddAttribute("grpcStatusCode", s.Code().String()) } // @@ -226,9 +226,9 @@ func WarningInterceptorStatusHandler(ctx context.Context, txn *newrelic.Transact // func InfoInterceptorStatusHandler(ctx context.Context, txn *newrelic.Transaction, s *status.Status) { txn.SetWebResponse(nil).WriteHeader(int(codes.OK)) - txn.AddAttribute("GrpcStatusLevel", "info") - txn.AddAttribute("GrpcStatusMessage", s.Message()) - txn.AddAttribute("GrpcStatusCode", s.Code().String()) + txn.AddAttribute("grpcStatusLevel", "info") + txn.AddAttribute("grpcStatusMessage", s.Message()) + txn.AddAttribute("grpcStatusCode", s.Code().String()) } // diff --git a/v3/integrations/nrgrpc/nrgrpc_server_test.go b/v3/integrations/nrgrpc/nrgrpc_server_test.go index f50b16a5e..103379b61 100644 --- a/v3/integrations/nrgrpc/nrgrpc_server_test.go +++ b/v3/integrations/nrgrpc/nrgrpc_server_test.go @@ -182,9 +182,9 @@ func TestUnaryServerInterceptorError(t *testing.T) { "traceId": internal.MatchAnything, }, UserAttributes: map[string]interface{}{ - "GrpcStatusMessage": "oooooops!", - "GrpcStatusCode": "DataLoss", - "GrpcStatusLevel": "error", + "grpcStatusMessage": "oooooops!", + "grpcStatusCode": "DataLoss", + "grpcStatusLevel": "error", }, AgentAttributes: map[string]interface{}{ "httpResponseCode": 0, @@ -215,9 +215,9 @@ func TestUnaryServerInterceptorError(t *testing.T) { "request.uri": "grpc://bufnet/TestApplication/DoUnaryUnaryError", }, UserAttributes: map[string]interface{}{ - "GrpcStatusMessage": "oooooops!", - "GrpcStatusCode": "DataLoss", - "GrpcStatusLevel": "error", + "grpcStatusMessage": "oooooops!", + "grpcStatusCode": "DataLoss", + "grpcStatusLevel": "error", }, }}) } @@ -592,9 +592,9 @@ func TestStreamServerInterceptorError(t *testing.T) { "traceId": internal.MatchAnything, }, UserAttributes: map[string]interface{}{ - "GrpcStatusLevel": "error", - "GrpcStatusMessage": "oooooops!", - "GrpcStatusCode": "DataLoss", + "grpcStatusLevel": "error", + "grpcStatusMessage": "oooooops!", + "grpcStatusCode": "DataLoss", }, AgentAttributes: map[string]interface{}{ "httpResponseCode": 0, @@ -625,9 +625,9 @@ func TestStreamServerInterceptorError(t *testing.T) { "request.uri": "grpc://bufnet/TestApplication/DoUnaryStreamError", }, UserAttributes: map[string]interface{}{ - "GrpcStatusLevel": "error", - "GrpcStatusMessage": "oooooops!", - "GrpcStatusCode": "DataLoss", + "grpcStatusLevel": "error", + "grpcStatusMessage": "oooooops!", + "grpcStatusCode": "DataLoss", }, }}) } From fd44db5bfaba61f32d5225e65311250e5e29f64b Mon Sep 17 00:00:00 2001 From: Mirac Kara Date: Mon, 27 Jun 2022 13:57:32 -0500 Subject: [PATCH 12/15] Revert "Added Max Samples per Custom Events Setting" This reverts commit 038517373aae20aa409e0aa37d10f122807d4d8f. --- internal/harvest.go | 3 --- v3/internal/connect_reply.go | 8 ++++---- v3/newrelic/app_run.go | 8 ++++---- v3/newrelic/config.go | 16 ++-------------- v3/newrelic/config_options.go | 8 -------- v3/newrelic/config_test.go | 5 +---- 6 files changed, 11 insertions(+), 37 deletions(-) diff --git a/internal/harvest.go b/internal/harvest.go index e9618f4b8..bfdb3b104 100644 --- a/internal/harvest.go +++ b/internal/harvest.go @@ -181,11 +181,8 @@ type HarvestConfigurer interface { MaxSpanEvents() int // MaxCustomEvents returns the maximum number of Custom Events that should be reported per period MaxCustomEvents() int - // MaxSamplesStored returns the maximum number of Custom Event Samples stored in a Custom Event - MaxSamplesStored() int // MaxErrorEvents returns the maximum number of Error Events that should be reported per period MaxErrorEvents() int - MaxTxnEventer } diff --git a/v3/internal/connect_reply.go b/v3/internal/connect_reply.go index 820518350..84b5d00b6 100644 --- a/v3/internal/connect_reply.go +++ b/v3/internal/connect_reply.go @@ -136,19 +136,19 @@ func (r *ConnectReply) ConfigurablePeriod() time.Duration { func uintPtr(x uint) *uint { return &x } // DefaultEventHarvestConfig provides faster event harvest defaults. -func DefaultEventHarvestConfig(maxTxnEvents int, maxCustomEvents int) EventHarvestConfig { +func DefaultEventHarvestConfig(maxTxnEvents int) EventHarvestConfig { cfg := EventHarvestConfig{} cfg.ReportPeriodMs = DefaultConfigurableEventHarvestMs cfg.Limits.TxnEvents = uintPtr(uint(maxTxnEvents)) - cfg.Limits.CustomEvents = uintPtr(uint(maxCustomEvents)) + cfg.Limits.CustomEvents = uintPtr(uint(MaxCustomEvents)) cfg.Limits.ErrorEvents = uintPtr(uint(MaxErrorEvents)) return cfg } // DefaultEventHarvestConfigWithDT is an extended version of DefaultEventHarvestConfig, // with the addition that it takes into account distributed tracer span event harvest limits. -func DefaultEventHarvestConfigWithDT(maxTxnEvents int, dtEnabled bool, spanEventLimit int, maxCustomEvents int) EventHarvestConfig { - cfg := DefaultEventHarvestConfig(maxTxnEvents,maxCustomEvents) +func DefaultEventHarvestConfigWithDT(maxTxnEvents int, dtEnabled bool, spanEventLimit int) EventHarvestConfig { + cfg := DefaultEventHarvestConfig(maxTxnEvents) if dtEnabled { cfg.Limits.SpanEvents = uintPtr(uint(spanEventLimit)) } diff --git a/v3/newrelic/app_run.go b/v3/newrelic/app_run.go index c58208e4f..3df42ea55 100644 --- a/v3/newrelic/app_run.go +++ b/v3/newrelic/app_run.go @@ -7,6 +7,7 @@ import ( "encoding/json" "strings" "time" + "github.com/newrelic/go-agent/v3/internal" ) @@ -190,7 +191,9 @@ func (run *appRun) ptrErrorEvents() *uint { return run.Reply.EventData.Limits.E func (run *appRun) ptrSpanEvents() *uint { return run.Reply.EventData.Limits.SpanEvents } func (run *appRun) MaxTxnEvents() int { return run.limit(run.Config.maxTxnEvents(), run.ptrTxnEvents) } - +func (run *appRun) MaxCustomEvents() int { + return run.limit(internal.MaxCustomEvents, run.ptrCustomEvents) +} func (run *appRun) MaxErrorEvents() int { return run.limit(internal.MaxErrorEvents, run.ptrErrorEvents) } @@ -201,9 +204,6 @@ func (run *appRun) MaxErrorEvents() int { func (run *appRun) MaxSpanEvents() int { return run.limit(run.Config.DistributedTracer.ReservoirLimit, run.ptrSpanEvents) } -func (run *appRun) MaxCustomEvents() int { - return run.limit(run.Config.maxCustomEvents(), run.ptrCustomEvents) -} func (run *appRun) limit(dflt int, field func() *uint) int { if field() != nil { diff --git a/v3/newrelic/config.go b/v3/newrelic/config.go index 9c7c9f49a..fe0f7c7f7 100644 --- a/v3/newrelic/config.go +++ b/v3/newrelic/config.go @@ -71,9 +71,8 @@ type Config struct { // custom analytics events. High security mode overrides this // setting. Enabled bool - // MaxSamplesStored sets the desired maximum custom events stored - MaxSamplesStored int } + // TransactionEvents controls the behavior of transaction analytics // events. TransactionEvents struct { @@ -383,7 +382,6 @@ func defaultConfig() Config { c.Enabled = true c.Labels = make(map[string]string) c.CustomInsightsEvents.Enabled = true - c.CustomInsightsEvents.MaxSamplesStored = internal.MaxCustomEvents c.TransactionEvents.Enabled = true c.TransactionEvents.Attributes.Enabled = true c.TransactionEvents.MaxSamplesStored = internal.MaxTxnEvents @@ -505,16 +503,6 @@ func (c Config) validateTraceObserverConfig() (*observerURL, error) { }, nil } -// maxCustomEvents returns the configured maximum number of Custom Events if it has been configured -// and is less than the default maximum; otherwise it returns the default max. -func (c Config) maxCustomEvents() int { - configured := c.CustomInsightsEvents.MaxSamplesStored - if configured < 0 || configured > internal.MaxCustomEvents { - return internal.MaxTxnEvents - } - return configured -} - // maxTxnEvents returns the configured maximum number of Transaction Events if it has been configured // and is less than the default maximum; otherwise it returns the default max. func (c Config) maxTxnEvents() int { @@ -679,7 +667,7 @@ func configConnectJSONInternal(c Config, pid int, util *utilization.Data, e envi Util: util, SecurityPolicies: securityPolicies, Metadata: metadata, - EventData: internal.DefaultEventHarvestConfigWithDT(c.maxTxnEvents(), c.DistributedTracer.Enabled, c.DistributedTracer.ReservoirLimit, c.maxCustomEvents()), + EventData: internal.DefaultEventHarvestConfigWithDT(c.maxTxnEvents(), c.DistributedTracer.Enabled, c.DistributedTracer.ReservoirLimit), }}) } diff --git a/v3/newrelic/config_options.go b/v3/newrelic/config_options.go index 2ce99c08c..3ba323924 100644 --- a/v3/newrelic/config_options.go +++ b/v3/newrelic/config_options.go @@ -36,14 +36,6 @@ func ConfigDistributedTracerEnabled(enabled bool) ConfigOption { return func(cfg *Config) { cfg.DistributedTracer.Enabled = enabled } } -// ConfigCustomInsightsEventsMaxSamplesLimit alters the sample size allowing control -// of how many custom events are stored in an agent for a given harvest cycle. -// Alters the CustomInsightsEvents.MaxSamplesStored setting. -func ConfigCustomInsightsEventsMaxSamplesStored(limit int) ConfigOption { - return func(cfg *Config) { cfg.CustomInsightsEvents.MaxSamplesStored = limit} -} - - // ConfigDistributedTracerReservoirLimit alters the sample reservoir size (maximum // number of span events to be collected) for distributed tracing instead of // using the built-in default. diff --git a/v3/newrelic/config_test.go b/v3/newrelic/config_test.go index c83972a48..b6df1731b 100644 --- a/v3/newrelic/config_test.go +++ b/v3/newrelic/config_test.go @@ -135,10 +135,7 @@ func TestCopyConfigReferenceFieldsPresent(t *testing.T) { "Enabled":true }, "CrossApplicationTracer":{"Enabled":false}, - "CustomInsightsEvents":{ - "Enabled":true, - "MaxSamplesStored":1000 - }, + "CustomInsightsEvents":{"Enabled":true}, "DatastoreTracer":{ "DatabaseNameReporting":{"Enabled":true}, "InstanceReporting":{"Enabled":true}, From e5543cb48c341015c94f7332effe166ac72b20bf Mon Sep 17 00:00:00 2001 From: Steve Willoughby Date: Mon, 27 Jun 2022 17:02:19 -0700 Subject: [PATCH 13/15] changed buffer alloc --- v3/newrelic/distributed_tracing.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/v3/newrelic/distributed_tracing.go b/v3/newrelic/distributed_tracing.go index f5e59b3e5..eb43f2c00 100644 --- a/v3/newrelic/distributed_tracing.go +++ b/v3/newrelic/distributed_tracing.go @@ -181,9 +181,9 @@ func (p payload) text(v distTraceVersion) []byte { p.TrustedAccountKey = "" } - var js bytes.Buffer + js := bytes.NewBuffer(make([]byte, 0, 256)) w := jsonFieldsWriter{ - buf: &js, + buf: js, } js.WriteByte('{') w.writerField("v", v) From 64ec42259a2ba484f6c40da3ef56190b87eead8f Mon Sep 17 00:00:00 2001 From: Steve Willoughby Date: Tue, 28 Jun 2022 09:19:22 -0700 Subject: [PATCH 14/15] code cleanup to conform to dev standards and style guide --- v3/go.mod | 1 - v3/newrelic/distributed_tracing.go | 37 +++++++++++++++++++----------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/v3/go.mod b/v3/go.mod index 94744cbac..92bc2eef5 100644 --- a/v3/go.mod +++ b/v3/go.mod @@ -4,6 +4,5 @@ go 1.7 require ( github.com/golang/protobuf v1.4.3 - github.com/pkg/profile v1.6.0 // indirect google.golang.org/grpc v1.39.0 ) diff --git a/v3/newrelic/distributed_tracing.go b/v3/newrelic/distributed_tracing.go index eb43f2c00..e0c9392d3 100644 --- a/v3/newrelic/distributed_tracing.go +++ b/v3/newrelic/distributed_tracing.go @@ -24,6 +24,9 @@ type distTraceVersion [2]int func (v distTraceVersion) major() int { return v[0] } func (v distTraceVersion) minor() int { return v[1] } +// WriteJSON implements the functionality to support writerField +// in our internal json builder. It appends the JSON representation +// of a distTraceVersion to the destination bytes.Buffer. func (v distTraceVersion) WriteJSON(buf *bytes.Buffer) { jsonx.AppendIntArray(buf, int64(v[0]), int64(v[1])) } @@ -63,6 +66,9 @@ func (tm timestampMillis) MarshalJSON() ([]byte, error) { return json.Marshal(timeToUnixMilliseconds(tm.Time())) } +// WriteJSON implements the functionality to support writerField +// in our internal json builder. It appends the JSON representation +// of a timestampMillis value to the destination bytes.Buffer. func (tm timestampMillis) WriteJSON(buf *bytes.Buffer) { jsonx.AppendUint(buf, timeToUnixMilliseconds(tm.Time())) } @@ -96,6 +102,9 @@ type payload struct { OriginalTraceState string `json:"-"` } +// WriteJSON implements the functionality to support writerField +// in our internal json builder. It appends the JSON representation +// of a payload struct to the destination bytes.Buffer. func (p payload) WriteJSON(buf *bytes.Buffer) { buf.WriteByte('{') w := jsonFieldsWriter{buf: buf} @@ -147,27 +156,27 @@ func (p payload) validateNewRelicData() error { // If a payload is missing both `guid` and `transactionId` is received, // a ParseException supportability metric should be generated. - if "" == p.TransactionID && "" == p.ID { + if p.TransactionID == "" && p.ID == "" { return errPayloadMissingGUIDTxnID } - if "" == p.Type { + if p.Type == "" { return errPayloadMissingType } - if "" == p.Account { + if p.Account == "" { return errPayloadMissingAccount } - if "" == p.App { + if p.App == "" { return errPayloadMissingApp } - if "" == p.TracedID { + if p.TracedID == "" { return errPayloadMissingTraceID } - if p.Timestamp.Time().IsZero() || 0 == p.Timestamp.Time().Unix() { + if p.Timestamp.Time().IsZero() || p.Timestamp.Time().Unix() == 0 { return errPayloadMissingTimestamp } @@ -300,12 +309,12 @@ func processNRDTString(str string, support *distributedTracingSupport) (*payload return nil, nil } var decoded []byte - if '{' == str[0] { + if str[0] == '{' { decoded = []byte(str) } else { var err error decoded, err = base64.StdEncoding.DecodeString(str) - if nil != err { + if err != nil { support.AcceptPayloadParseException = true return nil, fmt.Errorf("unable to decode payload: %v", err) } @@ -314,12 +323,12 @@ func processNRDTString(str string, support *distributedTracingSupport) (*payload Version distTraceVersion `json:"v"` Data json.RawMessage `json:"d"` }{} - if err := json.Unmarshal(decoded, &envelope); nil != err { + if err := json.Unmarshal(decoded, &envelope); err != nil { support.AcceptPayloadParseException = true return nil, fmt.Errorf("unable to unmarshal payload: %v", err) } - if 0 == envelope.Version.major() && 0 == envelope.Version.minor() { + if envelope.Version.major() == 0 && envelope.Version.minor() == 0 { support.AcceptPayloadParseException = true return nil, errPayloadMissingVersion } @@ -330,7 +339,7 @@ func processNRDTString(str string, support *distributedTracingSupport) (*payload envelope.Version.major()) } payload := new(payload) - if err := json.Unmarshal(envelope.Data, payload); nil != err { + if err := json.Unmarshal(envelope.Data, payload); err != nil { support.AcceptPayloadParseException = true return nil, fmt.Errorf("unable to unmarshal payload data: %v", err) } @@ -346,12 +355,12 @@ func processNRDTString(str string, support *distributedTracingSupport) (*payload func processW3CHeaders(hdrs http.Header, trustedAccountKey string, support *distributedTracingSupport) (*payload, error) { p, err := processTraceParent(hdrs) - if nil != err { + if err != nil { support.TraceContextParentParseException = true return nil, err } err = processTraceState(hdrs, trustedAccountKey, p) - if nil != err { + if err != nil { if err == errInvalidNRTraceState { support.TraceContextStateInvalidNrEntry = true } else { @@ -435,7 +444,7 @@ func processTraceState(hdrs http.Header, trustedAccountKey string, p *payload) e app := matches[3] timestamp, err := strconv.ParseUint(matches[8], 10, 64) - if nil != err || "" == version || "" == parentType || "" == account || "" == app { + if err != nil || version == "" || parentType == "" || account == "" || app == "" { return errInvalidNRTraceState } From c9cc4a1d7b98eccbb84856174f6fb6796b9541e7 Mon Sep 17 00:00:00 2001 From: Steve Willoughby Date: Tue, 28 Jun 2022 10:09:45 -0700 Subject: [PATCH 15/15] moved literal to constant value --- v3/newrelic/distributed_tracing.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/v3/newrelic/distributed_tracing.go b/v3/newrelic/distributed_tracing.go index e0c9392d3..9f04e09b8 100644 --- a/v3/newrelic/distributed_tracing.go +++ b/v3/newrelic/distributed_tracing.go @@ -183,6 +183,8 @@ func (p payload) validateNewRelicData() error { return nil } +const payloadJSONStartingSizeEstimate = 256 + func (p payload) text(v distTraceVersion) []byte { // TrustedAccountKey should only be attached to the outbound payload if its value differs // from the Account field. @@ -190,7 +192,7 @@ func (p payload) text(v distTraceVersion) []byte { p.TrustedAccountKey = "" } - js := bytes.NewBuffer(make([]byte, 0, 256)) + js := bytes.NewBuffer(make([]byte, 0, payloadJSONStartingSizeEstimate)) w := jsonFieldsWriter{ buf: js, }