Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clearer converter diagnostics #5505

Merged
merged 14 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)

### Enhancements
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(UnsupportedNotEquals(httpClientConfig.NoProxy, "", "HTTP Client no_proxy"))
mattdurham marked this conversation as resolved.
Show resolved Hide resolved
diags.AddAll(UnsupportedEquals(httpClientConfig.ProxyFromEnvironment, true, "HTTP Client proxy_from_environment"))
diags.AddAll(UnsupportedEquals(len(httpClientConfig.ProxyConnectHeader) > 0, true, "HTTP Client proxy_connect_header"))
diags.AddAll(UnsupportedNotEquals(httpClientConfig.TLSConfig.MaxVersion, prom_config.TLSVersion(0), "HTTP Client max_version"))

return diags
}
Expand Down
31 changes: 28 additions & 3 deletions converter/internal/common/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,44 @@ func UnsupportedNotDeepEqualsMessage(a any, b any, name string, message string)
var diags diag.Diagnostics
if !reflect.DeepEqual(a, b) {
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))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clayton-cornell please check out the wording in these diagnostic messages throughout to let me know what you think. There was some confusion between if something is a Flow error vs a Converter error that I'm trying to alleviate with these updates.

} 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 {
return UnsupportedNotEqualsMessage(a, b, name, "")
}

func UnsupportedNotEqualsMessage(a any, b any, name string, message string) diag.Diagnostics {
var diags diag.Diagnostics
if a != b {
diags.Add(diag.SeverityLevelError, fmt.Sprintf("unsupported %s config was provided.", name))
if 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("The converter does not support converting the provided %s config.", name))
}
}

return diags
}

func UnsupportedEquals(a any, b any, name string) diag.Diagnostics {
return UnsupportedEqualsMessage(a, b, name, "")
}

func UnsupportedEqualsMessage(a any, b any, name string, message string) diag.Diagnostics {
var diags diag.Diagnostics
if a == b {
if 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("The converter does not support converting the provided %s config.", name))
}
}

