Skip to content

Commit

Permalink
Clearer converter diagnostics (grafana#5505)
Browse files Browse the repository at this point in the history
* Update a bunch of converter messages and use standardized functions where possible for validation.

* default promtail log level from static converter so warnings don't show

* update migration doc output to match current behaviour

* refactor validation helper functions

Signed-off-by: erikbaranowski <[email protected]>

---------

Signed-off-by: erikbaranowski <[email protected]>
  • Loading branch information
erikbaranowski authored Oct 19, 2023
1 parent 08b9a23 commit 06c6792
Show file tree
Hide file tree
Showing 29 changed files with 266 additions and 225 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ Main (unreleased)
- Fixed issue where adding a module after initial start, that failed to load then subsequently resolving the issue would cause the module to
permanently fail to load with `id already exists` error. (@mattdurham)

- Fixed some converter diagnostics so they show as warnings rather than errors. Improve
clarity for various diagnostics. (@erikbaranowski)

- Allow the usage of encodings other than UTF8 to be used with environment variable expansion. (@mattdurham)

- Fixed an issue where native histogram time series were being dropped silently. (@krajorama)
Expand Down
2 changes: 1 addition & 1 deletion converter/diag/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (ds Diagnostics) GenerateReport(writer io.Writer, reportType string) error
case Text:
return generateTextReport(writer, ds)
default:
return fmt.Errorf("unsupported diagnostic report type %q", reportType)
return fmt.Errorf("Invalid diagnostic report type %q", reportType)
}
}

Expand Down
19 changes: 4 additions & 15 deletions converter/internal/common/http_client_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,10 @@ func ToHttpClientConfig(httpClientConfig *prom_config.HTTPClientConfig) *config.
func ValidateHttpClientConfig(httpClientConfig *prom_config.HTTPClientConfig) diag.Diagnostics {
var diags diag.Diagnostics

if httpClientConfig.NoProxy != "" {
diags.Add(diag.SeverityLevelError, "unsupported HTTP Client config no_proxy was provided")
}

if httpClientConfig.ProxyFromEnvironment {
diags.Add(diag.SeverityLevelError, "unsupported HTTP Client config proxy_from_environment was provided")
}

if len(httpClientConfig.ProxyConnectHeader) > 0 {
diags.Add(diag.SeverityLevelError, "unsupported HTTP Client config proxy_connect_header was provided")
}

if httpClientConfig.TLSConfig.MaxVersion != 0 {
diags.Add(diag.SeverityLevelError, "unsupported HTTP Client config max_version was provided")
}
diags.AddAll(ValidateSupported(NotEquals, httpClientConfig.NoProxy, "", "HTTP Client no_proxy", ""))
diags.AddAll(ValidateSupported(Equals, httpClientConfig.ProxyFromEnvironment, true, "HTTP Client proxy_from_environment", ""))
diags.AddAll(ValidateSupported(Equals, len(httpClientConfig.ProxyConnectHeader) > 0, true, "HTTP Client proxy_connect_header", ""))
diags.AddAll(ValidateSupported(NotEquals, httpClientConfig.TLSConfig.MaxVersion, prom_config.TLSVersion(0), "HTTP Client max_version", ""))

return diags
}
Expand Down
46 changes: 30 additions & 16 deletions converter/internal/common/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,32 +9,46 @@ import (
"github.com/grafana/river/token/builder"
)

func UnsupportedNotDeepEquals(a any, b any, name string) diag.Diagnostics {
return UnsupportedNotDeepEqualsMessage(a, b, name, "")
}
const (
Equals = iota
NotEquals
DeepEquals
NotDeepEquals
)

