diff --git a/agent/agent.go b/agent/agent.go index ebc5e87b6c..bf4f88c2b7 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -2924,7 +2924,8 @@ func (a *Agent) ReloadConfig(newCfg *config.RuntimeConfig) error { } // Update filtered metrics - metrics.UpdateFilter(newCfg.TelemetryAllowedPrefixes, newCfg.TelemetryBlockedPrefixes) + metrics.UpdateFilter(newCfg.Telemetry.AllowedPrefixes, + newCfg.Telemetry.BlockedPrefixes) a.State.SetDiscardCheckOutput(newCfg.DiscardCheckOutput) diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go index 66db9f1544..f71ea7df4e 100644 --- a/agent/agent_endpoint.go +++ b/agent/agent_endpoint.go @@ -9,6 +9,8 @@ import ( "strings" "time" + "github.com/mitchellh/mapstructure" + "github.com/hashicorp/go-memdb" "github.com/mitchellh/hashstructure" @@ -104,7 +106,7 @@ func (s *HTTPServer) AgentMetrics(resp http.ResponseWriter, req *http.Request) ( return nil, acl.ErrPermissionDenied } if enablePrometheusOutput(req) { - if s.agent.config.TelemetryPrometheusRetentionTime < 1 { + if s.agent.config.Telemetry.PrometheusRetentionTime < 1 { resp.WriteHeader(http.StatusUnsupportedMediaType) fmt.Fprint(resp, "Prometheus is not enable since its retention time is not positive") return nil, nil @@ -1061,26 +1063,35 @@ func (s *HTTPServer) AgentConnectProxyConfig(resp http.ResponseWriter, req *http target.Port) } - // Add telemetry config - telemetry := s.agent.config.TelemetryConfig(false) - if len(telemetry) > 0 { - // Rely on the fact that TelemetryConfig makes a new map each call to - // override the prefix here without affecting other callers. - telemetry["MetricsPrefix"] = "consul.proxy." + target.ID + // Add telemetry config. Copy the global config so we can customize the + // prefix. + telemetryCfg := s.agent.config.Telemetry + telemetryCfg.MetricsPrefix = telemetryCfg.MetricsPrefix + ".proxy." + target.ID - // Merge with any config passed by the user to allow service definition - // to override. - if userRaw, ok := config["telemetry"]; ok { - if userT, ok := userRaw.(map[string]interface{}); ok { - for k, v := range telemetry { - if _, ok := userT[k]; !ok { - userT[k] = v - } - } + // First see if the user has specified telemetry + if userRaw, ok := config["telemetry"]; ok { + // User specified domething, see if it is compatible with agent + // telemetry config: + var uCfg lib.TelemetryConfig + dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + Result: &uCfg, + // Make sure that if the user passes something that isn't just a + // simple override of a valid TelemetryConfig that we fail so that we + // don't clobber their custom config. + ErrorUnused: true, + }) + if err == nil { + if err = dec.Decode(userRaw); err == nil { + // It did decode! Merge any unspecified fields from agent config. + uCfg.MergeDefaults(&telemetryCfg) + config["telemetry"] = uCfg } - } else { - config["telemetry"] = telemetry } + // Failed to decode, just keep user's config["telemetry"] verbatim + // with no agent merge. + } else { + // Add agent telemetry config. + config["telemetry"] = telemetryCfg } reply := &api.ConnectProxyConfig{ diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index be9dc7d81a..c4086b8c04 100644 --- a/agent/agent_endpoint_test.go +++ b/agent/agent_endpoint_test.go @@ -21,6 +21,7 @@ import ( "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/logger" "github.com/hashicorp/consul/testutil/retry" "github.com/hashicorp/consul/types" @@ -3248,10 +3249,10 @@ func TestAgentConnectProxyConfig_aclServiceReadDeny(t *testing.T) { require.True(acl.IsErrPermissionDenied(err)) } -func makeTelemetryDefaults(targetID string) map[string]interface{} { - return map[string]interface{}{ - "FilterDefault": true, - "MetricsPrefix": "consul.proxy." + targetID, +func makeTelemetryDefaults(targetID string) lib.TelemetryConfig { + return lib.TelemetryConfig{ + FilterDefault: true, + MetricsPrefix: "consul.proxy." + targetID, } } @@ -3403,10 +3404,10 @@ func TestAgentConnectProxyConfig_ConfigHandling(t *testing.T) { "local_service_address": "127.0.0.1:8000", // port from service reg "connect_timeout_ms": 1000, "foo": "bar", - "telemetry": map[string]interface{}{ - "FilterDefault": true, - "MetricsPrefix": "consul.proxy." + reg.ID, - "StatsiteAddr": "localhost:8989", + "telemetry": lib.TelemetryConfig{ + FilterDefault: true, + MetricsPrefix: "consul.proxy." + reg.ID, + StatsiteAddr: "localhost:8989", }, }, }, @@ -3445,7 +3446,8 @@ func TestAgentConnectProxyConfig_ConfigHandling(t *testing.T) { "bind_port": 1024, "local_service_address": "127.0.0.1:9191", "telemetry": map[string]interface{}{ - "StatsiteAddr": "stats.it:10101", + "statsite_address": "stats.it:10101", + "metrics_prefix": "foo", // important! checks that our prefix logic respects user customization }, }, }, @@ -3456,10 +3458,47 @@ func TestAgentConnectProxyConfig_ConfigHandling(t *testing.T) { "bind_port": float64(1024), "local_service_address": "127.0.0.1:9191", "connect_timeout_ms": float64(2000), + "telemetry": lib.TelemetryConfig{ + FilterDefault: true, + MetricsPrefix: "foo", + StatsiteAddr: "stats.it:10101", + }, + }, + }, + { + name: "reg telemetry not compatible, preserved with no merge", + globalConfig: ` + connect { + enabled = true + proxy { + allow_managed_api_registration = true + } + } + ports { + proxy_min_port = 10000 + proxy_max_port = 10000 + } + telemetry { + statsite_address = "localhost:8989" + } + `, + proxy: structs.ServiceDefinitionConnectProxy{ + ExecMode: "script", + Command: []string{"foo.sh"}, + Config: map[string]interface{}{ + "telemetry": map[string]interface{}{ + "foo": "bar", + }, + }, + }, + wantMode: api.ProxyExecModeScript, + wantCommand: []string{"foo.sh"}, + wantConfig: map[string]interface{}{ + "bind_address": "127.0.0.1", + "bind_port": 10000, // "randomly" chosen from our range of 1 + "local_service_address": "127.0.0.1:8000", // port from service reg "telemetry": map[string]interface{}{ - "FilterDefault": true, - "MetricsPrefix": "consul.proxy." + reg.ID, - "StatsiteAddr": "stats.it:10101", + "foo": "bar", }, }, }, diff --git a/agent/config/builder.go b/agent/config/builder.go index 2ffbdd21ff..9e34d2fbad 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/consul/agent/consul" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/ipaddr" + "github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/tlsutil" "github.com/hashicorp/consul/types" multierror "github.com/hashicorp/go-multierror" @@ -625,29 +626,31 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) { HTTPResponseHeaders: c.HTTPConfig.ResponseHeaders, // Telemetry - TelemetryCirconusAPIApp: b.stringVal(c.Telemetry.CirconusAPIApp), - TelemetryCirconusAPIToken: b.stringVal(c.Telemetry.CirconusAPIToken), - TelemetryCirconusAPIURL: b.stringVal(c.Telemetry.CirconusAPIURL), - TelemetryCirconusBrokerID: b.stringVal(c.Telemetry.CirconusBrokerID), - TelemetryCirconusBrokerSelectTag: b.stringVal(c.Telemetry.CirconusBrokerSelectTag), - TelemetryCirconusCheckDisplayName: b.stringVal(c.Telemetry.CirconusCheckDisplayName), - TelemetryCirconusCheckForceMetricActivation: b.stringVal(c.Telemetry.CirconusCheckForceMetricActivation), - TelemetryCirconusCheckID: b.stringVal(c.Telemetry.CirconusCheckID), - TelemetryCirconusCheckInstanceID: b.stringVal(c.Telemetry.CirconusCheckInstanceID), - TelemetryCirconusCheckSearchTag: b.stringVal(c.Telemetry.CirconusCheckSearchTag), - TelemetryCirconusCheckTags: b.stringVal(c.Telemetry.CirconusCheckTags), - TelemetryCirconusSubmissionInterval: b.stringVal(c.Telemetry.CirconusSubmissionInterval), - TelemetryCirconusSubmissionURL: b.stringVal(c.Telemetry.CirconusSubmissionURL), - TelemetryDisableHostname: b.boolVal(c.Telemetry.DisableHostname), - TelemetryDogstatsdAddr: b.stringVal(c.Telemetry.DogstatsdAddr), - TelemetryDogstatsdTags: c.Telemetry.DogstatsdTags, - TelemetryPrometheusRetentionTime: b.durationVal("prometheus_retention_time", c.Telemetry.PrometheusRetentionTime), - TelemetryFilterDefault: b.boolVal(c.Telemetry.FilterDefault), - TelemetryAllowedPrefixes: telemetryAllowedPrefixes, - TelemetryBlockedPrefixes: telemetryBlockedPrefixes, - TelemetryMetricsPrefix: b.stringVal(c.Telemetry.MetricsPrefix), - TelemetryStatsdAddr: b.stringVal(c.Telemetry.StatsdAddr), - TelemetryStatsiteAddr: b.stringVal(c.Telemetry.StatsiteAddr), + Telemetry: lib.TelemetryConfig{ + CirconusAPIApp: b.stringVal(c.Telemetry.CirconusAPIApp), + CirconusAPIToken: b.stringVal(c.Telemetry.CirconusAPIToken), + CirconusAPIURL: b.stringVal(c.Telemetry.CirconusAPIURL), + CirconusBrokerID: b.stringVal(c.Telemetry.CirconusBrokerID), + CirconusBrokerSelectTag: b.stringVal(c.Telemetry.CirconusBrokerSelectTag), + CirconusCheckDisplayName: b.stringVal(c.Telemetry.CirconusCheckDisplayName), + CirconusCheckForceMetricActivation: b.stringVal(c.Telemetry.CirconusCheckForceMetricActivation), + CirconusCheckID: b.stringVal(c.Telemetry.CirconusCheckID), + CirconusCheckInstanceID: b.stringVal(c.Telemetry.CirconusCheckInstanceID), + CirconusCheckSearchTag: b.stringVal(c.Telemetry.CirconusCheckSearchTag), + CirconusCheckTags: b.stringVal(c.Telemetry.CirconusCheckTags), + CirconusSubmissionInterval: b.stringVal(c.Telemetry.CirconusSubmissionInterval), + CirconusSubmissionURL: b.stringVal(c.Telemetry.CirconusSubmissionURL), + DisableHostname: b.boolVal(c.Telemetry.DisableHostname), + DogstatsdAddr: b.stringVal(c.Telemetry.DogstatsdAddr), + DogstatsdTags: c.Telemetry.DogstatsdTags, + PrometheusRetentionTime: b.durationVal("prometheus_retention_time", c.Telemetry.PrometheusRetentionTime), + FilterDefault: b.boolVal(c.Telemetry.FilterDefault), + AllowedPrefixes: telemetryAllowedPrefixes, + BlockedPrefixes: telemetryBlockedPrefixes, + MetricsPrefix: b.stringVal(c.Telemetry.MetricsPrefix), + StatsdAddr: b.stringVal(c.Telemetry.StatsdAddr), + StatsiteAddr: b.stringVal(c.Telemetry.StatsiteAddr), + }, // Agent AdvertiseAddrLAN: advertiseAddrLAN, diff --git a/agent/config/runtime.go b/agent/config/runtime.go index 695bff954c..67a06a22a3 100644 --- a/agent/config/runtime.go +++ b/agent/config/runtime.go @@ -9,6 +9,7 @@ import ( "time" "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/tlsutil" "github.com/hashicorp/consul/types" "golang.org/x/time/rate" @@ -299,177 +300,8 @@ type RuntimeConfig struct { // hcl: http_config { response_headers = map[string]string } HTTPResponseHeaders map[string]string - // TelemetryCirconus*: see https://github.com/circonus-labs/circonus-gometrics - // for more details on the various configuration options. - // Valid configuration combinations: - // - CirconusAPIToken - // metric management enabled (search for existing check or create a new one) - // - CirconusSubmissionUrl - // metric management disabled (use check with specified submission_url, - // broker must be using a public SSL certificate) - // - CirconusAPIToken + CirconusCheckSubmissionURL - // metric management enabled (use check with specified submission_url) - // - CirconusAPIToken + CirconusCheckID - // metric management enabled (use check with specified id) - - // TelemetryCirconusAPIApp is an app name associated with API token. - // Default: "consul" - // - // hcl: telemetry { circonus_api_app = string } - TelemetryCirconusAPIApp string - - // TelemetryCirconusAPIToken is a valid API Token used to create/manage check. If provided, - // metric management is enabled. - // Default: none - // - // hcl: telemetry { circonus_api_token = string } - TelemetryCirconusAPIToken string - - // TelemetryCirconusAPIURL is the base URL to use for contacting the Circonus API. - // Default: "https://api.circonus.com/v2" - // - // hcl: telemetry { circonus_api_url = string } - TelemetryCirconusAPIURL string - - // TelemetryCirconusBrokerID is an explicit broker to use when creating a new check. The numeric portion - // of broker._cid. If metric management is enabled and neither a Submission URL nor Check ID - // is provided, an attempt will be made to search for an existing check using Instance ID and - // Search Tag. If one is not found, a new HTTPTRAP check will be created. - // Default: use Select Tag if provided, otherwise, a random Enterprise Broker associated - // with the specified API token or the default Circonus Broker. - // Default: none - // - // hcl: telemetry { circonus_broker_id = string } - TelemetryCirconusBrokerID string - - // TelemetryCirconusBrokerSelectTag is a special tag which will be used to select a broker when - // a Broker ID is not provided. The best use of this is to as a hint for which broker - // should be used based on *where* this particular instance is running. - // (e.g. a specific geo location or datacenter, dc:sfo) - // Default: none - // - // hcl: telemetry { circonus_broker_select_tag = string } - TelemetryCirconusBrokerSelectTag string - - // TelemetryCirconusCheckDisplayName is the name for the check which will be displayed in the Circonus UI. - // Default: value of CirconusCheckInstanceID - // - // hcl: telemetry { circonus_check_display_name = string } - TelemetryCirconusCheckDisplayName string - - // TelemetryCirconusCheckForceMetricActivation will force enabling metrics, as they are encountered, - // if the metric already exists and is NOT active. If check management is enabled, the default - // behavior is to add new metrics as they are encountered. If the metric already exists in the - // check, it will *NOT* be activated. This setting overrides that behavior. - // Default: "false" - // - // hcl: telemetry { circonus_check_metrics_activation = (true|false) - TelemetryCirconusCheckForceMetricActivation string - - // TelemetryCirconusCheckID is the check id (not check bundle id) from a previously created - // HTTPTRAP check. The numeric portion of the check._cid field. - // Default: none - // - // hcl: telemetry { circonus_check_id = string } - TelemetryCirconusCheckID string - - // TelemetryCirconusCheckInstanceID serves to uniquely identify the metrics coming from this "instance". - // It can be used to maintain metric continuity with transient or ephemeral instances as - // they move around within an infrastructure. - // Default: hostname:app - // - // hcl: telemetry { circonus_check_instance_id = string } - TelemetryCirconusCheckInstanceID string - - // TelemetryCirconusCheckSearchTag is a special tag which, when coupled with the instance id, helps to - // narrow down the search results when neither a Submission URL or Check ID is provided. - // Default: service:app (e.g. service:consul) - // - // hcl: telemetry { circonus_check_search_tag = string } - TelemetryCirconusCheckSearchTag string - - // TelemetryCirconusCheckSearchTag is a special tag which, when coupled with the instance id, helps to - // narrow down the search results when neither a Submission URL or Check ID is provided. - // Default: service:app (e.g. service:consul) - // - // hcl: telemetry { circonus_check_tags = string } - TelemetryCirconusCheckTags string - - // TelemetryCirconusSubmissionInterval is the interval at which metrics are submitted to Circonus. - // Default: 10s - // - // hcl: telemetry { circonus_submission_interval = "duration" } - TelemetryCirconusSubmissionInterval string - - // TelemetryCirconusCheckSubmissionURL is the check.config.submission_url field from a - // previously created HTTPTRAP check. - // Default: none - // - // hcl: telemetry { circonus_submission_url = string } - TelemetryCirconusSubmissionURL string - - // DisableHostname will disable hostname prefixing for all metrics. - // - // hcl: telemetry { disable_hostname = (true|false) - TelemetryDisableHostname bool - - // TelemetryDogStatsdAddr is the address of a dogstatsd instance. If provided, - // metrics will be sent to that instance - // - // hcl: telemetry { dogstatsd_addr = string } - TelemetryDogstatsdAddr string - - // TelemetryDogStatsdTags are the global tags that should be sent with each packet to dogstatsd - // It is a list of strings, where each string looks like "my_tag_name:my_tag_value" - // - // hcl: telemetry { dogstatsd_tags = []string } - TelemetryDogstatsdTags []string - - // PrometheusRetentionTime is the retention time for prometheus metrics if greater than 0. - // A value of 0 disable Prometheus support. Regarding Prometheus, it is considered a good - // practice to put large values here (such as a few days), and at least the interval between - // prometheus requests. - // - // hcl: telemetry { prometheus_retention_time = "duration" } - TelemetryPrometheusRetentionTime time.Duration - - // TelemetryFilterDefault is the default for whether to allow a metric that's not - // covered by the filter. - // - // hcl: telemetry { filter_default = (true|false) } - TelemetryFilterDefault bool - - // TelemetryAllowedPrefixes is a list of filter rules to apply for allowing metrics - // by prefix. Use the 'prefix_filter' option and prefix rules with '+' to be - // included. - // - // hcl: telemetry { prefix_filter = []string{"+", "+", ...} } - TelemetryAllowedPrefixes []string - - // TelemetryBlockedPrefixes is a list of filter rules to apply for blocking metrics - // by prefix. Use the 'prefix_filter' option and prefix rules with '-' to be - // excluded. - // - // hcl: telemetry { prefix_filter = []string{"-", "-", ...} } - TelemetryBlockedPrefixes []string - - // TelemetryMetricsPrefix is the prefix used to write stats values to. - // Default: "consul." - // - // hcl: telemetry { metrics_prefix = string } - TelemetryMetricsPrefix string - - // TelemetryStatsdAddr is the address of a statsd instance. If provided, - // metrics will be sent to that instance. - // - // hcl: telemetry { statsd_addr = string } - TelemetryStatsdAddr string - - // TelemetryStatsiteAddr is the address of a statsite instance. If provided, - // metrics will be streamed to that instance. - // - // hcl: telemetry { statsite_addr = string } - TelemetryStatsiteAddr string + // Embed Telemetry Config + Telemetry lib.TelemetryConfig // Datacenter is the datacenter this node is in. Defaults to "dc1". // diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 9852347c67..af3b1296e5 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -19,10 +19,11 @@ import ( "time" "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/types" "github.com/pascaldekloe/goe/verify" - "github.com/sergi/go-diff/diffmatchpatch" + "github.com/stretchr/testify/require" ) type configTest struct { @@ -1852,8 +1853,8 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { `}, patch: func(rt *RuntimeConfig) { rt.DataDir = dataDir - rt.TelemetryAllowedPrefixes = []string{"foo"} - rt.TelemetryBlockedPrefixes = []string{"bar"} + rt.Telemetry.AllowedPrefixes = []string{"foo"} + rt.Telemetry.BlockedPrefixes = []string{"bar"} }, warns: []string{`Filter rule must begin with either '+' or '-': "nix"`}, }, @@ -3822,41 +3823,43 @@ func TestFullConfig(t *testing.T) { }, }, }, - SerfAdvertiseAddrLAN: tcpAddr("17.99.29.16:8301"), - SerfAdvertiseAddrWAN: tcpAddr("78.63.37.19:8302"), - SerfBindAddrLAN: tcpAddr("99.43.63.15:8301"), - SerfBindAddrWAN: tcpAddr("67.88.33.19:8302"), - SessionTTLMin: 26627 * time.Second, - SkipLeaveOnInt: true, - StartJoinAddrsLAN: []string{"LR3hGDoG", "MwVpZ4Up"}, - StartJoinAddrsWAN: []string{"EbFSc3nA", "kwXTh623"}, - SyslogFacility: "hHv79Uia", - TelemetryCirconusAPIApp: "p4QOTe9j", - TelemetryCirconusAPIToken: "E3j35V23", - TelemetryCirconusAPIURL: "mEMjHpGg", - TelemetryCirconusBrokerID: "BHlxUhed", - TelemetryCirconusBrokerSelectTag: "13xy1gHm", - TelemetryCirconusCheckDisplayName: "DRSlQR6n", - TelemetryCirconusCheckForceMetricActivation: "Ua5FGVYf", - TelemetryCirconusCheckID: "kGorutad", - TelemetryCirconusCheckInstanceID: "rwoOL6R4", - TelemetryCirconusCheckSearchTag: "ovT4hT4f", - TelemetryCirconusCheckTags: "prvO4uBl", - TelemetryCirconusSubmissionInterval: "DolzaflP", - TelemetryCirconusSubmissionURL: "gTcbS93G", - TelemetryDisableHostname: true, - TelemetryDogstatsdAddr: "0wSndumK", - TelemetryDogstatsdTags: []string{"3N81zSUB", "Xtj8AnXZ"}, - TelemetryFilterDefault: true, - TelemetryAllowedPrefixes: []string{"oJotS8XJ"}, - TelemetryBlockedPrefixes: []string{"cazlEhGn"}, - TelemetryMetricsPrefix: "ftO6DySn", - TelemetryPrometheusRetentionTime: 15 * time.Second, - TelemetryStatsdAddr: "drce87cy", - TelemetryStatsiteAddr: "HpFwKB8R", - TLSCipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305, tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384}, - TLSMinVersion: "pAOWafkR", - TLSPreferServerCipherSuites: true, + SerfAdvertiseAddrLAN: tcpAddr("17.99.29.16:8301"), + SerfAdvertiseAddrWAN: tcpAddr("78.63.37.19:8302"), + SerfBindAddrLAN: tcpAddr("99.43.63.15:8301"), + SerfBindAddrWAN: tcpAddr("67.88.33.19:8302"), + SessionTTLMin: 26627 * time.Second, + SkipLeaveOnInt: true, + StartJoinAddrsLAN: []string{"LR3hGDoG", "MwVpZ4Up"}, + StartJoinAddrsWAN: []string{"EbFSc3nA", "kwXTh623"}, + SyslogFacility: "hHv79Uia", + Telemetry: lib.TelemetryConfig{ + CirconusAPIApp: "p4QOTe9j", + CirconusAPIToken: "E3j35V23", + CirconusAPIURL: "mEMjHpGg", + CirconusBrokerID: "BHlxUhed", + CirconusBrokerSelectTag: "13xy1gHm", + CirconusCheckDisplayName: "DRSlQR6n", + CirconusCheckForceMetricActivation: "Ua5FGVYf", + CirconusCheckID: "kGorutad", + CirconusCheckInstanceID: "rwoOL6R4", + CirconusCheckSearchTag: "ovT4hT4f", + CirconusCheckTags: "prvO4uBl", + CirconusSubmissionInterval: "DolzaflP", + CirconusSubmissionURL: "gTcbS93G", + DisableHostname: true, + DogstatsdAddr: "0wSndumK", + DogstatsdTags: []string{"3N81zSUB", "Xtj8AnXZ"}, + FilterDefault: true, + AllowedPrefixes: []string{"oJotS8XJ"}, + BlockedPrefixes: []string{"cazlEhGn"}, + MetricsPrefix: "ftO6DySn", + PrometheusRetentionTime: 15 * time.Second, + StatsdAddr: "drce87cy", + StatsiteAddr: "HpFwKB8R", + }, + TLSCipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305, tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384}, + TLSMinVersion: "pAOWafkR", + TLSPreferServerCipherSuites: true, TaggedAddresses: map[string]string{ "7MYgHrYH": "dALJAhLD", "h6DdBy6K": "ebrr9zZ8", @@ -4399,29 +4402,30 @@ func TestSanitize(t *testing.T) { "TLSMinVersion": "", "TLSPreferServerCipherSuites": false, "TaggedAddresses": {}, - "TelemetryAllowedPrefixes": [], - "TelemetryBlockedPrefixes": [], - "TelemetryCirconusAPIApp": "", - "TelemetryCirconusAPIToken": "hidden", - "TelemetryCirconusAPIURL": "", - "TelemetryCirconusBrokerID": "", - "TelemetryCirconusBrokerSelectTag": "", - "TelemetryCirconusCheckDisplayName": "", - "TelemetryCirconusCheckForceMetricActivation": "", - "TelemetryCirconusCheckID": "", - "TelemetryCirconusCheckInstanceID": "", - "TelemetryCirconusCheckSearchTag": "", - "TelemetryCirconusCheckTags": "", - "TelemetryCirconusSubmissionInterval": "", - "TelemetryCirconusSubmissionURL": "", - "TelemetryDisableHostname": false, - "TelemetryDogstatsdAddr": "", - "TelemetryDogstatsdTags": [], - "TelemetryFilterDefault": false, - "TelemetryMetricsPrefix": "", - "TelemetryPrometheusRetentionTime": "0s", - "TelemetryStatsdAddr": "", - "TelemetryStatsiteAddr": "", + "Telemetry":{ + "AllowedPrefixes": [], + "BlockedPrefixes": [], + "CirconusAPIApp": "", + "CirconusAPIToken": "hidden", + "CirconusAPIURL": "", + "CirconusBrokerID": "", + "CirconusBrokerSelectTag": "", + "CirconusCheckDisplayName": "", + "CirconusCheckForceMetricActivation": "", + "CirconusCheckID": "", + "CirconusCheckInstanceID": "", + "CirconusCheckSearchTag": "", + "CirconusCheckTags": "", + "CirconusSubmissionInterval": "", + "CirconusSubmissionURL": "", + "DisableHostname": false, + "DogstatsdAddr": "", + "DogstatsdTags": [], + "FilterDefault": false, + "MetricsPrefix": "", + "PrometheusRetentionTime": "0s", + "StatsdAddr": "" + }, "TranslateWANAddrs": false, "UIDir": "", "UnixSocketGroup": "", @@ -4441,11 +4445,7 @@ func TestSanitize(t *testing.T) { if err != nil { t.Fatal(err) } - if got, want := string(b), rtJSON; got != want { - dmp := diffmatchpatch.New() - diffs := dmp.DiffMain(want, got, false) - t.Fatal(dmp.DiffPrettyText(diffs)) - } + require.JSONEq(t, rtJSON, string(b)) } func splitIPPort(hostport string) (net.IP, int) { diff --git a/command/agent/agent.go b/command/agent/agent.go index 05f54f2dfd..69a1093c9c 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -198,7 +198,7 @@ func (c *cmd) run(args []string) int { c.logOutput = logOutput c.logger = log.New(logOutput, "", log.LstdFlags) - memSink, err := lib.InitTelemetry(config.TelemetryConfig(false)) + memSink, err := lib.InitTelemetry(config.Telemetry) if err != nil { c.UI.Error(err.Error()) return 1 diff --git a/connect/proxy/config.go b/connect/proxy/config.go index d98c2d633d..9f187808b4 100644 --- a/connect/proxy/config.go +++ b/connect/proxy/config.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/connect" + "github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/watch" "github.com/hashicorp/hcl" ) @@ -40,7 +41,7 @@ type Config struct { // Telemetry stores configuration for go-metrics. It is typically populated // from the agent's runtime config via the proxy config endpoint so that the // proxy will log metrics to the same location(s) as the agent. - Telemetry map[string]interface{} + Telemetry lib.TelemetryConfig } // Service returns the *connect.Service structure represented by this config. @@ -265,8 +266,11 @@ func (w *AgentConfigWatcher) handler(blockVal watch.BlockingParamVal, ProxiedServiceNamespace: "default", } - if t, ok := resp.Config["telemetry"].(map[string]interface{}); ok { - cfg.Telemetry = t + if tRaw, ok := resp.Config["telemetry"]; ok { + err := mapstructure.Decode(tRaw, &cfg.Telemetry) + if err != nil { + w.logger.Printf("[WARN] proxy telemetry config failed to parse: %s", err) + } } // Unmarshal configs diff --git a/connect/proxy/config_test.go b/connect/proxy/config_test.go index 0f92bac38f..e4e0d7bbd5 100644 --- a/connect/proxy/config_test.go +++ b/connect/proxy/config_test.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/consul/agent" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/connect" + "github.com/hashicorp/consul/lib" "github.com/stretchr/testify/require" ) @@ -118,7 +119,14 @@ func TestUpstreamResolverFromClient(t *testing.T) { func TestAgentConfigWatcher(t *testing.T) { t.Parallel() - a := agent.NewTestAgent("agent_smith", "") + a := agent.NewTestAgent("agent_smith", ` + connect { + enabled = true + proxy { + allow_managed_api_registration = true + } + } + `) defer a.Shutdown() client := a.Client() @@ -175,9 +183,9 @@ func TestAgentConfigWatcher(t *testing.T) { ConnectTimeoutMs: 10000, // from applyDefaults }, }, - Telemetry: map[string]interface{}{ - "FilterDefault": true, - "MetricsPrefix": "consul.proxy.web", + Telemetry: lib.TelemetryConfig{ + FilterDefault: true, + MetricsPrefix: "consul.proxy.web", }, } diff --git a/connect/proxy/proxy.go b/connect/proxy/proxy.go index cef9b71265..ba2d3e8f11 100644 --- a/connect/proxy/proxy.go +++ b/connect/proxy/proxy.go @@ -46,12 +46,9 @@ func (p *Proxy) Serve() error { // Initial setup // Setup telemetry if configured - if len(newCfg.Telemetry) > 0 { - p.logger.Printf("[DEBUG] got Telemetry confg: %v", newCfg.Telemetry) - _, err := lib.InitTelemetry(newCfg.Telemetry) - if err != nil { - p.logger.Printf("[ERR] proxy telemetry config error: %s", err) - } + _, err := lib.InitTelemetry(newCfg.Telemetry) + if err != nil { + p.logger.Printf("[ERR] proxy telemetry config error: %s", err) } // Setup Service instance now we know target ID etc diff --git a/lib/telemetry.go b/lib/telemetry.go index 843e6c73a9..a335d6cc85 100644 --- a/lib/telemetry.go +++ b/lib/telemetry.go @@ -1,6 +1,7 @@ package lib import ( + "reflect" "time" metrics "github.com/armon/go-metrics" @@ -9,24 +10,252 @@ import ( "github.com/armon/go-metrics/prometheus" ) -func statsiteSink(cfg map[string]interface{}, hostname string) (metrics.MetricSink, error) { - addr := cfgStringVal(cfg["StatsiteAddr"]) +// TelemetryConfig is embedded in config.RuntimeConfig and holds the +// configuration variables for go-metrics. It is a separate struct to allow it +// to be exported as JSON and passed to other process like managed connect +// proxies so they can inherit the agent's telemetry config. +// +// It is in lib package rather than agent/config because we need to use it in +// the shared InitTelemetry functions below, but we can't import agent/config +// due to a dependency cycle. +type TelemetryConfig struct { + // Circonus*: see https://github.com/circonus-labs/circonus-gometrics + // for more details on the various configuration options. + // Valid configuration combinations: + // - CirconusAPIToken + // metric management enabled (search for existing check or create a new one) + // - CirconusSubmissionUrl + // metric management disabled (use check with specified submission_url, + // broker must be using a public SSL certificate) + // - CirconusAPIToken + CirconusCheckSubmissionURL + // metric management enabled (use check with specified submission_url) + // - CirconusAPIToken + CirconusCheckID + // metric management enabled (use check with specified id) + + // CirconusAPIApp is an app name associated with API token. + // Default: "consul" + // + // hcl: telemetry { circonus_api_app = string } + CirconusAPIApp string `json:"circonus_api_app,omitempty" mapstructure:"circonus_api_app"` + + // CirconusAPIToken is a valid API Token used to create/manage check. If provided, + // metric management is enabled. + // Default: none + // + // hcl: telemetry { circonus_api_token = string } + CirconusAPIToken string `json:"circonus_api_token,omitempty" mapstructure:"circonus_api_token"` + + // CirconusAPIURL is the base URL to use for contacting the Circonus API. + // Default: "https://api.circonus.com/v2" + // + // hcl: telemetry { circonus_api_url = string } + CirconusAPIURL string `json:"circonus_apiurl,omitempty" mapstructure:"circonus_apiurl"` + + // CirconusBrokerID is an explicit broker to use when creating a new check. The numeric portion + // of broker._cid. If metric management is enabled and neither a Submission URL nor Check ID + // is provided, an attempt will be made to search for an existing check using Instance ID and + // Search Tag. If one is not found, a new HTTPTRAP check will be created. + // Default: use Select Tag if provided, otherwise, a random Enterprise Broker associated + // with the specified API token or the default Circonus Broker. + // Default: none + // + // hcl: telemetry { circonus_broker_id = string } + CirconusBrokerID string `json:"circonus_broker_id,omitempty" mapstructure:"circonus_broker_id"` + + // CirconusBrokerSelectTag is a special tag which will be used to select a broker when + // a Broker ID is not provided. The best use of this is to as a hint for which broker + // should be used based on *where* this particular instance is running. + // (e.g. a specific geo location or datacenter, dc:sfo) + // Default: none + // + // hcl: telemetry { circonus_broker_select_tag = string } + CirconusBrokerSelectTag string `json:"circonus_broker_select_tag,omitempty" mapstructure:"circonus_broker_select_tag"` + + // CirconusCheckDisplayName is the name for the check which will be displayed in the Circonus UI. + // Default: value of CirconusCheckInstanceID + // + // hcl: telemetry { circonus_check_display_name = string } + CirconusCheckDisplayName string `json:"circonus_check_display_name,omitempty" mapstructure:"circonus_check_display_name"` + + // CirconusCheckForceMetricActivation will force enabling metrics, as they are encountered, + // if the metric already exists and is NOT active. If check management is enabled, the default + // behavior is to add new metrics as they are encountered. If the metric already exists in the + // check, it will *NOT* be activated. This setting overrides that behavior. + // Default: "false" + // + // hcl: telemetry { circonus_check_metrics_activation = (true|false) + CirconusCheckForceMetricActivation string `json:"circonus_check_force_metric_activation,omitempty" mapstructure:"circonus_check_force_metric_activation"` + + // CirconusCheckID is the check id (not check bundle id) from a previously created + // HTTPTRAP check. The numeric portion of the check._cid field. + // Default: none + // + // hcl: telemetry { circonus_check_id = string } + CirconusCheckID string `json:"circonus_check_id,omitempty" mapstructure:"circonus_check_id"` + + // CirconusCheckInstanceID serves to uniquely identify the metrics coming from this "instance". + // It can be used to maintain metric continuity with transient or ephemeral instances as + // they move around within an infrastructure. + // Default: hostname:app + // + // hcl: telemetry { circonus_check_instance_id = string } + CirconusCheckInstanceID string `json:"circonus_check_instance_id,omitempty" mapstructure:"circonus_check_instance_id"` + + // CirconusCheckSearchTag is a special tag which, when coupled with the instance id, helps to + // narrow down the search results when neither a Submission URL or Check ID is provided. + // Default: service:app (e.g. service:consul) + // + // hcl: telemetry { circonus_check_search_tag = string } + CirconusCheckSearchTag string `json:"circonus_check_search_tag,omitempty" mapstructure:"circonus_check_search_tag"` + + // CirconusCheckSearchTag is a special tag which, when coupled with the instance id, helps to + // narrow down the search results when neither a Submission URL or Check ID is provided. + // Default: service:app (e.g. service:consul) + // + // hcl: telemetry { circonus_check_tags = string } + CirconusCheckTags string `json:"circonus_check_tags,omitempty" mapstructure:"circonus_check_tags"` + + // CirconusSubmissionInterval is the interval at which metrics are submitted to Circonus. + // Default: 10s + // + // hcl: telemetry { circonus_submission_interval = "duration" } + CirconusSubmissionInterval string `json:"circonus_submission_interval,omitempty" mapstructure:"circonus_submission_interval"` + + // CirconusCheckSubmissionURL is the check.config.submission_url field from a + // previously created HTTPTRAP check. + // Default: none + // + // hcl: telemetry { circonus_submission_url = string } + CirconusSubmissionURL string `json:"circonus_submission_url,omitempty" mapstructure:"circonus_submission_url"` + + // DisableHostname will disable hostname prefixing for all metrics. + // + // hcl: telemetry { disable_hostname = (true|false) + DisableHostname bool `json:"disable_hostname,omitempty" mapstructure:"disable_hostname"` + + // DogStatsdAddr is the address of a dogstatsd instance. If provided, + // metrics will be sent to that instance + // + // hcl: telemetry { dogstatsd_addr = string } + DogstatsdAddr string `json:"dogstatsd_addr,omitempty" mapstructure:"dogstatsd_addr"` + + // DogStatsdTags are the global tags that should be sent with each packet to dogstatsd + // It is a list of strings, where each string looks like "my_tag_name:my_tag_value" + // + // hcl: telemetry { dogstatsd_tags = []string } + DogstatsdTags []string `json:"dogstatsd_tags,omitempty" mapstructure:"dogstatsd_tags"` + + // PrometheusRetentionTime is the retention time for prometheus metrics if greater than 0. + // A value of 0 disable Prometheus support. Regarding Prometheus, it is considered a good + // practice to put large values here (such as a few days), and at least the interval between + // prometheus requests. + // + // hcl: telemetry { prometheus_retention_time = "duration" } + PrometheusRetentionTime time.Duration `json:"prometheus_retention_time,omitempty" mapstructure:"prometheus_retention_time"` + + // FilterDefault is the default for whether to allow a metric that's not + // covered by the filter. + // + // hcl: telemetry { filter_default = (true|false) } + FilterDefault bool `json:"filter_default,omitempty" mapstructure:"filter_default"` + + // AllowedPrefixes is a list of filter rules to apply for allowing metrics + // by prefix. Use the 'prefix_filter' option and prefix rules with '+' to be + // included. + // + // hcl: telemetry { prefix_filter = []string{"+", "+", ...} } + AllowedPrefixes []string `json:"allowed_prefixes,omitempty" mapstructure:"allowed_prefixes"` + + // BlockedPrefixes is a list of filter rules to apply for blocking metrics + // by prefix. Use the 'prefix_filter' option and prefix rules with '-' to be + // excluded. + // + // hcl: telemetry { prefix_filter = []string{"-", "-", ...} } + BlockedPrefixes []string `json:"blocked_prefixes,omitempty" mapstructure:"blocked_prefixes"` + + // MetricsPrefix is the prefix used to write stats values to. + // Default: "consul." + // + // hcl: telemetry { metrics_prefix = string } + MetricsPrefix string `json:"metrics_prefix,omitempty" mapstructure:"metrics_prefix"` + + // StatsdAddr is the address of a statsd instance. If provided, + // metrics will be sent to that instance. + // + // hcl: telemetry { statsd_address = string } + StatsdAddr string `json:"statsd_address,omitempty" mapstructure:"statsd_address"` + + // StatsiteAddr is the address of a statsite instance. If provided, + // metrics will be streamed to that instance. + // + // hcl: telemetry { statsite_address = string } + StatsiteAddr string `json:"statsite_address,omitempty" mapstructure:"statsite_address"` +} + +// MergeDefaults copies any non-zero field from defaults into the current +// config. +func (c *TelemetryConfig) MergeDefaults(defaults *TelemetryConfig) { + if defaults == nil { + return + } + cfgPtrVal := reflect.ValueOf(c) + cfgVal := cfgPtrVal.Elem() + otherVal := reflect.ValueOf(*defaults) + for i := 0; i < cfgVal.NumField(); i++ { + f := cfgVal.Field(i) + if !f.IsValid() || !f.CanSet() { + continue + } + // See if the current value is a zero-value, if _not_ skip it + // + // No built in way to check for zero-values for all types so only + // implementing this for the types we actually have for now. Test failure + // should catch the case where we add new types later. + switch f.Kind() { + case reflect.Slice: + if !f.IsNil() { + continue + } + case reflect.Int, reflect.Int64: // time.Duration == int64 + if f.Int() != 0 { + continue + } + case reflect.String: + if f.String() != "" { + continue + } + case reflect.Bool: + if f.Bool() != false { + continue + } + default: + // Needs implementing, should be caught by tests. + continue + } + + // It's zero, copy it from defaults + f.Set(otherVal.Field(i)) + } +} + +func statsiteSink(cfg TelemetryConfig, hostname string) (metrics.MetricSink, error) { + addr := cfg.StatsiteAddr if addr == "" { return nil, nil } return metrics.NewStatsiteSink(addr) } -func statsdSink(cfg map[string]interface{}, hostname string) (metrics.MetricSink, error) { - addr := cfgStringVal(cfg["StatsdAddr"]) +func statsdSink(cfg TelemetryConfig, hostname string) (metrics.MetricSink, error) { + addr := cfg.StatsdAddr if addr == "" { return nil, nil } return metrics.NewStatsdSink(addr) } -func dogstatdSink(cfg map[string]interface{}, hostname string) (metrics.MetricSink, error) { - addr := cfgStringVal(cfg["DogstatsdAddr"]) +func dogstatdSink(cfg TelemetryConfig, hostname string) (metrics.MetricSink, error) { + addr := cfg.DogstatsdAddr if addr == "" { return nil, nil } @@ -34,16 +263,16 @@ func dogstatdSink(cfg map[string]interface{}, hostname string) (metrics.MetricSi if err != nil { return nil, err } - sink.SetTags(cfgStrSliceVal(cfg["DogstatsdTags"])) + sink.SetTags(cfg.DogstatsdTags) return sink, nil } -func prometheusSink(cfg map[string]interface{}, hostname string) (metrics.MetricSink, error) { - if cfgDurationVal(cfg["PrometheusRetentionTime"]).Nanoseconds() < 1 { +func prometheusSink(cfg TelemetryConfig, hostname string) (metrics.MetricSink, error) { + if cfg.PrometheusRetentionTime.Nanoseconds() < 1 { return nil, nil } prometheusOpts := prometheus.PrometheusOpts{ - Expiration: cfgDurationVal(cfg["PrometheusRetentionTime"]), + Expiration: cfg.PrometheusRetentionTime, } sink, err := prometheus.NewPrometheusSinkFrom(prometheusOpts) if err != nil { @@ -52,27 +281,27 @@ func prometheusSink(cfg map[string]interface{}, hostname string) (metrics.Metric return sink, nil } -func circonusSink(cfg map[string]interface{}, hostname string) (metrics.MetricSink, error) { - token := cfgStringVal(cfg["CirconusAPIToken"]) - url := cfgStringVal(cfg["CirconusSubmissionURL"]) +func circonusSink(cfg TelemetryConfig, hostname string) (metrics.MetricSink, error) { + token := cfg.CirconusAPIToken + url := cfg.CirconusSubmissionURL if token == "" && url == "" { return nil, nil } conf := &circonus.Config{} - conf.Interval = cfgStringVal(cfg["CirconusSubmissionInterval"]) + conf.Interval = cfg.CirconusSubmissionInterval conf.CheckManager.API.TokenKey = token - conf.CheckManager.API.TokenApp = cfgStringVal(cfg["CirconusAPIApp"]) - conf.CheckManager.API.URL = cfgStringVal(cfg["CirconusAPIURL"]) + conf.CheckManager.API.TokenApp = cfg.CirconusAPIApp + conf.CheckManager.API.URL = cfg.CirconusAPIURL conf.CheckManager.Check.SubmissionURL = url - conf.CheckManager.Check.ID = cfgStringVal(cfg["CirconusCheckID"]) - conf.CheckManager.Check.ForceMetricActivation = cfgStringVal(cfg["CirconusCheckForceMetricActivation"]) - conf.CheckManager.Check.InstanceID = cfgStringVal(cfg["CirconusCheckInstanceID"]) - conf.CheckManager.Check.SearchTag = cfgStringVal(cfg["CirconusCheckSearchTag"]) - conf.CheckManager.Check.DisplayName = cfgStringVal(cfg["CirconusCheckDisplayName"]) - conf.CheckManager.Check.Tags = cfgStringVal(cfg["CirconusCheckTags"]) - conf.CheckManager.Broker.ID = cfgStringVal(cfg["CirconusBrokerID"]) - conf.CheckManager.Broker.SelectTag = cfgStringVal(cfg["CirconusBrokerSelectTag"]) + conf.CheckManager.Check.ID = cfg.CirconusCheckID + conf.CheckManager.Check.ForceMetricActivation = cfg.CirconusCheckForceMetricActivation + conf.CheckManager.Check.InstanceID = cfg.CirconusCheckInstanceID + conf.CheckManager.Check.SearchTag = cfg.CirconusCheckSearchTag + conf.CheckManager.Check.DisplayName = cfg.CirconusCheckDisplayName + conf.CheckManager.Check.Tags = cfg.CirconusCheckTags + conf.CheckManager.Broker.ID = cfg.CirconusBrokerID + conf.CheckManager.Broker.SelectTag = cfg.CirconusBrokerSelectTag if conf.CheckManager.Check.DisplayName == "" { conf.CheckManager.Check.DisplayName = "Consul" @@ -94,51 +323,22 @@ func circonusSink(cfg map[string]interface{}, hostname string) (metrics.MetricSi return sink, nil } -func cfgStringVal(i interface{}) string { - v, ok := i.(string) - if ok { - return v - } - return "" -} -func cfgBoolVal(i interface{}) bool { - v, ok := i.(bool) - if ok { - return v - } - return false -} -func cfgDurationVal(i interface{}) time.Duration { - v, ok := i.(time.Duration) - if ok { - return v - } - return time.Duration(0) -} -func cfgStrSliceVal(i interface{}) []string { - v, ok := i.([]string) - if ok { - return v - } - return nil -} - // InitTelemetry configures go-metrics based on map of telemetry config -// values as returned by RuntimecfgStringVal(cfg["Config"])(). -func InitTelemetry(cfg map[string]interface{}) (*metrics.InmemSink, error) { +// values as returned by Runtimecfg.Config(). +func InitTelemetry(cfg TelemetryConfig) (*metrics.InmemSink, error) { // Setup telemetry // Aggregate on 10 second intervals for 1 minute. Expose the // metrics over stderr when there is a SIGUSR1 received. memSink := metrics.NewInmemSink(10*time.Second, time.Minute) metrics.DefaultInmemSignal(memSink) - metricsConf := metrics.DefaultConfig(cfgStringVal(cfg["MetricsPrefix"])) - metricsConf.EnableHostname = !cfgBoolVal(cfg["DisableHostname"]) - metricsConf.FilterDefault = cfgBoolVal(cfg["FilterDefault"]) - metricsConf.AllowedPrefixes = cfgStrSliceVal(cfg["AllowedPrefixes"]) - metricsConf.BlockedPrefixes = cfgStrSliceVal(cfg["BlockedPrefixes"]) + metricsConf := metrics.DefaultConfig(cfg.MetricsPrefix) + metricsConf.EnableHostname = !cfg.DisableHostname + metricsConf.FilterDefault = cfg.FilterDefault + metricsConf.AllowedPrefixes = cfg.AllowedPrefixes + metricsConf.BlockedPrefixes = cfg.BlockedPrefixes var sinks metrics.FanoutSink - addSink := func(name string, fn func(map[string]interface{}, string) (metrics.MetricSink, error)) error { + addSink := func(name string, fn func(TelemetryConfig, string) (metrics.MetricSink, error)) error { s, err := fn(cfg, metricsConf.HostName) if err != nil { return err diff --git a/lib/telemetry_test.go b/lib/telemetry_test.go new file mode 100644 index 0000000000..f81b7b5c1a --- /dev/null +++ b/lib/telemetry_test.go @@ -0,0 +1,90 @@ +package lib + +import ( + "reflect" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func makeFullTelemetryConfig(t *testing.T) TelemetryConfig { + var ( + strSliceVal = []string{"foo"} + strVal = "foo" + intVal = int64(1 * time.Second) + ) + + cfg := TelemetryConfig{} + cfgP := reflect.ValueOf(&cfg) + cfgV := cfgP.Elem() + for i := 0; i < cfgV.NumField(); i++ { + f := cfgV.Field(i) + if !f.IsValid() || !f.CanSet() { + continue + } + // Set non-zero values for all fields. We only implement kinds that exist + // now for brevity but will fail the test if a new field type is added since + // this is likely not implemented in MergeDefaults either. + switch f.Kind() { + case reflect.Slice: + if f.Type() != reflect.TypeOf(strSliceVal) { + t.Fatalf("unknown slice type in TelemetryConfig." + + " You need to update MergeDefaults and this test code.") + } + f.Set(reflect.ValueOf(strSliceVal)) + case reflect.Int, reflect.Int64: // time.Duration == int64 + f.SetInt(intVal) + case reflect.String: + f.SetString(strVal) + case reflect.Bool: + f.SetBool(true) + default: + t.Fatalf("unknown field type in TelemetryConfig" + + " You need to update MergeDefaults and this test code.") + } + } + return cfg +} + +func TestTelemetryConfig_MergeDefaults(t *testing.T) { + tests := []struct { + name string + cfg TelemetryConfig + defaults TelemetryConfig + want TelemetryConfig + }{ + { + name: "basic merge", + cfg: TelemetryConfig{ + StatsiteAddr: "stats.it:4321", + }, + defaults: TelemetryConfig{ + StatsdAddr: "localhost:5678", + StatsiteAddr: "localhost:1234", + }, + want: TelemetryConfig{ + StatsdAddr: "localhost:5678", + StatsiteAddr: "stats.it:4321", + }, + }, + { + // This test uses reflect to build a TelemetryConfig with every value set + // to ensure that we exercise every possible field type. This means that + // if new fields are added that are not supported types in the code, this + // test should either ensure they work or fail to build the test case and + // fail the test. + name: "exhaustive", + cfg: TelemetryConfig{}, + defaults: makeFullTelemetryConfig(t), + want: makeFullTelemetryConfig(t), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := tt.cfg + c.MergeDefaults(&tt.defaults) + require.Equal(t, tt.want, c) + }) + } +}