return diags
Expand Down
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.UnsupportedNotEquals(sdConfig.HTTPClientConfig.BasicAuth, nilBasicAuth, "digitalocean_sd_configs basic_auth"))
diags.AddAll(common.UnsupportedNotEquals(sdConfig.HTTPClientConfig.Authorization, nilAuthorization, "digitalocean_sd_configs authorization"))
diags.AddAll(common.UnsupportedNotEquals(sdConfig.HTTPClientConfig.OAuth2, nilOAuth2, "digitalocean_sd_configs oauth2"))
diags.AddAll(common.UnsupportedNotDeepEquals(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.UnsupportedNotEquals(sdConfig.HTTPClientConfig.BasicAuth, nilBasicAuth, "ec2_sd_configs basic_auth"))
diags.AddAll(common.UnsupportedNotEquals(sdConfig.HTTPClientConfig.Authorization, nilAuthorization, "ec2_sd_configs authorization"))
diags.AddAll(common.UnsupportedNotEquals(sdConfig.HTTPClientConfig.OAuth2, nilOAuth2, "ec2_sd_configs oauth2"))
diags.AddAll(common.UnsupportedNotDeepEquals(sdConfig.HTTPClientConfig.BearerToken, prom_config.DefaultHTTPClientConfig.BearerToken, "ec2_sd_configs bearer_token"))
diags.AddAll(common.UnsupportedNotDeepEquals(sdConfig.HTTPClientConfig.BearerTokenFile, prom_config.DefaultHTTPClientConfig.BearerTokenFile, "ec2_sd_configs bearer_token_file"))
diags.AddAll(common.UnsupportedNotDeepEquals(sdConfig.HTTPClientConfig.FollowRedirects, prom_config.DefaultHTTPClientConfig.FollowRedirects, "ec2_sd_configs follow_redirects"))
diags.AddAll(common.UnsupportedNotDeepEquals(sdConfig.HTTPClientConfig.EnableHTTP2, prom_config.DefaultHTTPClientConfig.EnableHTTP2, "ec2_sd_configs enable_http2"))
diags.AddAll(common.UnsupportedNotDeepEquals(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.UnsupportedNotDeepEquals(sdConfig.HTTPClientConfig, prom_config.DefaultHTTPClientConfig, "ec2_sd_configs http_client_config"))
}

return diags
Expand Down
46 changes: 13 additions & 33 deletions converter/internal/prometheusconvert/component/lightsail.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 appendDiscoveryLightsail(pb *build.PrometheusBlocks, label string, sdConfig
func ValidateDiscoveryLightsail(sdConfig *prom_aws.LightsailSDConfig) 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.UnsupportedNotEquals(sdConfig.HTTPClientConfig.BasicAuth, nilBasicAuth, "lightsail_sd_configs basic_auth"))
diags.AddAll(common.UnsupportedNotEquals(sdConfig.HTTPClientConfig.Authorization, nilAuthorization, "lightsail_sd_configs authorization"))
diags.AddAll(common.UnsupportedNotEquals(sdConfig.HTTPClientConfig.OAuth2, nilOAuth2, "lightsail_sd_configs oauth2"))
diags.AddAll(common.UnsupportedNotDeepEquals(sdConfig.HTTPClientConfig.BearerToken, prom_config.DefaultHTTPClientConfig.BearerToken, "lightsail_sd_configs bearer_token"))
diags.AddAll(common.UnsupportedNotDeepEquals(sdConfig.HTTPClientConfig.BearerTokenFile, prom_config.DefaultHTTPClientConfig.BearerTokenFile, "lightsail_sd_configs bearer_token_file"))
diags.AddAll(common.UnsupportedNotDeepEquals(sdConfig.HTTPClientConfig.FollowRedirects, prom_config.DefaultHTTPClientConfig.FollowRedirects, "lightsail_sd_configs follow_redirects"))
diags.AddAll(common.UnsupportedNotDeepEquals(sdConfig.HTTPClientConfig.EnableHTTP2, prom_config.DefaultHTTPClientConfig.EnableHTTP2, "lightsail_sd_configs enable_http2"))
diags.AddAll(common.UnsupportedNotDeepEquals(sdConfig.HTTPClientConfig.ProxyConfig, prom_config.DefaultHTTPClientConfig.ProxyConfig, "lightsail_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.UnsupportedNotDeepEquals(sdConfig.HTTPClientConfig, prom_config.DefaultHTTPClientConfig, "lightsail_sd_configs http_client_config"))
}

return diags
Expand Down
5 changes: 1 addition & 4 deletions converter/internal/prometheusconvert/component/scrape.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,7 @@ func AppendPrometheusScrape(pb *build.PrometheusBlocks, scrapeConfig *prom_confi
func ValidatePrometheusScrape(scrapeConfig *prom_config.ScrapeConfig) diag.Diagnostics {
var diags diag.Diagnostics

if scrapeConfig.NativeHistogramBucketLimit != 0 {
diags.Add(diag.SeverityLevelError, "unsupported native_histogram_bucket_limit for scrape_configs")
}

diags.AddAll(common.UnsupportedNotEquals(scrapeConfig.NativeHistogramBucketLimit, uint(0), "scrape_configs native_histogram_bucket_limit"))
diags.AddAll(common.ValidateHttpClientConfig(&scrapeConfig.HTTPClientConfig))

return diags
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func ValidateServiceDiscoveryConfig(serviceDiscoveryConfig prom_discover.Config)
return ValidateDiscoveryDockerswarm(sdc)
default:
var diags diag.Diagnostics
diags.Add(diag.SeverityLevelError, fmt.Sprintf("unsupported service discovery %s was provided", serviceDiscoveryConfig.Name()))
diags.Add(diag.SeverityLevelError, fmt.Sprintf("The converter does not support converting the provided %s service discovery.", serviceDiscoveryConfig.Name()))
return diags
}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
(Error) unsupported basic_auth for digitalocean_sd_configs
(Error) unsupported HTTP Client config proxy_from_environment was provided
(Error) unsupported HTTP Client config proxy_from_environment was provided
(Error) The converter does not support converting the provided digitalocean_sd_configs basic_auth config.
(Error) The converter does not support converting the provided HTTP Client proxy_from_environment config.
(Error) The converter does not support converting the provided HTTP Client proxy_from_environment config.
2 changes: 1 addition & 1 deletion converter/internal/prometheusconvert/testdata/ec2.diags
Original file line number Diff line number Diff line change
@@ -1 +1 @@
(Error) unsupported bearer_token_file for ec2_sd_configs
(Error) The converter does not support converting the provided ec2_sd_configs bearer_token_file config.
Original file line number Diff line number Diff line change
@@ -1 +1 @@
(Error) unsupported bearer_token_file for ec2_sd_configs
(Error) The converter does not support converting the provided lightsail_sd_configs bearer_token_file config.
24 changes: 12 additions & 12 deletions converter/internal/prometheusconvert/testdata/unsupported.diags
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
(Error) unsupported global evaluation_interval config was provided
(Error) unsupported global query_log_file config was provided
(Error) unsupported alerting config was provided
(Error) unsupported rule_files config was provided
(Error) unsupported HTTP Client config no_proxy was provided
(Error) unsupported service discovery nomad was provided
(Error) unsupported native_histogram_bucket_limit for scrape_configs
(Error) unsupported storage config was provided
(Error) unsupported tracing config was provided
(Error) unsupported HTTP Client config proxy_from_environment was provided
(Error) unsupported HTTP Client config max_version was provided
(Error) unsupported remote_read config was provided
(Error) The converter does not support converting the provided global evaluation_interval config.
(Error) The converter does not support converting the provided global query_log_file config.
(Error) The converter does not support converting the provided alerting config.
(Error) The converter does not support converting the provided rule_files config.
(Error) The converter does not support converting the provided HTTP Client no_proxy config.
(Error) The converter does not support converting the provided nomad service discovery.
(Error) The converter does not support converting the provided scrape_configs native_histogram_bucket_limit config.
(Error) The converter does not support converting the provided storage config.
(Error) The converter does not support converting the provided tracing config.
(Error) The converter does not support converting the provided HTTP Client proxy_from_environment config.
(Error) The converter does not support converting the provided HTTP Client max_version config.
(Error) The converter does not support converting the provided remote_read config.
Loading