[HCP Observability] New MetricsClient

agent/hcp/client/metrics_client.go

@@ -8,24 +8,22 @@ import (
 	"net/http"
 	"time"
 
-	"golang.org/x/oauth2"
-	"google.golang.org/protobuf/proto"
-
-	colmetricpb "go.opentelemetry.io/proto/otlp/collector/metrics/v1"
-	metricpb "go.opentelemetry.io/proto/otlp/metrics/v1"
-
-	"github.com/hashicorp/consul/version"
 	"github.com/hashicorp/go-cleanhttp"
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-retryablehttp"
 	hcpcfg "github.com/hashicorp/hcp-sdk-go/config"
+	colmetricpb "go.opentelemetry.io/proto/otlp/collector/metrics/v1"
+	metricpb "go.opentelemetry.io/proto/otlp/metrics/v1"
+	"golang.org/x/oauth2"
+	"google.golang.org/protobuf/proto"
 )
 
 const (
 	// HTTP Client config
 	defaultStreamTimeout = 15 * time.Second
 
 	// Retry config
+	// TODO: Evenutally, we'd like to configure these values dynamically.
 	defaultRetryWaitMin = 1 * time.Second
 	defaultRetryWaitMax = 15 * time.Second
 	defaultRetryMax     = 4
@@ -37,48 +35,44 @@ type MetricsClient interface {
 }
 
 // cloudConfig represents cloud config for TLS abstracted in an interface for easy testing.
-type cloudConfig interface {
+type CloudConfig interface {
 	HCPConfig(opts ...hcpcfg.HCPConfigOption) (hcpcfg.HCPConfig, error)
 }
 
 // otlpClient is an implementation of MetricsClient with a retryable http client for retries and to honor throttle.
 // It also holds default HTTP headers to add to export requests.
 type otlpClient struct {
-	client  *retryablehttp.Client
-	headers map[string]string
-}
-
-// TelemetryClientCfg is used to configure the MetricsClient.
-type TelemetryClientCfg struct {
-	CloudCfg cloudConfig
-	Logger   hclog.Logger
+	client *retryablehttp.Client
+	header *http.Header
 }
 
 // NewMetricsClient returns a configured MetricsClient.
 // The current implementation uses otlpClient to provide retry functionality.
-func NewMetricsClient(cfg *TelemetryClientCfg) (MetricsClient, error) {
-	if cfg.CloudCfg == nil || cfg.Logger == nil {
-		return nil, fmt.Errorf("failed to init telemetry client: provide valid TelemetryClientCfg")
+func NewMetricsClient(cfg CloudConfig, logger hclog.Logger) (MetricsClient, error) {
+	if cfg == nil {
+		return nil, fmt.Errorf("failed to init telemetry client: provide valid cloudCfg (Cloud Configuration for TLS)")
+	}
+
+	if logger == nil {
+		return nil, fmt.Errorf("failed to init telemetry client: provide a valid logger")
 	}
 
-	c, err := newHTTPClient(cfg.CloudCfg, cfg.Logger)
+	c, err := newHTTPClient(cfg, logger)
 	if err != nil {
 		return nil, fmt.Errorf("failed to init telemetry client: %v", err)
 	}
 
-	headers := map[string]string{
-		"X-HCP-Source-Channel": fmt.Sprintf("consul %s hcp-go-sdk/%s", version.GetHumanVersion(), version.Version),
-		"Content-Type":         "application/x-protobuf",
-	}
+	header := make(http.Header)
+	header.Set("Content-Type", "application/x-protobuf")
 
 	return &otlpClient{
-		client:  c,
-		headers: headers,
+		client: c,
+		header: &header,
 	}, nil
 }
 
 // newHTTPClient configures the retryable HTTP client.
-func newHTTPClient(cloudCfg cloudConfig, logger hclog.Logger) (*retryablehttp.Client, error) {
+func newHTTPClient(cloudCfg CloudConfig, logger hclog.Logger) (*retryablehttp.Client, error) {
 	hcpCfg, err := cloudCfg.HCPConfig()
 	if err != nil {
 		return nil, err
@@ -127,15 +121,13 @@ func (o *otlpClient) ExportMetrics(ctx context.Context, protoMetrics *metricpb.R
 	if err != nil {
 		return fmt.Errorf("failed to export metrics: %v", err)
 	}
+	req.Header = *o.header
 
-	for k, v := range o.headers {
-		req.Header.Set(k, v)
-	}
-
-	resp, err := o.client.Do(req)
+	resp, err := o.client.Do(req.WithContext(ctx))
 	if err != nil {
 		return fmt.Errorf("failed to export metrics: %v", err)
 	}
+	defer resp.Body.Close()
 
 	var respData bytes.Buffer
 	if _, err := io.Copy(&respData, resp.Body); err != nil {

agent/hcp/client/metrics_client_test.go

@@ -2,92 +2,45 @@ package client
 
 import (
 	"context"
-	"crypto/tls"
-	"errors"
-	"fmt"
-	"io"
 	"net/http"
 	"net/http/httptest"
-	"net/url"
 	"testing"
 
-	"golang.org/x/oauth2"
-	"google.golang.org/protobuf/proto"
-
-	colpb "go.opentelemetry.io/proto/otlp/collector/metrics/v1"
-	metricpb "go.opentelemetry.io/proto/otlp/metrics/v1"
-
-	"github.com/hashicorp/consul/version"
 	"github.com/hashicorp/go-hclog"
-	hcpcfg "github.com/hashicorp/hcp-sdk-go/config"
 	"github.com/stretchr/testify/require"
+	colpb "go.opentelemetry.io/proto/otlp/collector/metrics/v1"
+	metricpb "go.opentelemetry.io/proto/otlp/metrics/v1"
+	"google.golang.org/protobuf/proto"
 )
 
-type mockHCPCfg struct{}
-
-func (m *mockHCPCfg) Token() (*oauth2.Token, error) {
-	return &oauth2.Token{
-		AccessToken: "test-token",
-	}, nil
-}
-
-func (m *mockHCPCfg) APITLSConfig() *tls.Config { return nil }
-
-func (m *mockHCPCfg) SCADAAddress() string { return "" }
-
-func (m *mockHCPCfg) SCADATLSConfig() *tls.Config { return &tls.Config{} }
-
-func (m *mockHCPCfg) APIAddress() string { return "" }
-
-func (m *mockHCPCfg) PortalURL() *url.URL { return &url.URL{} }
-
-type mockCloudCfg struct{}
-
-func (m mockCloudCfg) HCPConfig(opts ...hcpcfg.HCPConfigOption) (hcpcfg.HCPConfig, error) {
-	return &mockHCPCfg{}, nil
-}
-
-type mockErrCloudCfg struct{}
-
-func (m mockErrCloudCfg) HCPConfig(opts ...hcpcfg.HCPConfigOption) (hcpcfg.HCPConfig, error) {
-	return nil, errors.New("test bad HCP config")
-}
-
 func TestNewMetricsClient(t *testing.T) {
 	for name, test := range map[string]struct {
 		wantErr string
-		cfg     *TelemetryClientCfg
+		cfg     CloudConfig
+		logger  hclog.Logger
 	}{
 		"success": {
-			cfg: &TelemetryClientCfg{
-				Logger:   hclog.New(&hclog.LoggerOptions{Output: io.Discard}),
-				CloudCfg: &mockCloudCfg{},
-			},
+			cfg:    &MockCloudCfg{},
+			logger: hclog.NewNullLogger(),
 		},
 		"failsWithoutCloudCfg": {
-			wantErr: "failed to init telemetry client",
-			cfg: &TelemetryClientCfg{
-				Logger:   hclog.New(&hclog.LoggerOptions{Output: io.Discard}),
-				CloudCfg: nil,
-			},
+			wantErr: "failed to init telemetry client: provide valid cloudCfg (Cloud Configuration for TLS)",
+			cfg:     nil,
+			logger:  hclog.NewNullLogger(),
 		},
 		"failsWithoutLogger": {
-			wantErr: "failed to init telemetry client",
-			cfg: &TelemetryClientCfg{
-				Logger:   nil,
-				CloudCfg: &mockErrCloudCfg{},
-			},
+			wantErr: "failed to init telemetry client: provide a valid logger",
+			cfg:     MockCloudCfg{},
+			logger:  nil,
 		},
 		"failsHCPConfig": {
 			wantErr: "failed to init telemetry client",
-			cfg: &TelemetryClientCfg{
-				Logger:   hclog.New(&hclog.LoggerOptions{Output: io.Discard}),
-				CloudCfg: &mockErrCloudCfg{},
-			},
+			cfg:     MockErrCloudCfg{},
+			logger:  hclog.NewNullLogger(),
 		},
 	} {
 		t.Run(name, func(t *testing.T) {
-			client, err := NewMetricsClient(test.cfg)
+			client, err := NewMetricsClient(test.cfg, test.logger)
 			if test.wantErr != "" {
 				require.Error(t, err)
 				require.Contains(t, err.Error(), test.wantErr)
@@ -118,7 +71,6 @@ func TestExportMetrics(t *testing.T) {
 				require.Equal(t, r.Header.Get("Content-Type"), "application/x-protobuf")
 
 				require.Equal(t, r.Header.Get("Authorization"), "Bearer test-token")
-				require.Equal(t, r.Header.Get("X-HCP-Source-Channel"), fmt.Sprintf("consul %s hcp-go-sdk/%s", version.GetHumanVersion(), version.Version))
 
 				body := colpb.ExportMetricsServiceResponse{}
 
@@ -137,12 +89,7 @@ func TestExportMetrics(t *testing.T) {
 			}))
 			defer srv.Close()
 
-			cfg := &TelemetryClientCfg{
-				Logger:   hclog.New(&hclog.LoggerOptions{Output: io.Discard}),
-				CloudCfg: mockCloudCfg{},
-			}
-
-			client, err := NewMetricsClient(cfg)
+			client, err := NewMetricsClient(MockCloudCfg{}, hclog.NewNullLogger())
 			require.NoError(t, err)
 
 			ctx := context.Background()
@@ -158,5 +105,4 @@ func TestExportMetrics(t *testing.T) {
 			require.NoError(t, err)
 		})
 	}
-
 }

agent/hcp/client/mock_CloudConfig.go

@@ -0,0 +1,40 @@
+package client
+
+import (
+	"crypto/tls"
+	"errors"
+	"net/url"
+
+	hcpcfg "github.com/hashicorp/hcp-sdk-go/config"
+	"golang.org/x/oauth2"
+)
+
+type mockHCPCfg struct{}
+
+func (m *mockHCPCfg) Token() (*oauth2.Token, error) {
+	return &oauth2.Token{
+		AccessToken: "test-token",
+	}, nil
+}
+
+func (m *mockHCPCfg) APITLSConfig() *tls.Config { return nil }
+
+func (m *mockHCPCfg) SCADAAddress() string { return "" }
+
+func (m *mockHCPCfg) SCADATLSConfig() *tls.Config { return &tls.Config{} }
+
+func (m *mockHCPCfg) APIAddress() string { return "" }
+
+func (m *mockHCPCfg) PortalURL() *url.URL { return &url.URL{} }
+
+type MockCloudCfg struct{}
+
+func (m MockCloudCfg) HCPConfig(opts ...hcpcfg.HCPConfigOption) (hcpcfg.HCPConfig, error) {
+	return &mockHCPCfg{}, nil
+}
+
+type MockErrCloudCfg struct{}
+
+func (m MockErrCloudCfg) HCPConfig(opts ...hcpcfg.HCPConfigOption) (hcpcfg.HCPConfig, error) {
+	return nil, errors.New("test bad HCP config")
+}