func UnsupportedNotDeepEqualsMessage(a any, b any, name string, message string) diag.Diagnostics {
// ValidateSupported will return a diagnostic error if the validationType
// specified results in a match for value1 and value2.
//
// For example, if using validationType Equals and value1 is equal to value2,
// then a diagnostic error will be returned.
func ValidateSupported(validationType int, value1 any, value2 any, name string, message string) diag.Diagnostics {
var diags diag.Diagnostics
if !reflect.DeepEqual(a, b) {
var isInvalid bool

switch validationType {
case Equals:
isInvalid = value1 == value2
case NotEquals:
isInvalid = value1 != value2
case DeepEquals:
isInvalid = reflect.DeepEqual(value1, value2)
case NotDeepEquals:
isInvalid = !reflect.DeepEqual(value1, value2)
default:
diags.Add(diag.SeverityLevelCritical, fmt.Sprintf("Invalid converter validation type was requested: %d.", validationType))
}

if isInvalid {
if message != "" {
diags.Add(diag.SeverityLevelError, fmt.Sprintf("unsupported %s config was provided: %s", name, message))
diags.Add(diag.SeverityLevelError, fmt.Sprintf("The converter does not support converting the provided %s config: %s", name, message))
} else {
diags.Add(diag.SeverityLevelError, fmt.Sprintf("unsupported %s config was provided.", name))
diags.Add(diag.SeverityLevelError, fmt.Sprintf("The converter does not support converting the provided %s config.", name))
}
}

return diags
}

func UnsupportedNotEquals(a any, b any, name string) diag.Diagnostics {
var diags diag.Diagnostics
if a != b {
diags.Add(diag.SeverityLevelError, fmt.Sprintf("unsupported %s config was provided.", name))
}

return diags
}

// ValidateNodes will look at the final nodes and ensure that there are no
// duplicate labels.
func ValidateNodes(f *builder.File) diag.Diagnostics {
Expand Down
122 changes: 122 additions & 0 deletions converter/internal/common/validate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
package common_test

import (
"fmt"
"testing"

"github.com/grafana/agent/converter/diag"
"github.com/grafana/agent/converter/internal/common"
"github.com/stretchr/testify/require"
)

func TestInvalidValidationType(t *testing.T) {
diags := common.ValidateSupported(-1, nil, nil, "", "")
require.Len(t, diags, 1)
var expectedDiags diag.Diagnostics
expectedDiags.Add(diag.SeverityLevelCritical, "Invalid converter validation type was requested: -1.")
require.Equal(t, expectedDiags, diags)
}

func TestValidateSupported(t *testing.T) {
tt := []struct {
tcName string
validationType int
value1 any
value2 any
name string
message string
expectDiag bool
}{
{
tcName: "Unsupported Equals",
validationType: common.Equals,
value1: "match",
value2: "match",
name: "test",
message: "",
expectDiag: true,
},
{
tcName: "Supported Equals",
validationType: common.Equals,
value1: "not",
value2: "match",
name: "test",
message: "",
expectDiag: false,
},
{
tcName: "Unsupported NotEquals",
validationType: common.NotEquals,
value1: "not",
value2: "match",
name: "test",
message: "message",
expectDiag: true,
},
{
tcName: "Supported NotEquals",
validationType: common.NotEquals,
value1: "match",
value2: "match",
name: "test",
message: "message",
expectDiag: false,
},
{
tcName: "Unsupported DeepEquals",
validationType: common.DeepEquals,
value1: []string{"match"},
value2: []string{"match"},
name: "test",
message: "",
expectDiag: true,
},
{
tcName: "Supported DeepEquals",
validationType: common.DeepEquals,
value1: []string{"not"},
value2: []string{"match"},
name: "test",
message: "message",
expectDiag: false,
},
{
tcName: "Supported NotDeepEquals",
validationType: common.NotDeepEquals,
value1: []string{"not"},
value2: []string{"match"},
name: "test",
message: "",
expectDiag: true,
},
{
tcName: "Supported NotDeepEquals",
validationType: common.NotDeepEquals,
value1: []string{"match"},
value2: []string{"match"},
name: "test",
message: "message",
expectDiag: false,
},
}

for _, tc := range tt {
t.Run(tc.tcName, func(t *testing.T) {
diags := common.ValidateSupported(tc.validationType, tc.value1, tc.value2, tc.name, tc.message)
if tc.expectDiag {
require.Len(t, diags, 1)
var expectedDiags diag.Diagnostics
if tc.message != "" {
expectedDiags.Add(diag.SeverityLevelError, fmt.Sprintf("The converter does not support converting the provided %s config: %s", tc.name, tc.message))
} else {
expectedDiags.Add(diag.SeverityLevelError, fmt.Sprintf("The converter does not support converting the provided %s config.", tc.name))
}

require.Equal(t, expectedDiags, diags)
} else {
require.Len(t, diags, 0)
}
})
}
}
22 changes: 7 additions & 15 deletions converter/internal/prometheusconvert/component/digitalocean.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package component

import (
"reflect"
"time"

"github.com/grafana/agent/component/common/config"
Expand All @@ -26,21 +25,14 @@ func appendDiscoveryDigitalOcean(pb *build.PrometheusBlocks, label string, sdCon
func ValidateDiscoveryDigitalOcean(sdConfig *prom_digitalocean.SDConfig) diag.Diagnostics {
var diags diag.Diagnostics

if sdConfig.HTTPClientConfig.BasicAuth != nil {
diags.Add(diag.SeverityLevelError, "unsupported basic_auth for digitalocean_sd_configs")
}

if sdConfig.HTTPClientConfig.Authorization != nil {
diags.Add(diag.SeverityLevelError, "unsupported authorization for digitalocean_sd_configs")
}
var nilBasicAuth *prom_config.BasicAuth
var nilAuthorization *prom_config.Authorization
var nilOAuth2 *prom_config.OAuth2

if sdConfig.HTTPClientConfig.OAuth2 != nil {
diags.Add(diag.SeverityLevelError, "unsupported oauth2 for digitalocean_sd_configs")
}

if !reflect.DeepEqual(prom_config.TLSConfig{}, sdConfig.HTTPClientConfig.TLSConfig) {
diags.Add(diag.SeverityLevelError, "unsupported oauth2 for digitalocean_sd_configs")
}
diags.AddAll(common.ValidateSupported(common.NotEquals, sdConfig.HTTPClientConfig.BasicAuth, nilBasicAuth, "digitalocean_sd_configs basic_auth", ""))
diags.AddAll(common.ValidateSupported(common.NotEquals, sdConfig.HTTPClientConfig.Authorization, nilAuthorization, "digitalocean_sd_configs authorization", ""))
diags.AddAll(common.ValidateSupported(common.NotEquals, sdConfig.HTTPClientConfig.OAuth2, nilOAuth2, "digitalocean_sd_configs oauth2", ""))
diags.AddAll(common.ValidateSupported(common.NotDeepEquals, sdConfig.HTTPClientConfig.TLSConfig, prom_config.TLSConfig{}, "digitalocean_sd_configs tls_config", ""))

diags.AddAll(common.ValidateHttpClientConfig(&sdConfig.HTTPClientConfig))

Expand Down
10 changes: 5 additions & 5 deletions converter/internal/prometheusconvert/component/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,22 @@ import (
"github.com/grafana/agent/converter/diag"
"github.com/grafana/agent/converter/internal/common"
"github.com/grafana/agent/converter/internal/prometheusconvert/build"
prom_docker "github.com/prometheus/prometheus/discovery/moby"
prom_moby "github.com/prometheus/prometheus/discovery/moby"
)

func appendDiscoveryDocker(pb *build.PrometheusBlocks, label string, sdConfig *prom_docker.DockerSDConfig) discovery.Exports {
func appendDiscoveryDocker(pb *build.PrometheusBlocks, label string, sdConfig *prom_moby.DockerSDConfig) discovery.Exports {
discoveryDockerArgs := toDiscoveryDocker(sdConfig)
name := []string{"discovery", "docker"}
block := common.NewBlockWithOverride(name, label, discoveryDockerArgs)
pb.DiscoveryBlocks = append(pb.DiscoveryBlocks, build.NewPrometheusBlock(block, name, label, "", ""))
return common.NewDiscoveryExports("discovery.docker." + label + ".targets")
}

func ValidateDiscoveryDocker(sdConfig *prom_docker.DockerSDConfig) diag.Diagnostics {
func ValidateDiscoveryDocker(sdConfig *prom_moby.DockerSDConfig) diag.Diagnostics {
return common.ValidateHttpClientConfig(&sdConfig.HTTPClientConfig)
}

func toDiscoveryDocker(sdConfig *prom_docker.DockerSDConfig) *docker.Arguments {
func toDiscoveryDocker(sdConfig *prom_moby.DockerSDConfig) *docker.Arguments {
if sdConfig == nil {
return nil
}
Expand All @@ -38,7 +38,7 @@ func toDiscoveryDocker(sdConfig *prom_docker.DockerSDConfig) *docker.Arguments {
}
}

func toDockerFilters(filtersConfig []prom_docker.Filter) []docker.Filter {
func toDockerFilters(filtersConfig []prom_moby.Filter) []docker.Filter {
filters := make([]docker.Filter, 0)

for _, filter := range filtersConfig {
Expand Down
46 changes: 13 additions & 33 deletions converter/internal/prometheusconvert/component/ec2.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package component

import (
"reflect"
"time"

"github.com/grafana/agent/component/discovery"
Expand All @@ -25,41 +24,22 @@ func appendDiscoveryEC2(pb *build.PrometheusBlocks, label string, sdConfig *prom
func ValidateDiscoveryEC2(sdConfig *prom_aws.EC2SDConfig) diag.Diagnostics {
var diags diag.Diagnostics

if sdConfig.HTTPClientConfig.BasicAuth != nil {
diags.Add(diag.SeverityLevelError, "unsupported basic_auth for ec2_sd_configs")
}

if sdConfig.HTTPClientConfig.Authorization != nil {
diags.Add(diag.SeverityLevelError, "unsupported authorization for ec2_sd_configs")
}

if sdConfig.HTTPClientConfig.OAuth2 != nil {
diags.Add(diag.SeverityLevelError, "unsupported oauth2 for ec2_sd_configs")
}

if !reflect.DeepEqual(sdConfig.HTTPClientConfig.BearerToken, prom_config.DefaultHTTPClientConfig.BearerToken) {
diags.Add(diag.SeverityLevelError, "unsupported bearer_token for ec2_sd_configs")
}
var nilBasicAuth *prom_config.BasicAuth
var nilAuthorization *prom_config.Authorization
var nilOAuth2 *prom_config.OAuth2

if !reflect.DeepEqual(sdConfig.HTTPClientConfig.BearerTokenFile, prom_config.DefaultHTTPClientConfig.BearerTokenFile) {
diags.Add(diag.SeverityLevelError, "unsupported bearer_token_file for ec2_sd_configs")
}

if !reflect.DeepEqual(sdConfig.HTTPClientConfig.FollowRedirects, prom_config.DefaultHTTPClientConfig.FollowRedirects) {
diags.Add(diag.SeverityLevelError, "unsupported follow_redirects for ec2_sd_configs")
}

if !reflect.DeepEqual(sdConfig.HTTPClientConfig.EnableHTTP2, prom_config.DefaultHTTPClientConfig.EnableHTTP2) {
diags.Add(diag.SeverityLevelError, "unsupported enable_http2 for ec2_sd_configs")
}

if !reflect.DeepEqual(sdConfig.HTTPClientConfig.ProxyConfig, prom_config.DefaultHTTPClientConfig.ProxyConfig) {
diags.Add(diag.SeverityLevelError, "unsupported proxy for ec2_sd_configs")
}
diags.AddAll(common.ValidateSupported(common.NotEquals, sdConfig.HTTPClientConfig.BasicAuth, nilBasicAuth, "ec2_sd_configs basic_auth", ""))
diags.AddAll(common.ValidateSupported(common.NotEquals, sdConfig.HTTPClientConfig.Authorization, nilAuthorization, "ec2_sd_configs authorization", ""))
diags.AddAll(common.ValidateSupported(common.NotEquals, sdConfig.HTTPClientConfig.OAuth2, nilOAuth2, "ec2_sd_configs oauth2", ""))
diags.AddAll(common.ValidateSupported(common.NotDeepEquals, sdConfig.HTTPClientConfig.BearerToken, prom_config.DefaultHTTPClientConfig.BearerToken, "ec2_sd_configs bearer_token", ""))
diags.AddAll(common.ValidateSupported(common.NotDeepEquals, sdConfig.HTTPClientConfig.BearerTokenFile, prom_config.DefaultHTTPClientConfig.BearerTokenFile, "ec2_sd_configs bearer_token_file", ""))
diags.AddAll(common.ValidateSupported(common.NotDeepEquals, sdConfig.HTTPClientConfig.FollowRedirects, prom_config.DefaultHTTPClientConfig.FollowRedirects, "ec2_sd_configs follow_redirects", ""))
diags.AddAll(common.ValidateSupported(common.NotDeepEquals, sdConfig.HTTPClientConfig.EnableHTTP2, prom_config.DefaultHTTPClientConfig.EnableHTTP2, "ec2_sd_configs enable_http2", ""))
diags.AddAll(common.ValidateSupported(common.NotDeepEquals, sdConfig.HTTPClientConfig.ProxyConfig, prom_config.DefaultHTTPClientConfig.ProxyConfig, "ec2_sd_configs proxy", ""))

// Do a last check in case any of the specific checks missed anything.
if len(diags) == 0 && !reflect.DeepEqual(sdConfig.HTTPClientConfig, prom_config.DefaultHTTPClientConfig) {
diags.Add(diag.SeverityLevelError, "unsupported http_client_config for ec2_sd_configs")
if len(diags) == 0 {
diags.AddAll(common.ValidateSupported(common.NotDeepEquals, sdConfig.HTTPClientConfig, prom_config.DefaultHTTPClientConfig, "ec2_sd_configs http_client_config", ""))
}

return diags
Expand Down
Loading

0 comments on commit 06c6792

Please sign in to comment.