Repository: newrelic/newrelic-client-go
Pull request #322 (2020-05-22 23:09:16)

HTTP Compression / Prep for Insights
This PR accomplishes two goals:
* Add Insights URLs / building to Region
* Implement compression (none and gzip) in the http client, so sending data is optimized

The compression is not directly inline by default, however there is on spicy refactor: Removal of RawPost, and collapsing that logic into the [request](https://github.com/newrelic/newrelic-client-go/pull/322/files#diff-c8120cadd719e78b5424e31344a1b4e0R51).  In this case, anything that is already a `[]byte` is not marshaled to JSON, anything else is.  The one RawPost use-case has been updated (wrap: `[]byte("string:notJSON")`)


Patch:
diff --git a/internal/http/auth.go b/internal/http/auth.go
index b761078..dcaa1c0 100644
--- a/internal/http/auth.go
+++ b/internal/http/auth.go
@@ -37,3 +37,10 @@ type ClassicV2Authorizer struct{}
 func (a *ClassicV2Authorizer) AuthorizeRequest(r *Request, c *config.Config) {
 	r.SetHeader("X-Api-Key", c.AdminAPIKey)
 }
+
+// InsightsInsertKeyAuthorizer authorizes sending custom events to New Relic.
+type InsightsInsertKeyAuthorizer struct{}
+
+func (a *InsightsInsertKeyAuthorizer) AuthorizeRequest(r *Request, c *config.Config) {
+	r.SetHeader("X-Insert-Key", c.InsightsInsertKey)
+}
diff --git a/internal/http/client.go b/internal/http/client.go
index 36327ee..949436b 100644
--- a/internal/http/client.go
+++ b/internal/http/client.go
@@ -1,7 +1,6 @@
 package http
 
 import (
-	"bytes"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -38,6 +37,9 @@ type Client struct {
 	// authStrategy allows us to use multiple authentication methods for API calls
 	authStrategy RequestAuthorizer
 
+	// compressor is used to compress the body of a request, and set the content-encoding header
+	compressor RequestCompressor
+
 	errorValue ErrorResponse
 }
 
@@ -78,12 +80,21 @@ func NewClient(cfg config.Config) Client {
 	// Disable logging in go-retryablehttp since we are logging requests directly here
 	r.Logger = nil
 
-	return Client{
+	client := Client{
 		authStrategy: &ClassicV2Authorizer{},
 		client:       r,
 		config:       cfg,
 		errorValue:   &DefaultErrorResponse{},
 	}
+
+	switch cfg.Compression {
+	case config.Compression.Gzip:
+		client.compressor = &GzipCompressor{}
+	default:
+		client.compressor = &NoneCompressor{}
+	}
+
+	return client
 }
 
 // SetAuthStrategy is used to set the default auth strategy for this client
@@ -92,6 +103,12 @@ func (c *Client) SetAuthStrategy(da RequestAuthorizer) {
 	c.authStrategy = da
 }
 
+// SetRequestCompressor is used to enable compression on the request using
+// the RequestCompressor specified
+func (c *Client) SetRequestCompressor(compressor RequestCompressor) {
+	c.compressor = compressor
+}
+
 // SetErrorValue is used to unmarshal error body responses in JSON format.
 func (c *Client) SetErrorValue(v ErrorResponse) *Client {
 	c.errorValue = v
@@ -134,34 +151,6 @@ func (c *Client) Post(
 	return c.Do(req)
 }
 
-// RawPost behaves the same as Post, but without marshaling the body into JSON before making the request.
-// This is required at least in the case of Synthetics Labels, since the POST doesn't handle JSON.
-func (c *Client) RawPost(
-	url string,
-	queryParams interface{},
-	reqBody interface{},
-	respBody interface{},
-) (*http.Response, error) {
-
-	var requestBody []byte
-
-	switch val := reqBody.(type) {
-	case []byte:
-		requestBody = val
-	case string:
-		requestBody = []byte(val)
-	default:
-		return nil, errors.New("invalid request body")
-	}
-
-	req, err := c.NewRequest(http.MethodPost, url, queryParams, requestBody, respBody)
-	if err != nil {
-		return nil, err
-	}
-
-	return c.Do(req)
-}
-
 // Put represents an HTTP PUT request to a New Relic API.
 // The queryParams argument can be used to add query string parameters to the requested URL.
 // The reqBody argument will be marshaled to JSON from the type provided and included in the request body.
@@ -271,21 +260,6 @@ func isResponseSuccess(resp *http.Response) bool {
 	return statusCode >= http.StatusOK && statusCode <= 299
 }
 
-func makeRequestBodyReader(reqBody interface{}) (*bytes.Buffer, error) {
-	if reqBody == nil {
-		return nil, nil
-	}
-
-	j, err := json.Marshal(reqBody)
-	if err != nil {
-		return nil, err
-	}
-
-	b := bytes.NewBuffer(j)
-
-	return b, nil
-}
-
 // NerdGraphQuery runs a Nerdgraph query.
 func (c *Client) NerdGraphQuery(query string, vars map[string]interface{}, respBody interface{}) error {
 	req, err := c.NewNerdGraphRequest(query, vars, respBody)
diff --git a/internal/http/compress.go b/internal/http/compress.go
new file mode 100644
index 0000000..145a804
--- /dev/null
+++ b/internal/http/compress.go
@@ -0,0 +1,62 @@
+package http
+
+import (
+	"bufio"
+	"bytes"
+	"compress/gzip"
+	"io"
+)
+
+const (
+	// CompressorMinimumSize is the required size in bytes that a body exceeds before it is compressed
+	CompressorMinimumSize = 150
+)
+
+type RequestCompressor interface {
+	Compress(r *Request, body []byte) (io.Reader, error)
+}
+
+// NoneCompressor does not compress the request at all.
+type NoneCompressor struct{}
+
+// CompressRequest returns a reader for the body to the caller
+func (c *NoneCompressor) Compress(r *Request, body []byte) (io.Reader, error) {
+	buffer := bytes.NewBuffer(body)
+	r.DelHeader("Content-Encoding")
+
+	return buffer, nil
+}
+
+// GzipCompressor compresses the body with gzip
+type GzipCompressor struct{}
+
+// CompressRequest gzips the body, sets the content encoding header, and returns a reader to the caller
+func (c *GzipCompressor) Compress(r *Request, body []byte) (io.Reader, error) {
+	var err error
+
+	// Adaptive compression
+	if len(body) < CompressorMinimumSize {
+		buf := bytes.NewBuffer(body)
+		r.DelHeader("Content-Encoding")
+
+		return buf, nil
+	}
+
+	readBuffer := bufio.NewReader(bytes.NewReader(body))
+	buffer := bytes.NewBuffer([]byte{})
+	writer := gzip.NewWriter(buffer)
+
+	_, err = readBuffer.WriteTo(writer)
+	if err != nil {
+		return nil, err
+	}
+
+	err = writer.Close()
+	if err != nil {
+		return nil, err
+	}
+
+	r.SetHeader("Content-Encoding", "gzip")
+
+	return buffer, nil
+}
diff --git a/internal/http/request.go b/internal/http/request.go
index 5be9dca..5d0f337 100644
--- a/internal/http/request.go
+++ b/internal/http/request.go
@@ -1,7 +1,9 @@
 package http
 
 import (
+	"encoding/json"
 	"fmt"
+	"io"
 
 	"github.com/google/go-querystring/query"
 	retryablehttp "github.com/hashicorp/go-retryablehttp"
@@ -24,7 +26,11 @@ type Request struct {
 
 // NewRequest creates a new Request struct.
 func (c *Client) NewRequest(method string, url string, params interface{}, reqBody interface{}, value interface{}) (*Request, error) {
-	var err error
+	var (
+		err         error
+		requestBody []byte
+		readBuffer  io.Reader
+	)
 
 	req := &Request{
 		method:       method,
@@ -42,15 +48,23 @@ func (c *Client) NewRequest(method string, url string, params interface{}, reqBo
 	req.config = cfg
 
 	if reqBody != nil {
-		if _, ok := reqBody.([]byte); !ok {
-			reqBody, err = makeRequestBodyReader(reqBody)
+		switch val := reqBody.(type) {
+		case []byte:
+			requestBody = val
+		default:
+			requestBody, err = json.Marshal(val)
 			if err != nil {
 				return nil, err
 			}
 		}
+
+		readBuffer, err = c.compressor.Compress(req, requestBody)
+		if err != nil {
+			return nil, err
+		}
 	}
 
-	req.request, err = retryablehttp.NewRequest(req.method, url, reqBody)
+	req.request, err = retryablehttp.NewRequest(req.method, url, readBuffer)
 	if err != nil {
 		return nil, err
 	}
@@ -67,6 +81,18 @@ func (r *Request) SetHeader(key string, value string) {
 	r.request.Header.Set(key, value)
 }
 
+// GetHeader returns the value of the header requested
+func (r *Request) GetHeader(key string) string {
+	return r.request.Header.Get(key)
+}
+
+// DelHeader deletes the specified header if it exists
+func (r *Request) DelHeader(key string) {
+	if r != nil && r.request != nil && r.request.Header != nil {
+		r.request.Header.Del(key)
+	}
+}
+
 // SetAuthStrategy sets the authentication strategy for the request.
 func (r *Request) SetAuthStrategy(ra RequestAuthorizer) {
 	r.authStrategy = ra
diff --git a/pkg/config/compression.go b/pkg/config/compression.go
new file mode 100644
index 0000000..6fed152
--- /dev/null
+++ b/pkg/config/compression.go
@@ -0,0 +1,34 @@
+package config
+
+// CompressionType to use during transport.
+type CompressionType string
+
+// Compression is an Enum of compression types available
+var Compression = struct {
+	None CompressionType
+	Gzip CompressionType
+}{
+	None: "",
+	Gzip: "gzip",
+}
+
+// String returns the name of the compression type
+func (c CompressionType) String() string {
+	name := string(c)
+
+	if name == "" {
+		return "none"
+	}
+
+	return name
+}
+
+// ParseCompression takes a named compression and returns the Type
+func ParseCompression(name string) CompressionType {
+	switch name {
+	case "gzip":
+		return Compression.Gzip
+	default:
+		return Compression.None
+	}
+}
diff --git a/pkg/config/config.go b/pkg/config/config.go
index 784d18e..954cc73 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -21,6 +21,9 @@ type Config struct {
 	// see: https://docs.newrelic.com/docs/apis/get-started/intro-apis/types-new-relic-api-keys#admin
 	AdminAPIKey string
 
+	// InsightsInsertKey to send custom events to Insights
+	InsightsInsertKey string
+
 	// region of the New Relic platform to use
 	region *region.Region
 
@@ -30,6 +33,9 @@ type Config struct {
 	// HTTPTransport allows customization of the client's underlying transport.
 	HTTPTransport http.RoundTripper
 
+	// Compression used in sending data in HTTP requests.
+	Compression CompressionType
+
 	// UserAgent updates the default user agent string used by the client.
 	UserAgent string
 
@@ -52,9 +58,10 @@ func New() Config {
 	reg, _ := region.Get(region.Default)
 
 	return Config{
-		region:    reg,
-		UserAgent: "newrelic/newrelic-client-go",
-		LogLevel:  "info",
+		region:      reg,
+		UserAgent:   "newrelic/newrelic-client-go",
+		LogLevel:    "info",
+		Compression: Compression.None,
 	}
 }
 
diff --git a/pkg/region/region.go b/pkg/region/region.go
index 8c33cfb..479462b 100644
--- a/pkg/region/region.go
+++ b/pkg/region/region.go
@@ -7,6 +7,7 @@ package region
 
 import (
 	"errors"
+	"fmt"
 	"strings"
 
 	log "github.com/sirupsen/logrus"
@@ -22,6 +23,7 @@ type Region struct {
 	infrastructureBaseURL string
 	syntheticsBaseURL     string
 	nerdGraphBaseURL      string
+	insightsBaseURL       string
 }
 
 // String returns a human readable value for the specified Region Name
@@ -130,7 +132,7 @@ func (r *Region) SetSyntheticsBaseURL(url string) {
 	}
 }
 
-// SyntheticsURL returns the Full URL for Infrastructure REST API Calls, with any additional path elements appended
+// SyntheticsURL returns the Full URL for Synthetics REST API Calls, with any additional path elements appended
 func (r *Region) SyntheticsURL(path ...string) string {
 	if r == nil {
 		log.Errorf("call to nil region.SyntheticsURL")
@@ -146,6 +148,30 @@ func (r *Region) SyntheticsURL(path ...string) string {
 	return url
 }
 
+//
+// Insights Insert URL
+//
+
+func (r *Region) SetInsightsBaseURL(url string) {
+	if r != nil && url != "" {
+		r.insightsBaseURL = url
+	}
+}
+
+// InsightsURL returns the Full URL for Insights custom insert API calls
+func (r *Region) InsightsURL(accountID int) string {
+	if r == nil {
+		log.Errorf("call to nil region.SyntheticsURL")
+		return ""
+	}
+	if accountID < 1 {
+		log.Errorf("invalid account ID: %d", accountID)
+		return ""
+	}
+
+	return fmt.Sprintf("%s/%d/events", r.insightsBaseURL, accountID)
+}
+
 // concatURLPaths is a helper function for the URL builders below
 func concatURLPaths(host string, path []string) (string, error) {
 	if host == "" {
diff --git a/pkg/region/region_constants.go b/pkg/region/region_constants.go
index 509d0d1..94d1d9d 100644
--- a/pkg/region/region_constants.go
+++ b/pkg/region/region_constants.go
@@ -20,6 +20,7 @@ var Regions = map[Name]*Region{
 	US: {
 		name:                  "US",
 		infrastructureBaseURL: "https://infra-api.newrelic.com/v2",
+		insightsBaseURL:       "https://insights-collector.newrelic.com",
 		nerdGraphBaseURL:      "https://api.newrelic.com/graphql",
 		restBaseURL:           "https://api.newrelic.com/v2",
 		syntheticsBaseURL:     "https://synthetics.newrelic.com/synthetics/api",
@@ -27,6 +28,7 @@ var Regions = map[Name]*Region{
 	EU: {
 		name:                  "EU",
 		infrastructureBaseURL: "https://infra-api.eu.newrelic.com/v2",
+		insightsBaseURL:       "https://insights-collector.eu.newrelic.com",
 		nerdGraphBaseURL:      "https://api.eu.newrelic.com/graphql",
 		restBaseURL:           "https://api.eu.newrelic.com/v2",
 		syntheticsBaseURL:     "https://synthetics.eu.newrelic.com/synthetics/api",
@@ -34,6 +36,7 @@ var Regions = map[Name]*Region{
 	Staging: {
 		name:                  "Staging",
 		infrastructureBaseURL: "https://staging-infra-api.newrelic.com/v2",
+		insightsBaseURL:       "https://staging-insights-collector.newrelic.com",
 		nerdGraphBaseURL:      "https://staging-api.newrelic.com/graphql",
 		restBaseURL:           "https://staging-api.newrelic.com/v2",
 		syntheticsBaseURL:     "https://staging-synthetics.newrelic.com/synthetics/api",
diff --git a/pkg/synthetics/monitor_labels.go b/pkg/synthetics/monitor_labels.go
index cf88430..ec27b2e 100644
--- a/pkg/synthetics/monitor_labels.go
+++ b/pkg/synthetics/monitor_labels.go
@@ -32,9 +32,9 @@ func (s *Synthetics) AddMonitorLabel(monitorID, labelKey, labelValue string) err
 
 	data := fmt.Sprintf("%s:%s", strings.Title(labelKey), strings.Title(labelValue))
 
-	// We use RawPost here due to the Syntheics API's lack of support for JSON on
-	// this call.  The values must be POSTed as bare key:value word string.
-	_, err := s.client.RawPost(s.config.Region().SyntheticsURL(url), nil, data, nil)
+	// We pass []byte of data do avoid JSON encoding due to the Syntheics API's lack of
+	// support for JSON on this call.  The values must be POSTed as bare key:value word string.
+	_, err := s.client.Post(s.config.Region().SyntheticsURL(url), nil, []byte(data), nil)
 	if err != nil {
 		return err
 	}


Test patch:
diff --git a/internal/http/client_test.go b/internal/http/client_test.go
index fe28933..21045c5 100644
--- a/internal/http/client_test.go
+++ b/internal/http/client_test.go
@@ -314,29 +314,17 @@ func TestPost(t *testing.T) {
 		_, _ = w.Write([]byte(`{}`))
 	}))
 
-	_, err := c.Post(c.config.Region().RestURL("path"), &struct{}{}, &struct{}{}, &struct{}{})
-
-	assert.NoError(t, err)
-}
-
-func TestRawPost(t *testing.T) {
-	t.Parallel()
-	c := NewTestAPIClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.WriteHeader(http.StatusOK)
-		_, _ = w.Write([]byte(`{}`))
-	}))
-
 	// string
-	_, err := c.RawPost(c.config.Region().RestURL("path"), &struct{}{}, "test string payload", &struct{}{})
+	_, err := c.Post(c.config.Region().RestURL("path"), &struct{}{}, "test string payload", &struct{}{})
 	assert.NoError(t, err)
 
 	// []byte
-	_, err = c.RawPost(c.config.Region().RestURL("path"), &struct{}{}, []byte(`bytes`), &struct{}{})
+	_, err = c.Post(c.config.Region().RestURL("path"), &struct{}{}, []byte(`bytes`), &struct{}{})
 	assert.NoError(t, err)
 
-	// invalid
-	_, err = c.RawPost(c.config.Region().RestURL("path"), &struct{}{}, &struct{}{}, &struct{}{})
-	assert.Error(t, err)
+	// other data type
+	_, err = c.Post(c.config.Region().RestURL("path"), &struct{}{}, &struct{}{}, &struct{}{})
+	assert.NoError(t, err)
 }
 
 func TestPut(t *testing.T) {
diff --git a/internal/http/compress_test.go b/internal/http/compress_test.go
new file mode 100644
index 0000000..07efe9f
--- /dev/null
+++ b/internal/http/compress_test.go
@@ -0,0 +1,96 @@
+// +build unit
+
+package http
+
+import (
+	"io"
+	"io/ioutil"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	mock "github.com/newrelic/newrelic-client-go/pkg/testhelpers"
+)
+
+var testCompressionCases = []struct {
+	compress bool
+	data     []byte
+	gzip     []byte
+}{
+	{
+		compress: false,
+		data:     []byte("test"),
+	},
+	{
+		compress: false,
+		data:     []byte(`abcdefghijklmnopqrstuvwxyz1234567890`),
+	},
+	{
+		compress: true,
+		data:     []byte(`{"data": "json", "maybe":"didn't check", "handcrafted":true, "example": { "sub": "object", "arry": [ "why", "not" ] }, "todo": [ "make", "it", "larger" }`),
+		gzip: []byte{
+			0x1f, 0x8b, 0x8, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xff, 0x24, 0x8c, 0x4d, 0xae, 0xc2, 0x30, 0xc, 0x6, 0xaf, 0xf2,
+			0xe9, 0xdb, 0xbc, 0x4d, 0x4f, 0x90, 0xab, 0x3c, 0xb1, 0x70, 0x63, 0x43, 0x7f, 0x13, 0xe4, 0xba, 0x82, 0xaa, 0xea, 0xdd, 0x51, 0x60, 0x3b, 0xa3,
+			0x99, 0x93, 0x2a, 0x21, 0x4c, 0xe0, 0xb4, 0xd5, 0xc2, 0xe, 0x5c, 0xe5, 0xe8, 0x8d, 0x89, 0x3a, 0x6a, 0xf9, 0xb, 0xe4, 0xc1, 0xf2, 0xdc, 0xf8,
+			0x20, 0x45, 0xb3, 0xcb, 0x3d, 0x4c, 0x99, 0xc2, 0x77, 0xeb, 0x40, 0x7b, 0xcb, 0xfa, 0x5c, 0x8c, 0x9, 0x27, 0xb8, 0xed, 0x7d, 0xfb, 0xd4, 0x7e,
+			0xb2, 0x1c, 0xad, 0x10, 0xf7, 0x83, 0x9, 0xff, 0xe0, 0x6b, 0x38, 0x1a, 0x28, 0x35, 0x88, 0x1b, 0xae, 0xe, 0x8c, 0xaa, 0xf5, 0xe7, 0x56, 0x99,
+			0xad, 0xc9, 0xf1, 0xdb, 0x2c, 0xe2, 0xf, 0x73, 0xe2, 0xfa, 0x4, 0x0, 0x0, 0xff, 0xff, 0x60, 0x64, 0xbb, 0x7c, 0x99, 0x0, 0x0, 0x0,
+		},
+	},
+}
+
+func TestNoneCompressor(t *testing.T) {
+	t.Parallel()
+
+	tc := mock.NewTestConfig(t, nil)
+	c := NewClient(tc)
+
+	req, err := c.NewRequest("POST", c.config.Region().RestURL("path"), nil, nil, nil)
+	assert.NoError(t, err)
+
+	compress := NoneCompressor{}
+	var bodyReader io.Reader
+
+	for _, d := range testCompressionCases {
+		req.SetHeader("content-encoding", "<invalid>")
+
+		bodyReader, err = compress.Compress(req, d.data)
+		assert.NoError(t, err)
+		assert.Equal(t, "", req.GetHeader("content-encoding"))
+
+		res, err := ioutil.ReadAll(bodyReader)
+		assert.NoError(t, err)
+		assert.Equal(t, d.data, res)
+	}
+}
+
+func TestGzipCompressor(t *testing.T) {
+	t.Parallel()
+
+	tc := mock.NewTestConfig(t, nil)
+	c := NewClient(tc)
+
+	req, err := c.NewRequest("POST", c.config.Region().RestURL("path"), nil, nil, nil)
+	assert.NoError(t, err)
+
+	compress := GzipCompressor{}
+	var bodyReader io.Reader
+
+	for _, d := range testCompressionCases {
+		req.SetHeader("content-encoding", "<invalid>")
+
+		bodyReader, err = compress.Compress(req, d.data)
+		assert.NoError(t, err)
+
+		res, err := ioutil.ReadAll(bodyReader)
+		assert.NoError(t, err)
+
+		if d.compress {
+			assert.Equal(t, "gzip", req.GetHeader("content-encoding"))
+			assert.Equal(t, d.gzip, res)
+		} else {
+			assert.Equal(t, "", req.GetHeader("content-encoding"))
+			assert.Equal(t, d.data, res)
+		}
+	}
+}