Skip to content

Commit

Permalink
remove require.Contains by using error types, fix structure logging, …
Browse files Browse the repository at this point in the history
…and fix success metric typo in exporter
  • Loading branch information
Achooo committed Aug 1, 2023
1 parent 952129c commit b46d5b9
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 21 deletions.
18 changes: 9 additions & 9 deletions agent/hcp/client/telemetry_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"time"

"github.com/hashicorp/go-hclog"
"github.com/hashicorp/go-multierror"
hcptelemetry "github.com/hashicorp/hcp-sdk-go/clients/cloud-consul-telemetry-gateway/preview/2023-04-14/client/consul_telemetry_service"

"github.com/hashicorp/consul/agent/hcp/config"
Expand All @@ -25,6 +24,8 @@ var (
errMissingTelemetryConfig = errors.New("missing telemetry config")
errMissingRefreshConfig = errors.New("missing refresh config")
errMissingMetricsConfig = errors.New("missing metrics config")
errInvalidRefreshInterval = errors.New("invalid refresh interval")
errInvalidEndpoint = errors.New("invalid metrics endpoint")
)

// TelemetryConfig contains configuration for telemetry data forwarded by Consul servers
Expand Down Expand Up @@ -76,14 +77,15 @@ func validateAgentTelemetryConfigPayload(resp *hcptelemetry.AgentTelemetryConfig
func convertAgentTelemetryResponse(ctx context.Context, resp *hcptelemetry.AgentTelemetryConfigOK, cfg config.CloudConfig) (*TelemetryConfig, error) {
refreshInterval, err := time.ParseDuration(resp.Payload.RefreshConfig.RefreshInterval)
if err != nil {
return nil, fmt.Errorf("invalid refresh interval: %w", err)
return nil, fmt.Errorf("%w: %w", errInvalidRefreshInterval, err)
}

telemetryConfig := resp.Payload.TelemetryConfig
metricsEndpoint, err := convertMetricEndpoint(telemetryConfig.Endpoint, telemetryConfig.Metrics.Endpoint)
if err != nil {
return nil, fmt.Errorf("failed to parse metrics endpoint: %w", err)
return nil, errInvalidEndpoint
}

metricsFilters := convertMetricFilters(ctx, telemetryConfig.Metrics.IncludeList)
metricLabels := convertMetricLabels(telemetryConfig.Labels, cfg)

Expand Down Expand Up @@ -118,7 +120,7 @@ func convertMetricEndpoint(telemetryEndpoint string, metricsEndpoint string) (*u
rawUrl := endpoint + metricsGatewayPath
u, err := url.ParseRequestURI(rawUrl)
if err != nil {
return nil, fmt.Errorf("failed to parse url: %w", err)
return nil, fmt.Errorf("%w: %w", errInvalidEndpoint, err)
}

return u, nil
Expand All @@ -128,28 +130,26 @@ func convertMetricEndpoint(telemetryEndpoint string, metricsEndpoint string) (*u
// if invalid filters are given, a defaults regex that allow all metrics is returned.
func convertMetricFilters(ctx context.Context, payloadFilters []string) *regexp.Regexp {
logger := hclog.FromContext(ctx)

var mErr error
validFilters := make([]string, 0, len(payloadFilters))
for _, filter := range payloadFilters {
_, err := regexp.Compile(filter)
if err != nil {
mErr = multierror.Append(mErr, fmt.Errorf("compilation of filter %q failed: %w", filter, err))
logger.Error("invalid filter", "error", err)
continue
}
validFilters = append(validFilters, filter)
}

if len(validFilters) == 0 {
logger.Error("no valid filters", "error", mErr)
logger.Error("no valid filters")
return defaultMetricFilters
}

// Combine the valid regex strings with OR.
finalRegex := strings.Join(validFilters, "|")
composedRegex, err := regexp.Compile(finalRegex)
if err != nil {
logger.Error("failed to compile regex", "error", mErr)
logger.Error("failed to compile final regex", "error", err)
return defaultMetricFilters
}

Expand Down
20 changes: 9 additions & 11 deletions agent/hcp/client/telemetry_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestConvertAgentTelemetryResponse(t *testing.T) {
for name, tc := range map[string]struct {
resp *consul_telemetry_service.AgentTelemetryConfigOK
expectedTelemetryCfg *TelemetryConfig
wantErr string
wantErr error
expectedEnabled bool
}{
"success": {
Expand Down Expand Up @@ -179,7 +179,7 @@ func TestConvertAgentTelemetryResponse(t *testing.T) {
},
},
},
wantErr: "invalid refresh interval",
wantErr: errInvalidRefreshInterval,
},
"errorsWithInvalidEndpoint": {
resp: &consul_telemetry_service.AgentTelemetryConfigOK{
Expand All @@ -194,14 +194,13 @@ func TestConvertAgentTelemetryResponse(t *testing.T) {
},
},
},
wantErr: "failed to parse metrics endpoint",
wantErr: errInvalidEndpoint,
},
} {
t.Run(name, func(t *testing.T) {
telemetryCfg, err := convertAgentTelemetryResponse(context.Background(), tc.resp, config.CloudConfig{})
if tc.wantErr != "" {
require.Error(t, err)
require.Contains(t, err.Error(), tc.wantErr)
if tc.wantErr != nil {
require.ErrorIs(t, err, tc.wantErr)
require.Nil(t, telemetryCfg)
return
}
Expand All @@ -218,7 +217,7 @@ func TestConvertMetricEndpoint(t *testing.T) {
endpoint string
override string
expected string
wantErr string
wantErr error
}{
"success": {
endpoint: "https://test.com",
Expand All @@ -237,16 +236,15 @@ func TestConvertMetricEndpoint(t *testing.T) {
"errorWithInvalidURL": {
endpoint: " ",
override: "",
wantErr: "failed to parse url",
wantErr: errInvalidEndpoint,
},
} {
tc := tc
t.Run(name, func(t *testing.T) {
t.Parallel()
u, err := convertMetricEndpoint(tc.endpoint, tc.override)
if tc.wantErr != "" {
require.Error(t, err)
require.Contains(t, err.Error(), tc.wantErr)
if tc.wantErr != nil {
require.ErrorIs(t, err, tc.wantErr)
require.Empty(t, u)
return
}
Expand Down
2 changes: 1 addition & 1 deletion agent/hcp/telemetry/custom_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ package telemetry
var (
internalMetricTransformFailure []string = []string{"hcp", "otel", "transform", "failure"}

internalMetricExportSuccess []string = []string{"hcp", "otel", "exporter", "export", "sucess"}
internalMetricExportSuccess []string = []string{"hcp", "otel", "exporter", "export", "success"}
internalMetricExportFailure []string = []string{"hcp", "otel", "exporter", "export", "failure"}

internalMetricExporterShutdown []string = []string{"hcp", "otel", "exporter", "shutdown"}
Expand Down

0 comments on commit b46d5b9

Please sign in to comment.