From b0cba2ec0329a8442ff31023e8e2f418daf592a8 Mon Sep 17 00:00:00 2001 From: FFMMM Date: Fri, 1 Apr 2022 10:35:56 -0700 Subject: [PATCH] mark disable_compat_1.9 to deprecate in 1.13, change default to true (#12675) Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com> --- .changelog/12675.txt | 3 + agent/agent.go | 4 +- agent/config/builder.go | 2 +- agent/http.go | 3 +- agent/metrics_test.go | 61 +++++++++++++++++++ website/content/docs/agent/options.mdx | 2 +- .../docs/upgrading/upgrade-specific.mdx | 14 ++++- 7 files changed, 82 insertions(+), 7 deletions(-) create mode 100644 .changelog/12675.txt diff --git a/.changelog/12675.txt b/.changelog/12675.txt new file mode 100644 index 000000000..0f46bb5a9 --- /dev/null +++ b/.changelog/12675.txt @@ -0,0 +1,3 @@ +```release-note:breaking-change +telemetry: the disable_compat_1.9 option now defaults to true. 1.9 style `consul.http...` metrics can still be enabled by setting `disable_compat_1.9 = false`. However, we will remove these metrics in 1.13. +``` \ No newline at end of file diff --git a/agent/agent.go b/agent/agent.go index fef53c0f2..7a313cb4f 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -698,7 +698,7 @@ func (a *Agent) Start(ctx context.Context) error { // DEPRECATED: Warn users if they're emitting deprecated metrics. Remove this warning and the flagged metrics in a // future release of Consul. if !a.config.Telemetry.DisableCompatOneNine { - a.logger.Warn("DEPRECATED Backwards compatibility with pre-1.9 metrics enabled. These metrics will be removed in a future version of Consul. Set `telemetry { disable_compat_1.9 = true }` to disable them.") + a.logger.Warn("DEPRECATED Backwards compatibility with pre-1.9 metrics enabled. These metrics will be removed in Consul 1.13. Consider not using this flag and rework instrumentation for 1.10 style http metrics.") } if a.tlsConfigurator.Cert() != nil { @@ -3797,7 +3797,7 @@ func (a *Agent) reloadConfig(autoReload bool) error { // DEPRECATED: Warn users on reload if they're emitting deprecated metrics. Remove this warning and the flagged // metrics in a future release of Consul. if !a.config.Telemetry.DisableCompatOneNine { - a.logger.Warn("DEPRECATED Backwards compatibility with pre-1.9 metrics enabled. These metrics will be removed in a future version of Consul. Set `telemetry { disable_compat_1.9 = true }` to disable them.") + a.logger.Warn("DEPRECATED Backwards compatibility with pre-1.9 metrics enabled. These metrics will be removed in Consul 1.13. Consider not using this flag and rework instrumentation for 1.10 style http metrics.") } return a.reloadConfigInternal(newCfg) diff --git a/agent/config/builder.go b/agent/config/builder.go index 1762f3f6d..b3bdc46f3 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -910,7 +910,7 @@ func (b *builder) build() (rt RuntimeConfig, err error) { CirconusCheckTags: stringVal(c.Telemetry.CirconusCheckTags), CirconusSubmissionInterval: stringVal(c.Telemetry.CirconusSubmissionInterval), CirconusSubmissionURL: stringVal(c.Telemetry.CirconusSubmissionURL), - DisableCompatOneNine: boolVal(c.Telemetry.DisableCompatOneNine), + DisableCompatOneNine: boolValWithDefault(c.Telemetry.DisableCompatOneNine, true), DisableHostname: boolVal(c.Telemetry.DisableHostname), DogstatsdAddr: stringVal(c.Telemetry.DogstatsdAddr), DogstatsdTags: c.Telemetry.DogstatsdTags, diff --git a/agent/http.go b/agent/http.go index ba0067b62..16a3a2150 100644 --- a/agent/http.go +++ b/agent/http.go @@ -228,8 +228,7 @@ func (s *HTTPHandlers) handler(enableDebug bool) http.Handler { labels := []metrics.Label{{Name: "method", Value: req.Method}, {Name: "path", Value: path_label}} metrics.MeasureSinceWithLabels([]string{"api", "http"}, start, labels) - // DEPRECATED Emit pre-1.9 metric as `consul.http...` to maintain backwards compatibility. Enabled by - // default. Users may set `telemetry { disable_compat_1.9 = true }` + // DEPRECATED Emit pre-1.9 metric as `consul.http...`. This will be removed in 1.13. if !s.agent.config.Telemetry.DisableCompatOneNine { key := append([]string{"http", req.Method}, parts...) metrics.MeasureSince(key, start) diff --git a/agent/metrics_test.go b/agent/metrics_test.go index 5bbc7de23..2aedc0180 100644 --- a/agent/metrics_test.go +++ b/agent/metrics_test.go @@ -255,6 +255,67 @@ func TestHTTPHandlers_AgentMetrics_ConsulAutopilot_Prometheus(t *testing.T) { }) } +// TestHTTPHandlers_AgentMetrics_Disable1Dot9MetricsChange adds testing around the 1.9 style metrics +// https://www.consul.io/docs/agent/options#telemetry-disable_compat_1.9 +func TestHTTPHandlers_AgentMetrics_Disable1Dot9MetricsChange(t *testing.T) { + skipIfShortTesting(t) + // This test cannot use t.Parallel() since we modify global state, ie the global metrics instance + + // 1.9 style http metrics looked like this: + // agent_http_2_http_GET_v1_agent_members{quantile="0.5"} 0.1329520046710968 + t.Run("check that no consul.http metrics are emitted by default", func(t *testing.T) { + hcl := ` + telemetry = { + prometheus_retention_time = "5s" + disable_hostname = true + metrics_prefix = "agent_http" + } + ` + + a := StartTestAgent(t, TestAgent{HCL: hcl}) + defer a.Shutdown() + + // we have to use the `a.srv.handler()` to actually trigger the wrapped function + uri := fmt.Sprintf("http://%s%s", a.HTTPAddr(), "/v1/agent/members") + req, err := http.NewRequest("GET", uri, nil) + require.NoError(t, err) + resp := httptest.NewRecorder() + handler := a.srv.handler(true) + handler.ServeHTTP(resp, req) + + respRec := httptest.NewRecorder() + recordPromMetrics(t, a, respRec) + + assertMetricNotExists(t, respRec, "agent_http_http_GET_v1_agent_members") + }) + + t.Run("check that we can still turn on consul.http metrics", func(t *testing.T) { + hcl := ` + telemetry = { + prometheus_retention_time = "5s", + disable_compat_1.9 = false + metrics_prefix = "agent_http_2" + } + ` + + a := StartTestAgent(t, TestAgent{HCL: hcl}) + defer a.Shutdown() + + uri := fmt.Sprintf("http://%s%s", a.HTTPAddr(), "/v1/agent/members") + req, err := http.NewRequest("GET", uri, nil) + require.NoError(t, err) + resp := httptest.NewRecorder() + + handler := a.srv.handler(true) + handler.ServeHTTP(resp, req) + + respRec := httptest.NewRecorder() + recordPromMetrics(t, a, respRec) + + assertMetricExists(t, respRec, "agent_http_2_http_GET_v1_agent_members") + }) +} + func TestHTTPHandlers_AgentMetrics_TLSCertExpiry_Prometheus(t *testing.T) { skipIfShortTesting(t) // This test cannot use t.Parallel() since we modify global state, ie the global metrics instance diff --git a/website/content/docs/agent/options.mdx b/website/content/docs/agent/options.mdx index 2b1c1b867..696d8bc88 100644 --- a/website/content/docs/agent/options.mdx +++ b/website/content/docs/agent/options.mdx @@ -2175,7 +2175,7 @@ There are also a number of common configuration options supported by all provide geo location or datacenter, dc:sfo). By default, this is left blank and not used. - `disable_compat_1.9` ((#telemetry-disable_compat_1.9)) - This allows users to disable metrics deprecated in 1.9 so they are no longer emitted, saving on performance and storage in large deployments. Defaults to false. + This allows users to disable metrics deprecated in 1.9 so they are no longer emitted, improving performance and reducing storage in large deployments. As of 1.12 this defaults to `true` and will be removed, along with 1.9 style http metrics in 1.13. - `disable_hostname` ((#telemetry-disable_hostname)) This controls whether or not to prepend runtime telemetry with the machine's diff --git a/website/content/docs/upgrading/upgrade-specific.mdx b/website/content/docs/upgrading/upgrade-specific.mdx index e7284dd38..b3fb7477f 100644 --- a/website/content/docs/upgrading/upgrade-specific.mdx +++ b/website/content/docs/upgrading/upgrade-specific.mdx @@ -14,12 +14,24 @@ provided for their upgrades as a result of new features or changed behavior. This page is used to document those details separately from the standard upgrade flow. +## Consul 1.12.0 + +### 1.9 Telemetry Compatibility + +#### Changing the default behavior for option + +The [`disable_compat_19`](/docs/agent/options#telemetry-disable_compat_1.9) telemetry configuration option now defaults +to `true`. In prior Consul versions (1.10.x through 1.11.x), the config defaulted to `false`. If you require 1.9 style +`consul.http...` metrics, you may enable them by setting the flag to `false`. However, be advised that these metrics, as +well as the flag will be removed in upcoming Consul 1.13. We recommend changing your instrumentation to use 1.10 and later +style `consul.api.http...` metrics and removing the configuration flag from your setup. + ## Consul 1.11.0 ### 1.10 Compatibility Consul Enterprise versions 1.10.0 through 1.10.4 contain a latent bug that causes those client or server agents to deregister their own services or health -checks when some of the servers have been upgraded to 1.11. Before upgrading Consul Enterprise servers to 1.11, all Consul agents should first +checks when some of the servers have been upgraded to 1.11. Before upgrading Consul Enterprise servers to 1.11, all Consul agents should first be upgraded to 1.10.7 or higher to ensure forward compatibility and prevent flapping of catalog registrations.