Changes based on review comments

Signed-off-by: Alex Greenbank <alex.greenbank@grafana.com>

storage/remote/client.go

@@ -90,7 +90,6 @@ func init() {
 type Client struct {
 	remoteName   string // Used to differentiate clients in metrics.
 	urlString    string // url.String()
-	rwFormat     config.RemoteWriteFormat // For write clients, ignored for read clients.
 	lastRWHeader string
 	Client       *http.Client
 	timeout      time.Duration
@@ -173,7 +172,6 @@ func NewWriteClient(name string, conf *ClientConfig) (WriteClient, error) {
 	httpClient.Transport = otelhttp.NewTransport(t)
 
 	return &Client{
-		rwFormat:         conf.RemoteWriteFormat,
 		remoteName:       name,
 		urlString:        conf.URL.String(),
 		Client:           httpClient,
@@ -206,17 +204,14 @@ type RecoverableError struct {
 }
 
 // Attempt a HEAD request against a remote write endpoint to see what it supports.
-func (c *Client) GetProtoVersions(ctx context.Context) (string, error) {
-	// If we are in Version1 mode then don't even bother
-	if c.rwFormat == Version1 {
-		return RemoteWriteVersion1HeaderValue, nil
-	}
+func (c *Client) probeRemoteVersions(ctx context.Context) error {
+	// We assume we are in Version2 mode otherwise we shouldn't be calling this
 
 	httpReq, err := http.NewRequest("HEAD", c.urlString, nil)
 	if err != nil {
 		// Errors from NewRequest are from unparsable URLs, so are not
 		// recoverable.
-		return "", err
+		return err
 	}
 
 	// Set the version header to be nice
@@ -229,7 +224,7 @@ func (c *Client) GetProtoVersions(ctx context.Context) (string, error) {
 	httpResp, err := c.Client.Do(httpReq.WithContext(ctx))
 	if err != nil {
 		// We don't attempt a retry here
-		return "", err
+		return err
 	}
 
 	// See if we got a header anyway
@@ -250,11 +245,11 @@ func (c *Client) GetProtoVersions(ctx context.Context) (string, error) {
 			// a response that confirms it can speak 2.0
 			c.lastRWHeader = promHeader
 		}
-		return promHeader, fmt.Errorf(httpResp.Status)
+		return fmt.Errorf(httpResp.Status)
 	}
 
-	// All ok, return header and no error
-	return promHeader, nil
+	// All ok, return no error
+	return nil
 }
 
 // Store sends a batch of samples to the HTTP endpoint, the request is the proto marshalled
@@ -448,8 +443,8 @@ func NewTestClient(name, url string) WriteClient {
 	return &TestClient{name: name, url: url}
 }
 
-func (c *TestClient) GetProtoVersions(_ context.Context) (string, error) {
-	return "2.0;snappy,0.1.0", nil
+func (c *TestClient) probeRemoteVersions(_ context.Context) error {
+	return nil
 }
 
 func (c *TestClient) Store(_ context.Context, req []byte, _ int, _ config.RemoteWriteFormat, _ string) error {

storage/remote/queue_manager.go

@@ -395,7 +395,7 @@ type WriteClient interface {
 	// Endpoint is the remote read or write endpoint for the storage client.
 	Endpoint() string
 	// Get the protocol versions supported by the endpoint
-	GetProtoVersions(ctx context.Context) (string, error)
+	probeRemoteVersions(ctx context.Context) error
 	// Get the last RW header received from the endpoint
 	GetLastRWHeader() string
 }

storage/remote/queue_manager_test.go

@@ -1317,8 +1317,8 @@ func (c *TestWriteClient) Endpoint() string {
 	return "http://test-remote.com/1234"
 }
 
-func (c *TestWriteClient) GetProtoVersions(_ context.Context) (string, error) {
-	return "2.0;snappy,0.1.0", nil
+func (c *TestWriteClient) probeRemoteVersions(_ context.Context) error {
+	return nil
 }
 
 func (c *TestWriteClient) GetLastRWHeader() string {
@@ -1361,8 +1361,8 @@ func (c *TestBlockingWriteClient) Endpoint() string {
 	return "http://test-remote-blocking.com/1234"
 }
 
-func (c *TestBlockingWriteClient) GetProtoVersions(_ context.Context) (string, error) {
-	return "2.0;snappy,0.1.0", nil
+func (c *TestBlockingWriteClient) probeRemoteVersions(_ context.Context) error {
+	return nil
 }
 
 func (c *TestBlockingWriteClient) GetLastRWHeader() string {
@@ -1378,8 +1378,8 @@ func (c *NopWriteClient) Store(context.Context, []byte, int, config.RemoteWriteF
 }
 func (c *NopWriteClient) Name() string     { return "nopwriteclient" }
 func (c *NopWriteClient) Endpoint() string { return "http://test-remote.com/1234" }
-func (c *NopWriteClient) GetProtoVersions(_ context.Context) (string, error) {
-	return "2.0;snappy,0.1.0", nil
+func (c *NopWriteClient) probeRemoteVersions(_ context.Context) error {
+	return nil
 }
 func (c *NopWriteClient) GetLastRWHeader() string { return "2.0;snappy,0.1.0" }
 

storage/remote/write.go

@@ -203,8 +203,8 @@ func (rws *WriteStorage) ApplyConfig(conf *config.Config) error {
 			// If this newer remote write format is enabled then we need to probe the remote server
 			// to work out the desired protocol version and compressions
 			// The value of the header is kept in the client so no need to see it here
-			_, _ = c.GetProtoVersions(context.Background())
-			// TODO(alexg): Since they're never used should I remove the return values of GetProtoVersion()?
+			_ = c.probeRemoteVersions(context.Background())
+			// We ignore any error here, at some point we may choose to log it
 		}
 
 		// Redacted to remove any passwords in the URL (that are