refactor remotecfg.go

internal/service/remotecfg/api_client.go

@@ -0,0 +1,119 @@
+package remotecfg
+
+import (
+	"context"
+	"time"
+
+	"connectrpc.com/connect"
+	collectorv1 "github.com/grafana/alloy-remote-config/api/gen/proto/go/collector/v1"
+	"github.com/grafana/alloy-remote-config/api/gen/proto/go/collector/v1/collectorv1connect"
+	"github.com/grafana/alloy/internal/useragent"
+	commonconfig "github.com/prometheus/common/config"
+)
+
+var userAgent = useragent.Get()
+
+// Package-level function for creating API clients - can be replaced in tests to
+// use a mock client which doesn't make any API calls.
+var createAPIClient = newAPIClient
+
+// apiClient is a wrapper around the collectorv1connect.CollectorServiceClient that
+// provides metrics and error handling.
+type apiClient struct {
+	client  collectorv1connect.CollectorServiceClient
+	metrics *metrics
+}
+
+var _ collectorv1connect.CollectorServiceClient = (*apiClient)(nil)
+
+// newAPIClient creates a CollectorServiceClient instance with metrics wrapper based on the provided Arguments configuration.
+func newAPIClient(args Arguments, metrics *metrics) (collectorv1connect.CollectorServiceClient, error) {
+	client, err := newCollectorClient(args)
+	if err != nil {
+		return nil, err
+	}
+	return newAPIClientWithClient(client, metrics), nil
+}
+
+// newAPIClientWithClient creates a metrics-wrapped apiClient from an existing CollectorServiceClient.
+// This is primarily used for testing with mock clients.
+func newAPIClientWithClient(client collectorv1connect.CollectorServiceClient, metrics *metrics) *apiClient {
+	return &apiClient{
+		client:  client,
+		metrics: metrics,
+	}
+}
+
+// newCollectorClient creates a CollectorServiceClient instance based on the provided Arguments configuration.
+func newCollectorClient(args Arguments) (collectorv1connect.CollectorServiceClient, error) {
+	httpClient, err := commonconfig.NewClientFromConfig(*args.HTTPClientConfig.Convert(), "remoteconfig")
+	if err != nil {
+		return nil, err
+	}
+	return collectorv1connect.NewCollectorServiceClient(
+		httpClient,
+		args.URL,
+		connect.WithHTTPGet(),
+		connect.WithInterceptors(newAgentInterceptor()),
+	), nil
+}
+
+func (c *apiClient) GetConfig(ctx context.Context, req *connect.Request[collectorv1.GetConfigRequest]) (*connect.Response[collectorv1.GetConfigResponse], error) {
+	start := time.Now()
+	resp, err := c.client.GetConfig(ctx, req)
+	if err != nil {
+		return nil, err
+	}
+	c.metrics.getConfigTime.Observe(time.Since(start).Seconds())
+	if resp.Msg.NotModified {
+		return nil, errNotModified
+	}
+
+	return resp, nil
+}
+
+func (c *apiClient) RegisterCollector(ctx context.Context, req *connect.Request[collectorv1.RegisterCollectorRequest]) (*connect.Response[collectorv1.RegisterCollectorResponse], error) {
+	resp, err := c.client.RegisterCollector(ctx, req)
+	if err != nil {
+		return nil, err
+	}
+	return resp, nil
+}
+
+func (c *apiClient) UnregisterCollector(ctx context.Context, req *connect.Request[collectorv1.UnregisterCollectorRequest]) (*connect.Response[collectorv1.UnregisterCollectorResponse], error) {
+	resp, err := c.client.UnregisterCollector(ctx, req)
+	if err != nil {
+		return nil, err
+	}
+	return resp, nil
+}
+
+// agentInterceptor adds User-Agent headers to requests
+type agentInterceptor struct {
+	agent string
+}
+
+func newAgentInterceptor() *agentInterceptor {
+	return &agentInterceptor{
+		agent: userAgent,
+	}
+}
+
+func (i *agentInterceptor) WrapUnary(next connect.UnaryFunc) connect.UnaryFunc {
+	return func(ctx context.Context, req connect.AnyRequest) (connect.AnyResponse, error) {
+		req.Header().Set("User-Agent", i.agent)
+		return next(ctx, req)
+	}
+}
+
+func (i *agentInterceptor) WrapStreamingClient(next connect.StreamingClientFunc) connect.StreamingClientFunc {
+	return func(ctx context.Context, spec connect.Spec) connect.StreamingClientConn {
+		conn := next(ctx, spec)
+		conn.RequestHeader().Set("User-Agent", i.agent)
+		return conn
+	}
+}
+
+func (i *agentInterceptor) WrapStreamingHandler(next connect.StreamingHandlerFunc) connect.StreamingHandlerFunc {
+	return next
+}

