From a7495f711b1562b87789726e801c1cce0c8b699c Mon Sep 17 00:00:00 2001 From: Hamilton Turner Date: Sat, 29 Feb 2020 12:48:04 -0500 Subject: [PATCH 1/5] fix typo --- docs/content/https/acme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/https/acme.md b/docs/content/https/acme.md index 81a3db668..9ed02473f 100644 --- a/docs/content/https/acme.md +++ b/docs/content/https/acme.md @@ -191,7 +191,7 @@ Use the `HTTP-01` challenge to generate and renew ACME certificates by provision As described on the Let's Encrypt [community forum](https://community.letsencrypt.org/t/support-for-ports-other-than-80-and-443/3419/72), when using the `HTTP-01` challenge, `certificatesResolvers.myresolver.acme.httpChallenge.entryPoint` must be reachable by Let's Encrypt through port 80. -??? example "Using an EntryPoint Called http for the `httpChallenge`" +??? example "Using an EntryPoint Called web for the `httpChallenge`" ```toml tab="File (TOML)" [entryPoints] From 353bd3d06fa093c76d51982b657eb920511e9839 Mon Sep 17 00:00:00 2001 From: robotte Date: Tue, 3 Mar 2020 16:20:05 +0100 Subject: [PATCH 2/5] Added support for replacement containing escaped characters Co-authored-by: Ludovic Fernandez --- pkg/middlewares/replacepath/replace_path.go | 19 +++- .../replacepath/replace_path_test.go | 86 +++++++++++++---- .../replacepathregex/replace_path_regex.go | 28 +++++- .../replace_path_regex_test.go | 93 ++++++++++++++----- 4 files changed, 180 insertions(+), 46 deletions(-) diff --git a/pkg/middlewares/replacepath/replace_path.go b/pkg/middlewares/replacepath/replace_path.go index bec16827d..13dd2d98e 100644 --- a/pkg/middlewares/replacepath/replace_path.go +++ b/pkg/middlewares/replacepath/replace_path.go @@ -3,6 +3,7 @@ package replacepath import ( "context" "net/http" + "net/url" "github.com/containous/traefik/v2/pkg/config/dynamic" "github.com/containous/traefik/v2/pkg/log" @@ -40,8 +41,22 @@ func (r *replacePath) GetTracingInformation() (string, ext.SpanKindEnum) { } func (r *replacePath) ServeHTTP(rw http.ResponseWriter, req *http.Request) { - req.Header.Add(ReplacedPathHeader, req.URL.Path) - req.URL.Path = r.path + if req.URL.RawPath == "" { + req.Header.Add(ReplacedPathHeader, req.URL.Path) + } else { + req.Header.Add(ReplacedPathHeader, req.URL.RawPath) + } + + req.URL.RawPath = r.path + + var err error + req.URL.Path, err = url.PathUnescape(req.URL.RawPath) + if err != nil { + log.FromContext(middlewares.GetLoggerCtx(context.Background(), r.name, typeName)).Error(err) + http.Error(rw, err.Error(), http.StatusInternalServerError) + return + } + req.RequestURI = req.URL.RequestURI() r.next.ServeHTTP(rw, req) diff --git a/pkg/middlewares/replacepath/replace_path_test.go b/pkg/middlewares/replacepath/replace_path_test.go index 65824dcde..2042f3586 100644 --- a/pkg/middlewares/replacepath/replace_path_test.go +++ b/pkg/middlewares/replacepath/replace_path_test.go @@ -3,43 +3,93 @@ package replacepath import ( "context" "net/http" + "net/http/httptest" "testing" "github.com/containous/traefik/v2/pkg/config/dynamic" - "github.com/containous/traefik/v2/pkg/testhelpers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestReplacePath(t *testing.T) { - var replacementConfig = dynamic.ReplacePath{ - Path: "/replacement-path", + testCases := []struct { + desc string + path string + config dynamic.ReplacePath + expectedPath string + expectedRawPath string + expectedHeader string + }{ + { + desc: "simple path", + path: "/example", + config: dynamic.ReplacePath{ + Path: "/replacement-path", + }, + expectedPath: "/replacement-path", + expectedRawPath: "", + expectedHeader: "/example", + }, + { + desc: "long path", + path: "/some/really/long/path", + config: dynamic.ReplacePath{ + Path: "/replacement-path", + }, + expectedPath: "/replacement-path", + expectedRawPath: "", + expectedHeader: "/some/really/long/path", + }, + { + desc: "path with escaped value", + path: "/foo%2Fbar", + config: dynamic.ReplacePath{ + Path: "/replacement-path", + }, + expectedPath: "/replacement-path", + expectedRawPath: "", + expectedHeader: "/foo%2Fbar", + }, + { + desc: "replacement with escaped value", + path: "/path", + config: dynamic.ReplacePath{ + Path: "/foo%2Fbar", + }, + expectedPath: "/foo/bar", + expectedRawPath: "/foo%2Fbar", + expectedHeader: "/path", + }, } - paths := []string{ - "/example", - "/some/really/long/path", - } - - for _, path := range paths { - t.Run(path, func(t *testing.T) { - var expectedPath, actualHeader, requestURI string + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + var actualPath, actualRawPath, actualHeader, requestURI string next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - expectedPath = r.URL.Path + actualPath = r.URL.Path + actualRawPath = r.URL.RawPath actualHeader = r.Header.Get(ReplacedPathHeader) requestURI = r.RequestURI }) - handler, err := New(context.Background(), next, replacementConfig, "foo-replace-path") + handler, err := New(context.Background(), next, test.config, "foo-replace-path") require.NoError(t, err) - req := testhelpers.MustNewRequest(http.MethodGet, "http://localhost"+path, nil) + server := httptest.NewServer(handler) + defer server.Close() - handler.ServeHTTP(nil, req) + resp, err := http.Get(server.URL + test.path) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) - assert.Equal(t, expectedPath, replacementConfig.Path, "Unexpected path.") - assert.Equal(t, path, actualHeader, "Unexpected '%s' header.", ReplacedPathHeader) - assert.Equal(t, expectedPath, requestURI, "Unexpected request URI.") + assert.Equal(t, test.expectedPath, actualPath, "Unexpected path.") + assert.Equal(t, test.expectedHeader, actualHeader, "Unexpected '%s' header.", ReplacedPathHeader) + + if actualRawPath == "" { + assert.Equal(t, actualPath, requestURI, "Unexpected request URI.") + } else { + assert.Equal(t, actualRawPath, requestURI, "Unexpected request URI.") + } }) } } diff --git a/pkg/middlewares/replacepathregex/replace_path_regex.go b/pkg/middlewares/replacepathregex/replace_path_regex.go index c23820fbd..9aa755455 100644 --- a/pkg/middlewares/replacepathregex/replace_path_regex.go +++ b/pkg/middlewares/replacepathregex/replace_path_regex.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "net/url" "regexp" "strings" @@ -49,10 +50,31 @@ func (rp *replacePathRegex) GetTracingInformation() (string, ext.SpanKindEnum) { } func (rp *replacePathRegex) ServeHTTP(rw http.ResponseWriter, req *http.Request) { - if rp.regexp != nil && len(rp.replacement) > 0 && rp.regexp.MatchString(req.URL.Path) { - req.Header.Add(replacepath.ReplacedPathHeader, req.URL.Path) - req.URL.Path = rp.regexp.ReplaceAllString(req.URL.Path, rp.replacement) + var currentPath string + if req.URL.RawPath == "" { + currentPath = req.URL.Path + } else { + currentPath = req.URL.RawPath + } + + if rp.regexp != nil && len(rp.replacement) > 0 && rp.regexp.MatchString(currentPath) { + req.Header.Add(replacepath.ReplacedPathHeader, currentPath) + + req.URL.RawPath = rp.regexp.ReplaceAllString(currentPath, rp.replacement) + + // as replacement can introduce escaped characters + // Path must remain an unescaped version of RawPath + // Doesn't handle multiple times encoded replacement (`/` => `%2F` => `%252F` => ...) + var err error + req.URL.Path, err = url.PathUnescape(req.URL.RawPath) + if err != nil { + log.FromContext(middlewares.GetLoggerCtx(context.Background(), rp.name, typeName)).Error(err) + http.Error(rw, err.Error(), http.StatusInternalServerError) + return + } + req.RequestURI = req.URL.RequestURI() } + rp.next.ServeHTTP(rw, req) } diff --git a/pkg/middlewares/replacepathregex/replace_path_regex_test.go b/pkg/middlewares/replacepathregex/replace_path_regex_test.go index 8215df968..594e4eabd 100644 --- a/pkg/middlewares/replacepathregex/replace_path_regex_test.go +++ b/pkg/middlewares/replacepathregex/replace_path_regex_test.go @@ -3,23 +3,24 @@ package replacepathregex import ( "context" "net/http" + "net/http/httptest" "testing" "github.com/containous/traefik/v2/pkg/config/dynamic" "github.com/containous/traefik/v2/pkg/middlewares/replacepath" - "github.com/containous/traefik/v2/pkg/testhelpers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestReplacePathRegex(t *testing.T) { testCases := []struct { - desc string - path string - config dynamic.ReplacePathRegex - expectedPath string - expectedHeader string - expectsError bool + desc string + path string + config dynamic.ReplacePathRegex + expectedPath string + expectedRawPath string + expectedHeader string + expectsError bool }{ { desc: "simple regex", @@ -28,8 +29,9 @@ func TestReplacePathRegex(t *testing.T) { Replacement: "/who-am-i/$1", Regex: `^/whoami/(.*)`, }, - expectedPath: "/who-am-i/and/whoami", - expectedHeader: "/whoami/and/whoami", + expectedPath: "/who-am-i/and/whoami", + expectedRawPath: "/who-am-i/and/whoami", + expectedHeader: "/whoami/and/whoami", }, { desc: "simple replace (no regex)", @@ -38,8 +40,9 @@ func TestReplacePathRegex(t *testing.T) { Replacement: "/who-am-i", Regex: `/whoami`, }, - expectedPath: "/who-am-i/and/who-am-i", - expectedHeader: "/whoami/and/whoami", + expectedPath: "/who-am-i/and/who-am-i", + expectedRawPath: "/who-am-i/and/who-am-i", + expectedHeader: "/whoami/and/whoami", }, { desc: "no match", @@ -57,8 +60,9 @@ func TestReplacePathRegex(t *testing.T) { Replacement: "/downloads/$1-$2", Regex: `^(?i)/downloads/([^/]+)/([^/]+)$`, }, - expectedPath: "/downloads/src-source.go", - expectedHeader: "/downloads/src/source.go", + expectedPath: "/downloads/src-source.go", + expectedRawPath: "/downloads/src-source.go", + expectedHeader: "/downloads/src/source.go", }, { desc: "invalid regular expression", @@ -70,13 +74,46 @@ func TestReplacePathRegex(t *testing.T) { expectedPath: "/invalid/regexp/test", expectsError: true, }, + { + desc: "replacement with escaped char", + path: "/aaa/bbb", + config: dynamic.ReplacePathRegex{ + Replacement: "/foo%2Fbar", + Regex: `/aaa/bbb`, + }, + expectedPath: "/foo/bar", + expectedRawPath: "/foo%2Fbar", + expectedHeader: "/aaa/bbb", + }, + { + desc: "path and regex with escaped char", + path: "/aaa%2Fbbb", + config: dynamic.ReplacePathRegex{ + Replacement: "/foo/bar", + Regex: `/aaa%2Fbbb`, + }, + expectedPath: "/foo/bar", + expectedRawPath: "/foo/bar", + expectedHeader: "/aaa%2Fbbb", + }, + { + desc: "path with escaped char (no match)", + path: "/aaa%2Fbbb", + config: dynamic.ReplacePathRegex{ + Replacement: "/foo/bar", + Regex: `/aaa/bbb`, + }, + expectedPath: "/aaa/bbb", + expectedRawPath: "/aaa%2Fbbb", + }, } for _, test := range testCases { t.Run(test.desc, func(t *testing.T) { - var actualPath, actualHeader, requestURI string + var actualPath, actualRawPath, actualHeader, requestURI string next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { actualPath = r.URL.Path + actualRawPath = r.URL.RawPath actualHeader = r.Header.Get(replacepath.ReplacedPathHeader) requestURI = r.RequestURI }) @@ -84,19 +121,29 @@ func TestReplacePathRegex(t *testing.T) { handler, err := New(context.Background(), next, test.config, "foo-replace-path-regexp") if test.expectsError { require.Error(t, err) - } else { - require.NoError(t, err) + return + } - req := testhelpers.MustNewRequest(http.MethodGet, "http://localhost"+test.path, nil) - req.RequestURI = test.path + require.NoError(t, err) - handler.ServeHTTP(nil, req) + server := httptest.NewServer(handler) + defer server.Close() - assert.Equal(t, test.expectedPath, actualPath, "Unexpected path.") + resp, err := http.Get(server.URL + test.path) + require.NoError(t, err, "Unexpected error while making test request") + require.Equal(t, http.StatusOK, resp.StatusCode) + + assert.Equal(t, test.expectedPath, actualPath, "Unexpected path.") + assert.Equal(t, test.expectedRawPath, actualRawPath, "Unexpected raw path.") + + if actualRawPath == "" { assert.Equal(t, actualPath, requestURI, "Unexpected request URI.") - if test.expectedHeader != "" { - assert.Equal(t, test.expectedHeader, actualHeader, "Unexpected '%s' header.", replacepath.ReplacedPathHeader) - } + } else { + assert.Equal(t, actualRawPath, requestURI, "Unexpected request URI.") + } + + if test.expectedHeader != "" { + assert.Equal(t, test.expectedHeader, actualHeader, "Unexpected '%s' header.", replacepath.ReplacedPathHeader) } }) } From 5fdec48854bb2cbbc820bda953708a70d6727cf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ole=20R=C3=B6=C3=9Fner?= Date: Wed, 4 Mar 2020 13:24:05 +0100 Subject: [PATCH 3/5] Added wildcard ACME example --- docs/content/https/include-acme-multiple-domains-example.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/content/https/include-acme-multiple-domains-example.md b/docs/content/https/include-acme-multiple-domains-example.md index 2a628e035..8e92627f3 100644 --- a/docs/content/https/include-acme-multiple-domains-example.md +++ b/docs/content/https/include-acme-multiple-domains-example.md @@ -37,6 +37,10 @@ spec: port: 8080 tls: certResolver: myresolver + domains: + - main: company.org + sans: + - *.company.org ``` ```json tab="Marathon" From dccc075f2c12bc8053085ee07eb58a488a733d85 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Wed, 4 Mar 2020 16:48:05 +0100 Subject: [PATCH 4/5] Add some missing doc. --- docs/content/https/acme.md | 17 +++++++------ docs/content/providers/docker.md | 24 +++++++++++++++++++ .../reference/static-configuration/cli-ref.md | 2 +- .../reference/static-configuration/env-ref.md | 2 +- pkg/provider/docker/docker.go | 2 +- 5 files changed, 37 insertions(+), 10 deletions(-) diff --git a/docs/content/https/acme.md b/docs/content/https/acme.md index 9ed02473f..b4c4aa50e 100644 --- a/docs/content/https/acme.md +++ b/docs/content/https/acme.md @@ -396,6 +396,13 @@ As described in [Let's Encrypt's post](https://community.letsencrypt.org/t/stagi ### `caServer` +_Required, Default="https://acme-v02.api.letsencrypt.org/directory"_ + +The CA server to use: + +- Let's Encrypt production server: https://acme-v02.api.letsencrypt.org/directory +- Let's Encrypt staging server: https://acme-staging-v02.api.letsencrypt.org/directory + ??? example "Using the Let's Encrypt staging server" ```toml tab="File (TOML)" @@ -422,6 +429,8 @@ As described in [Let's Encrypt's post](https://community.letsencrypt.org/t/stagi ### `storage` +_Required, Default="acme.json"_ + The `storage` option sets the location where your ACME certificates are saved to. ```toml tab="File (TOML)" @@ -446,13 +455,7 @@ certificatesResolvers: # ... ``` -The value can refer to some kinds of storage: - -- a JSON file - -#### In a File - -ACME certificates can be stored in a JSON file that needs to have a `600` file mode . +ACME certificates are stored in a JSON file that needs to have a `600` file mode. In Docker you can mount either the JSON file, or the folder containing it: diff --git a/docs/content/providers/docker.md b/docs/content/providers/docker.md index 5151872a0..fc0e6058d 100644 --- a/docs/content/providers/docker.md +++ b/docs/content/providers/docker.md @@ -452,6 +452,30 @@ providers: Defines the polling interval (in seconds) in Swarm Mode. +### `watch` + +_Optional, Default=true_ + +```toml tab="File (TOML)" +[providers.docker] + watch = false + # ... +``` + +```yaml tab="File (YAML)" +providers: + docker: + watch: false + # ... +``` + +```bash tab="CLI" +--providers.docker.watch=false +# ... +``` + +Watch Docker Swarm events. + ### `constraints` _Optional, Default=""_ diff --git a/docs/content/reference/static-configuration/cli-ref.md b/docs/content/reference/static-configuration/cli-ref.md index 29c04ca27..7bfd845a1 100644 --- a/docs/content/reference/static-configuration/cli-ref.md +++ b/docs/content/reference/static-configuration/cli-ref.md @@ -346,7 +346,7 @@ TLS key Use the ip address from the bound port, rather than from the inner network. (Default: ```false```) `--providers.docker.watch`: -Watch provider. (Default: ```true```) +Watch Docker Swarm events. (Default: ```true```) `--providers.file.debugloggeneratedtemplate`: Enable debug logging of generated configuration template. (Default: ```false```) diff --git a/docs/content/reference/static-configuration/env-ref.md b/docs/content/reference/static-configuration/env-ref.md index 18ddad5d3..6335db991 100644 --- a/docs/content/reference/static-configuration/env-ref.md +++ b/docs/content/reference/static-configuration/env-ref.md @@ -346,7 +346,7 @@ TLS key Use the ip address from the bound port, rather than from the inner network. (Default: ```false```) `TRAEFIK_PROVIDERS_DOCKER_WATCH`: -Watch provider. (Default: ```true```) +Watch Docker Swarm events. (Default: ```true```) `TRAEFIK_PROVIDERS_FILE_DEBUGLOGGENERATEDTEMPLATE`: Enable debug logging of generated configuration template. (Default: ```false```) diff --git a/pkg/provider/docker/docker.go b/pkg/provider/docker/docker.go index 15c8bba1f..e91b03ef9 100644 --- a/pkg/provider/docker/docker.go +++ b/pkg/provider/docker/docker.go @@ -46,7 +46,7 @@ var _ provider.Provider = (*Provider)(nil) // Provider holds configurations of the provider. type Provider struct { Constraints string `description:"Constraints is an expression that Traefik matches against the container's labels to determine whether to create any route for that container." json:"constraints,omitempty" toml:"constraints,omitempty" yaml:"constraints,omitempty" export:"true"` - Watch bool `description:"Watch provider." json:"watch,omitempty" toml:"watch,omitempty" yaml:"watch,omitempty" export:"true"` + Watch bool `description:"Watch Docker Swarm events." json:"watch,omitempty" toml:"watch,omitempty" yaml:"watch,omitempty" export:"true"` Endpoint string `description:"Docker server endpoint. Can be a tcp or a unix socket endpoint." json:"endpoint,omitempty" toml:"endpoint,omitempty" yaml:"endpoint,omitempty"` DefaultRule string `description:"Default rule." json:"defaultRule,omitempty" toml:"defaultRule,omitempty" yaml:"defaultRule,omitempty"` TLS *types.ClientTLS `description:"Enable Docker TLS support." json:"tls,omitempty" toml:"tls,omitempty" yaml:"tls,omitempty" export:"true"` From b5d205b78c23095546c6ff7e66d98df6dbb58b20 Mon Sep 17 00:00:00 2001 From: Traefiker Bot <30906710+traefiker@users.noreply.github.com> Date: Thu, 5 Mar 2020 15:10:07 +0100 Subject: [PATCH 5/5] fix statsd scale for duration based metrics --- pkg/metrics/datadog.go | 4 +- pkg/metrics/influxdb.go | 4 +- pkg/metrics/metrics.go | 117 ++++++++++++++++++++++++++--- pkg/metrics/metrics_test.go | 40 +++++++++- pkg/metrics/prometheus.go | 5 +- pkg/metrics/statsd.go | 4 +- pkg/middlewares/metrics/metrics.go | 12 ++- 7 files changed, 161 insertions(+), 25 deletions(-) diff --git a/pkg/metrics/datadog.go b/pkg/metrics/datadog.go index 988fb0b7a..34994d7a6 100644 --- a/pkg/metrics/datadog.go +++ b/pkg/metrics/datadog.go @@ -50,14 +50,14 @@ func RegisterDatadog(ctx context.Context, config *types.Datadog) Registry { if config.AddEntryPointsLabels { registry.epEnabled = config.AddEntryPointsLabels registry.entryPointReqsCounter = datadogClient.NewCounter(ddEntryPointReqsName, 1.0) - registry.entryPointReqDurationHistogram = datadogClient.NewHistogram(ddEntryPointReqDurationName, 1.0) + registry.entryPointReqDurationHistogram, _ = NewHistogramWithScale(datadogClient.NewHistogram(ddEntryPointReqDurationName, 1.0), time.Second) registry.entryPointOpenConnsGauge = datadogClient.NewGauge(ddEntryPointOpenConnsName) } if config.AddServicesLabels { registry.svcEnabled = config.AddServicesLabels registry.serviceReqsCounter = datadogClient.NewCounter(ddMetricsServiceReqsName, 1.0) - registry.serviceReqDurationHistogram = datadogClient.NewHistogram(ddMetricsServiceLatencyName, 1.0) + registry.serviceReqDurationHistogram, _ = NewHistogramWithScale(datadogClient.NewHistogram(ddMetricsServiceLatencyName, 1.0), time.Second) registry.serviceRetriesCounter = datadogClient.NewCounter(ddRetriesTotalName, 1.0) registry.serviceOpenConnsGauge = datadogClient.NewGauge(ddOpenConnsName) registry.serviceServerUpGauge = datadogClient.NewGauge(ddServerUpName) diff --git a/pkg/metrics/influxdb.go b/pkg/metrics/influxdb.go index 3bc859889..b8bbc363c 100644 --- a/pkg/metrics/influxdb.go +++ b/pkg/metrics/influxdb.go @@ -64,14 +64,14 @@ func RegisterInfluxDB(ctx context.Context, config *types.InfluxDB) Registry { if config.AddEntryPointsLabels { registry.epEnabled = config.AddEntryPointsLabels registry.entryPointReqsCounter = influxDBClient.NewCounter(influxDBEntryPointReqsName) - registry.entryPointReqDurationHistogram = influxDBClient.NewHistogram(influxDBEntryPointReqDurationName) + registry.entryPointReqDurationHistogram, _ = NewHistogramWithScale(influxDBClient.NewHistogram(influxDBEntryPointReqDurationName), time.Second) registry.entryPointOpenConnsGauge = influxDBClient.NewGauge(influxDBEntryPointOpenConnsName) } if config.AddServicesLabels { registry.svcEnabled = config.AddServicesLabels registry.serviceReqsCounter = influxDBClient.NewCounter(influxDBMetricsServiceReqsName) - registry.serviceReqDurationHistogram = influxDBClient.NewHistogram(influxDBMetricsServiceLatencyName) + registry.serviceReqDurationHistogram, _ = NewHistogramWithScale(influxDBClient.NewHistogram(influxDBMetricsServiceLatencyName), time.Second) registry.serviceRetriesCounter = influxDBClient.NewCounter(influxDBRetriesTotalName) registry.serviceOpenConnsGauge = influxDBClient.NewGauge(influxDBOpenConnsName) registry.serviceServerUpGauge = influxDBClient.NewGauge(influxDBServerUpName) diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 8cdb1eae9..4e7749d07 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -1,6 +1,9 @@ package metrics import ( + "errors" + "time" + "github.com/go-kit/kit/metrics" "github.com/go-kit/kit/metrics/multi" ) @@ -20,12 +23,12 @@ type Registry interface { // entry point metrics EntryPointReqsCounter() metrics.Counter - EntryPointReqDurationHistogram() metrics.Histogram + EntryPointReqDurationHistogram() ScalableHistogram EntryPointOpenConnsGauge() metrics.Gauge // service metrics ServiceReqsCounter() metrics.Counter - ServiceReqDurationHistogram() metrics.Histogram + ServiceReqDurationHistogram() ScalableHistogram ServiceOpenConnsGauge() metrics.Gauge ServiceRetriesCounter() metrics.Counter ServiceServerUpGauge() metrics.Gauge @@ -46,10 +49,10 @@ func NewMultiRegistry(registries []Registry) Registry { var lastConfigReloadSuccessGauge []metrics.Gauge var lastConfigReloadFailureGauge []metrics.Gauge var entryPointReqsCounter []metrics.Counter - var entryPointReqDurationHistogram []metrics.Histogram + var entryPointReqDurationHistogram []ScalableHistogram var entryPointOpenConnsGauge []metrics.Gauge var serviceReqsCounter []metrics.Counter - var serviceReqDurationHistogram []metrics.Histogram + var serviceReqDurationHistogram []ScalableHistogram var serviceOpenConnsGauge []metrics.Gauge var serviceRetriesCounter []metrics.Counter var serviceServerUpGauge []metrics.Gauge @@ -101,10 +104,10 @@ func NewMultiRegistry(registries []Registry) Registry { lastConfigReloadSuccessGauge: multi.NewGauge(lastConfigReloadSuccessGauge...), lastConfigReloadFailureGauge: multi.NewGauge(lastConfigReloadFailureGauge...), entryPointReqsCounter: multi.NewCounter(entryPointReqsCounter...), - entryPointReqDurationHistogram: multi.NewHistogram(entryPointReqDurationHistogram...), + entryPointReqDurationHistogram: NewMultiHistogram(entryPointReqDurationHistogram...), entryPointOpenConnsGauge: multi.NewGauge(entryPointOpenConnsGauge...), serviceReqsCounter: multi.NewCounter(serviceReqsCounter...), - serviceReqDurationHistogram: multi.NewHistogram(serviceReqDurationHistogram...), + serviceReqDurationHistogram: NewMultiHistogram(serviceReqDurationHistogram...), serviceOpenConnsGauge: multi.NewGauge(serviceOpenConnsGauge...), serviceRetriesCounter: multi.NewCounter(serviceRetriesCounter...), serviceServerUpGauge: multi.NewGauge(serviceServerUpGauge...), @@ -119,10 +122,10 @@ type standardRegistry struct { lastConfigReloadSuccessGauge metrics.Gauge lastConfigReloadFailureGauge metrics.Gauge entryPointReqsCounter metrics.Counter - entryPointReqDurationHistogram metrics.Histogram + entryPointReqDurationHistogram ScalableHistogram entryPointOpenConnsGauge metrics.Gauge serviceReqsCounter metrics.Counter - serviceReqDurationHistogram metrics.Histogram + serviceReqDurationHistogram ScalableHistogram serviceOpenConnsGauge metrics.Gauge serviceRetriesCounter metrics.Counter serviceServerUpGauge metrics.Gauge @@ -156,7 +159,7 @@ func (r *standardRegistry) EntryPointReqsCounter() metrics.Counter { return r.entryPointReqsCounter } -func (r *standardRegistry) EntryPointReqDurationHistogram() metrics.Histogram { +func (r *standardRegistry) EntryPointReqDurationHistogram() ScalableHistogram { return r.entryPointReqDurationHistogram } @@ -168,7 +171,7 @@ func (r *standardRegistry) ServiceReqsCounter() metrics.Counter { return r.serviceReqsCounter } -func (r *standardRegistry) ServiceReqDurationHistogram() metrics.Histogram { +func (r *standardRegistry) ServiceReqDurationHistogram() ScalableHistogram { return r.serviceReqDurationHistogram } @@ -183,3 +186,97 @@ func (r *standardRegistry) ServiceRetriesCounter() metrics.Counter { func (r *standardRegistry) ServiceServerUpGauge() metrics.Gauge { return r.serviceServerUpGauge } + +// ScalableHistogram is a Histogram with a predefined time unit, +// used when producing observations without explicitly setting the observed value. +type ScalableHistogram interface { + With(labelValues ...string) ScalableHistogram + StartAt(t time.Time) + Observe(v float64) + ObserveDuration() +} + +// HistogramWithScale is a histogram that will convert its observed value to the specified unit. +type HistogramWithScale struct { + histogram metrics.Histogram + unit time.Duration + start time.Time +} + +// With implements ScalableHistogram. +func (s *HistogramWithScale) With(labelValues ...string) ScalableHistogram { + s.histogram = s.histogram.With(labelValues...) + return s +} + +// StartAt implements ScalableHistogram. +func (s *HistogramWithScale) StartAt(t time.Time) { + s.start = t +} + +// ObserveDuration implements ScalableHistogram. +func (s *HistogramWithScale) ObserveDuration() { + if s.unit <= 0 { + return + } + + d := float64(time.Since(s.start).Nanoseconds()) / float64(s.unit) + if d < 0 { + d = 0 + } + s.histogram.Observe(d) +} + +// Observe implements ScalableHistogram. +func (s *HistogramWithScale) Observe(v float64) { + s.histogram.Observe(v) +} + +// NewHistogramWithScale returns a ScalableHistogram. It returns an error if the given unit is <= 0. +func NewHistogramWithScale(histogram metrics.Histogram, unit time.Duration) (ScalableHistogram, error) { + if unit <= 0 { + return nil, errors.New("invalid time unit") + } + return &HistogramWithScale{ + histogram: histogram, + unit: unit, + }, nil +} + +// MultiHistogram collects multiple individual histograms and treats them as a unit. +type MultiHistogram []ScalableHistogram + +// NewMultiHistogram returns a multi-histogram, wrapping the passed histograms. +func NewMultiHistogram(h ...ScalableHistogram) MultiHistogram { + return MultiHistogram(h) +} + +// StartAt implements ScalableHistogram. +func (h MultiHistogram) StartAt(t time.Time) { + for _, histogram := range h { + histogram.StartAt(t) + } +} + +// ObserveDuration implements ScalableHistogram. +func (h MultiHistogram) ObserveDuration() { + for _, histogram := range h { + histogram.ObserveDuration() + } +} + +// Observe implements ScalableHistogram. +func (h MultiHistogram) Observe(v float64) { + for _, histogram := range h { + histogram.Observe(v) + } +} + +// With implements ScalableHistogram. +func (h MultiHistogram) With(labelValues ...string) ScalableHistogram { + next := make(MultiHistogram, len(h)) + for i := range h { + next[i] = h[i].With(labelValues...) + } + return next +} diff --git a/pkg/metrics/metrics_test.go b/pkg/metrics/metrics_test.go index 6477aac96..221193a42 100644 --- a/pkg/metrics/metrics_test.go +++ b/pkg/metrics/metrics_test.go @@ -1,18 +1,44 @@ package metrics import ( + "bytes" + "strings" "testing" + "time" "github.com/go-kit/kit/metrics" + "github.com/go-kit/kit/metrics/generic" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func TestScalableHistogram(t *testing.T) { + h := generic.NewHistogram("test", 1) + sh, err := NewHistogramWithScale(h, time.Millisecond) + require.NoError(t, err) + + ticker := time.NewTicker(500 * time.Millisecond) + <-ticker.C + sh.StartAt(time.Now()) + <-ticker.C + sh.ObserveDuration() + + var b bytes.Buffer + h.Print(&b) + + extractedDurationString := strings.Split(strings.Split(b.String(), "\n")[1], " ") + measuredDuration, err := time.ParseDuration(extractedDurationString[0] + "ms") + assert.NoError(t, err) + + assert.InDelta(t, 500*time.Millisecond, measuredDuration, float64(1*time.Millisecond)) +} + func TestNewMultiRegistry(t *testing.T) { registries := []Registry{newCollectingRetryMetrics(), newCollectingRetryMetrics()} registry := NewMultiRegistry(registries) registry.ServiceReqsCounter().With("key", "requests").Add(1) - registry.ServiceReqDurationHistogram().With("key", "durations").Observe(2) + registry.ServiceReqDurationHistogram().With("key", "durations").Observe(float64(2)) registry.ServiceRetriesCounter().With("key", "retries").Add(3) for _, collectingRegistry := range registries { @@ -66,11 +92,17 @@ type histogramMock struct { lastLabelValues []string } -func (c *histogramMock) With(labelValues ...string) metrics.Histogram { +func (c *histogramMock) With(labelValues ...string) ScalableHistogram { c.lastLabelValues = labelValues return c } -func (c *histogramMock) Observe(value float64) { - c.lastHistogramValue = value +func (c *histogramMock) Start() {} + +func (c *histogramMock) StartAt(t time.Time) {} + +func (c *histogramMock) ObserveDuration() {} + +func (c *histogramMock) Observe(v float64) { + c.lastHistogramValue = v } diff --git a/pkg/metrics/prometheus.go b/pkg/metrics/prometheus.go index 6003da55b..822eeff01 100644 --- a/pkg/metrics/prometheus.go +++ b/pkg/metrics/prometheus.go @@ -6,6 +6,7 @@ import ( "sort" "strings" "sync" + "time" "github.com/containous/traefik/v2/pkg/config/dynamic" "github.com/containous/traefik/v2/pkg/log" @@ -152,7 +153,7 @@ func initStandardRegistry(config *types.Prometheus) Registry { entryPointOpenConns.gv.Describe, }...) reg.entryPointReqsCounter = entryPointReqs - reg.entryPointReqDurationHistogram = entryPointReqDurations + reg.entryPointReqDurationHistogram, _ = NewHistogramWithScale(entryPointReqDurations, time.Second) reg.entryPointOpenConnsGauge = entryPointOpenConns } if config.AddServicesLabels { @@ -187,7 +188,7 @@ func initStandardRegistry(config *types.Prometheus) Registry { }...) reg.serviceReqsCounter = serviceReqs - reg.serviceReqDurationHistogram = serviceReqDurations + reg.serviceReqDurationHistogram, _ = NewHistogramWithScale(serviceReqDurations, time.Second) reg.serviceOpenConnsGauge = serviceOpenConns reg.serviceRetriesCounter = serviceRetries reg.serviceServerUpGauge = serviceServerUp diff --git a/pkg/metrics/statsd.go b/pkg/metrics/statsd.go index eb08de8f4..f8f14de63 100644 --- a/pkg/metrics/statsd.go +++ b/pkg/metrics/statsd.go @@ -55,14 +55,14 @@ func RegisterStatsd(ctx context.Context, config *types.Statsd) Registry { if config.AddEntryPointsLabels { registry.epEnabled = config.AddEntryPointsLabels registry.entryPointReqsCounter = statsdClient.NewCounter(statsdEntryPointReqsName, 1.0) - registry.entryPointReqDurationHistogram = statsdClient.NewTiming(statsdEntryPointReqDurationName, 1.0) + registry.entryPointReqDurationHistogram, _ = NewHistogramWithScale(statsdClient.NewTiming(statsdEntryPointReqDurationName, 1.0), time.Millisecond) registry.entryPointOpenConnsGauge = statsdClient.NewGauge(statsdEntryPointOpenConnsName) } if config.AddServicesLabels { registry.svcEnabled = config.AddServicesLabels registry.serviceReqsCounter = statsdClient.NewCounter(statsdMetricsServiceReqsName, 1.0) - registry.serviceReqDurationHistogram = statsdClient.NewTiming(statsdMetricsServiceLatencyName, 1.0) + registry.serviceReqDurationHistogram, _ = NewHistogramWithScale(statsdClient.NewTiming(statsdMetricsServiceLatencyName, 1.0), time.Millisecond) registry.serviceRetriesCounter = statsdClient.NewCounter(statsdRetriesTotalName, 1.0) registry.serviceOpenConnsGauge = statsdClient.NewGauge(statsdOpenConnsName) registry.serviceServerUpGauge = statsdClient.NewGauge(statsdServerUpName) diff --git a/pkg/middlewares/metrics/metrics.go b/pkg/middlewares/metrics/metrics.go index 34d9dccc8..53ce890fa 100644 --- a/pkg/middlewares/metrics/metrics.go +++ b/pkg/middlewares/metrics/metrics.go @@ -32,7 +32,7 @@ type metricsMiddleware struct { openConns int64 next http.Handler reqsCounter gokitmetrics.Counter - reqDurationHistogram gokitmetrics.Histogram + reqDurationHistogram metrics.ScalableHistogram openConnsGauge gokitmetrics.Gauge baseLabels []string } @@ -88,13 +88,19 @@ func (m *metricsMiddleware) ServeHTTP(rw http.ResponseWriter, req *http.Request) m.openConnsGauge.With(labelValues...).Set(float64(openConns)) }(labels) - start := time.Now() recorder := newResponseRecorder(rw) + start := time.Now() + m.next.ServeHTTP(recorder, req) labels = append(labels, "code", strconv.Itoa(recorder.getCode())) + + histograms := m.reqDurationHistogram.With(labels...) + histograms.StartAt(start) + m.reqsCounter.With(labels...).Add(1) - m.reqDurationHistogram.With(labels...).Observe(time.Since(start).Seconds()) + + histograms.ObserveDuration() } func getRequestProtocol(req *http.Request) string {