internal/service/remotecfg/noop.go → internal/service/remotecfg/api_client_noop.go

@@ -6,12 +6,21 @@ import (
 
 	"connectrpc.com/connect"
 	collectorv1 "github.com/grafana/alloy-remote-config/api/gen/proto/go/collector/v1"
+	"github.com/grafana/alloy-remote-config/api/gen/proto/go/collector/v1/collectorv1connect"
 )
 
+// noopClient is a no-op implementation of the collectorv1connect.CollectorServiceClient.
+// It is used when the remotecfg service is disabled.
 type noopClient struct{}
 
+var _ collectorv1connect.CollectorServiceClient = (*noopClient)(nil)
+
 var errNoopClient = errors.New("noop client")
 
+func newNoopClient() *noopClient {
+	return &noopClient{}
+}
+
 // GetConfig returns the collector's configuration.
 func (c noopClient) GetConfig(context.Context, *connect.Request[collectorv1.GetConfigRequest]) (*connect.Response[collectorv1.GetConfigResponse], error) {
 	return nil, errNoopClient

internal/service/remotecfg/api_client_noop_test.go

@@ -0,0 +1,87 @@
+package remotecfg
+
+import (
+	"testing"
+
+	"connectrpc.com/connect"
+	collectorv1 "github.com/grafana/alloy-remote-config/api/gen/proto/go/collector/v1"
+	"github.com/grafana/alloy-remote-config/api/gen/proto/go/collector/v1/collectorv1connect"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestNewNoopClient(t *testing.T) {
+	client := newNoopClient()
+
+	require.NotNil(t, client)
+	assert.IsType(t, &noopClient{}, client)
+}
+
+func TestNoopClient_InterfaceCompliance(t *testing.T) {
+	var _ collectorv1connect.CollectorServiceClient = (*noopClient)(nil)
+
+	client := newNoopClient()
+	var _ collectorv1connect.CollectorServiceClient = client
+}
+
+func TestNoopClient_GetConfig(t *testing.T) {
+	client := newNoopClient()
+	ctx := t.Context()
+
+	req := &connect.Request[collectorv1.GetConfigRequest]{
+		Msg: &collectorv1.GetConfigRequest{
+			Id:              "test-id",
+			LocalAttributes: map[string]string{"test": "value"},
+			Hash:            "test-hash",
+		},
+	}
+
+	resp, err := client.GetConfig(ctx, req)
+
+	assert.Nil(t, resp)
+	assert.Error(t, err)
+	assert.Equal(t, errNoopClient, err)
+	assert.Equal(t, "noop client", err.Error())
+}
+
+func TestNoopClient_RegisterCollector(t *testing.T) {
+	client := newNoopClient()
+	ctx := t.Context()
+
+	req := &connect.Request[collectorv1.RegisterCollectorRequest]{
+		Msg: &collectorv1.RegisterCollectorRequest{
+			Id:              "test-id",
+			LocalAttributes: map[string]string{"test": "value"},
+			Name:            "test-collector",
+		},
+	}
+
+	resp, err := client.RegisterCollector(ctx, req)
+
+	assert.Nil(t, resp)
+	assert.Error(t, err)
+	assert.Equal(t, errNoopClient, err)
+	assert.Equal(t, "noop client", err.Error())
+}
+
+func TestNoopClient_UnregisterCollector(t *testing.T) {
+	client := newNoopClient()
+	ctx := t.Context()
+
+	req := &connect.Request[collectorv1.UnregisterCollectorRequest]{
+		Msg: &collectorv1.UnregisterCollectorRequest{
+			Id: "test-id",
+		},
+	}
+
+	resp, err := client.UnregisterCollector(ctx, req)
+
+	assert.Nil(t, resp)
+	assert.Error(t, err)
+	assert.Equal(t, errNoopClient, err)
+	assert.Equal(t, "noop client", err.Error())
+}
+
+func TestErrNoopClient(t *testing.T) {
+	assert.Equal(t, "noop client", errNoopClient.Error())
+}