diff --git a/CHANGELOG.md b/CHANGELOG.md index fe16c520cd..af5c97b00f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ or `/query` HTTP endpoints. 1. [20827](https://github.com/influxdata/influxdb/pull/20827): Upgrade `http.pprof-enabled` config in `influxd upgrade`. 1. [20911](https://github.com/influxdata/influxdb/pull/20911): Add support for explicitly setting shard-group durations on buckets. Thanks @hinst! 1. [20882](https://github.com/influxdata/influxdb/pull/20882): Rewrite regex conditions in InfluxQL subqueries for performance. Thanks @yujiahaol68! +1. [20963](https://github.com/influxdata/influxdb/pull/20963): Add `--metrics-disabled` option to `influxd` to disable exposing Prometheus metrics over HTTP. ### Bug Fixes diff --git a/cmd/influxd/launcher/cmd.go b/cmd/influxd/launcher/cmd.go index 584b44b458..95ee418f32 100644 --- a/cmd/influxd/launcher/cmd.go +++ b/cmd/influxd/launcher/cmd.go @@ -143,6 +143,7 @@ type InfluxdOpts struct { SessionRenewDisabled bool ProfilingDisabled bool + MetricsDisabled bool NatsPort int NatsMaxPayloadBytes int @@ -189,6 +190,7 @@ func newOpts(viper *viper.Viper) *InfluxdOpts { SessionRenewDisabled: false, ProfilingDisabled: false, + MetricsDisabled: false, StoreType: BoltStore, SecretStore: BoltStore, @@ -519,5 +521,13 @@ func (o *InfluxdOpts) bindCliOpts() []cli.Opt { Desc: "Don't expose debugging information over HTTP at /debug/pprof", Default: o.ProfilingDisabled, }, + + // Metrics config + { + DestP: &o.MetricsDisabled, + Flag: "metrics-disabled", + Desc: "Don't expose metrics over HTTP at /metrics", + Default: o.MetricsDisabled, + }, } } diff --git a/cmd/influxd/launcher/launcher.go b/cmd/influxd/launcher/launcher.go index 0fac36c1f5..3d3c776419 100644 --- a/cmd/influxd/launcher/launcher.go +++ b/cmd/influxd/launcher/launcher.go @@ -915,12 +915,12 @@ func (m *Launcher) run(ctx context.Context, opts *InfluxdOpts) (err error) { ) httpLogger := m.log.With(zap.String("service", "http")) - m.httpServer.Handler = http.NewHandlerFromRegistry( + m.httpServer.Handler = http.NewRootHandler( "platform", - m.reg, http.WithLog(httpLogger), http.WithAPIHandler(platformHandler), http.WithPprofEnabled(!opts.ProfilingDisabled), + http.WithMetrics(m.reg, !opts.MetricsDisabled), ) if opts.LogLevel == zap.DebugLevel { diff --git a/http/handler.go b/http/handler.go index de700133a8..6fd280e984 100644 --- a/http/handler.go +++ b/http/handler.go @@ -6,6 +6,7 @@ import ( "net/http" "github.com/go-chi/chi" + "github.com/influxdata/influxdb/v2" "github.com/influxdata/influxdb/v2/kit/prom" kithttp "github.com/influxdata/influxdb/v2/kit/transport/http" "github.com/influxdata/influxdb/v2/pprof" @@ -39,17 +40,31 @@ type Handler struct { type ( handlerOpts struct { - log *zap.Logger - apiHandler http.Handler - healthHandler http.Handler - metricsHandler http.Handler - readyHandler http.Handler - pprofEnabled bool + log *zap.Logger + apiHandler http.Handler + healthHandler http.Handler + readyHandler http.Handler + pprofEnabled bool + + // NOTE: Track the registry even if metricsExposed = false + // so we can report HTTP metrics via telemetry. + metricsRegistry *prom.Registry + metricsExposed bool } HandlerOptFn func(opts *handlerOpts) ) +func (o *handlerOpts) metricsHTTPHandler() http.Handler { + if o.metricsRegistry != nil && o.metricsExposed { + return o.metricsRegistry.HTTPHandler() + } + handlerFunc := func(rw http.ResponseWriter, r *http.Request) { + kithttp.WriteErrorResponse(r.Context(), rw, influxdb.EForbidden, "metrics disabled") + } + return http.HandlerFunc(handlerFunc) +} + func WithLog(l *zap.Logger) HandlerOptFn { return func(opts *handlerOpts) { opts.log = l @@ -68,34 +83,23 @@ func WithPprofEnabled(enabled bool) HandlerOptFn { } } -func WithHealthHandler(h http.Handler) HandlerOptFn { +func WithMetrics(reg *prom.Registry, exposed bool) HandlerOptFn { return func(opts *handlerOpts) { - opts.healthHandler = h + opts.metricsRegistry = reg + opts.metricsExposed = exposed } } -func WithMetricsHandler(h http.Handler) HandlerOptFn { - return func(opts *handlerOpts) { - opts.metricsHandler = h - } -} - -func WithReadyHandler(h http.Handler) HandlerOptFn { - return func(opts *handlerOpts) { - opts.readyHandler = h - } -} - -// NewHandlerFromRegistry creates a new handler with the given name, -// and sets the /metrics endpoint to use the metrics from the given registry, -// after self-registering h's metrics. -func NewHandlerFromRegistry(name string, reg *prom.Registry, opts ...HandlerOptFn) *Handler { +// NewRootHandler creates a new handler with the given name and registers any root-level +// (non-API) routes enabled by the caller. +func NewRootHandler(name string, opts ...HandlerOptFn) *Handler { opt := handlerOpts{ - log: zap.NewNop(), - healthHandler: http.HandlerFunc(HealthHandler), - metricsHandler: reg.HTTPHandler(), - readyHandler: ReadyHandler(), - pprofEnabled: false, + log: zap.NewNop(), + healthHandler: http.HandlerFunc(HealthHandler), + readyHandler: ReadyHandler(), + pprofEnabled: false, + metricsRegistry: nil, + metricsExposed: false, } for _, o := range opts { o(&opt) @@ -113,7 +117,7 @@ func NewHandlerFromRegistry(name string, reg *prom.Registry, opts ...HandlerOptF r.Use( kithttp.Metrics(name, h.requests, h.requestDur), ) - r.Mount(MetricsPath, opt.metricsHandler) + r.Mount(MetricsPath, opt.metricsHTTPHandler()) r.Mount(ReadyPath, opt.readyHandler) r.Mount(HealthPath, opt.healthHandler) r.Mount(DebugPath, pprof.NewHTTPHandler(opt.pprofEnabled)) @@ -130,7 +134,9 @@ func NewHandlerFromRegistry(name string, reg *prom.Registry, opts ...HandlerOptF h.r = r - reg.MustRegister(h.PrometheusCollectors()...) + if opt.metricsRegistry != nil { + opt.metricsRegistry.MustRegister(h.PrometheusCollectors()...) + } return h } diff --git a/http/handler_test.go b/http/handler_test.go index bbf0c5015a..783e937b9d 100644 --- a/http/handler_test.go +++ b/http/handler_test.go @@ -7,24 +7,21 @@ import ( "github.com/influxdata/influxdb/v2/kit/prom" "github.com/influxdata/influxdb/v2/kit/prom/promtest" + "github.com/stretchr/testify/require" "go.uber.org/zap" "go.uber.org/zap/zaptest" ) func TestHandler_ServeHTTP(t *testing.T) { type fields struct { - name string - handler http.Handler - log *zap.Logger - } - type args struct { - w *httptest.ResponseRecorder - r *http.Request + name string + handler http.Handler + handlerHidden bool + log *zap.Logger } tests := []struct { name string fields fields - args args }{ { name: "should record metrics when http handling", @@ -33,29 +30,33 @@ func TestHandler_ServeHTTP(t *testing.T) { handler: http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), log: zaptest.NewLogger(t), }, - args: args{ - r: httptest.NewRequest(http.MethodGet, "/", nil), - w: httptest.NewRecorder(), + }, + { + name: "should record metrics even when not exposed over HTTP", + fields: fields{ + name: "test", + handler: http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), + handlerHidden: true, + log: zaptest.NewLogger(t), }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { reg := prom.NewRegistry(zaptest.NewLogger(t)) - h := NewHandlerFromRegistry( + h := NewRootHandler( tt.fields.name, - reg, WithLog(tt.fields.log), WithAPIHandler(tt.fields.handler), + WithMetrics(reg, !tt.fields.handlerHidden), ) - tt.args.r.Header.Set("User-Agent", "ua1") - h.ServeHTTP(tt.args.w, tt.args.r) + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Header.Set("User-Agent", "ua1") + h.ServeHTTP(httptest.NewRecorder(), req) mfs, err := reg.Gather() - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) c := promtest.MustFindMetric(t, mfs, "http_api_requests_total", map[string]string{ "handler": "test", @@ -65,9 +66,7 @@ func TestHandler_ServeHTTP(t *testing.T) { "user_agent": "ua1", "response_code": "200", }) - if got := c.GetCounter().GetValue(); got != 1 { - t.Fatalf("expected counter to be 1, got %v", got) - } + require.Equal(t, 1, int(c.GetCounter().GetValue())) g := promtest.MustFindMetric(t, mfs, "http_api_request_duration_seconds", map[string]string{ "handler": "test", @@ -77,10 +76,17 @@ func TestHandler_ServeHTTP(t *testing.T) { "user_agent": "ua1", "response_code": "200", }) - if got := g.GetHistogram().GetSampleCount(); got != 1 { - t.Fatalf("expected histogram sample count to be 1, got %v", got) + require.Equal(t, 1, int(g.GetHistogram().GetSampleCount())) + + req = httptest.NewRequest(http.MethodGet, "/metrics", nil) + recorder := httptest.NewRecorder() + h.ServeHTTP(recorder, req) + + if tt.fields.handlerHidden { + require.Equal(t, http.StatusForbidden, recorder.Code) + } else { + require.Equal(t, http.StatusOK, recorder.Code) } }) - } } diff --git a/kit/transport/http/error_handler.go b/kit/transport/http/error_handler.go index da57d8dc4a..cdb1369394 100644 --- a/kit/transport/http/error_handler.go +++ b/kit/transport/http/error_handler.go @@ -27,18 +27,26 @@ func (h ErrorHandler) HandleHTTPError(ctx context.Context, err error, w http.Res } code := influxdb.ErrorCode(err) + var msg string + if err, ok := err.(*influxdb.Error); ok { + msg = err.Error() + } else { + msg = "An internal error has occurred" + } + + WriteErrorResponse(ctx, w, code, msg) +} + +func WriteErrorResponse(ctx context.Context, w http.ResponseWriter, code string, msg string) { w.Header().Set(PlatformErrorCodeHeader, code) w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(ErrorCodeToStatusCode(ctx, code)) - var e struct { + e := struct { Code string `json:"code"` Message string `json:"message"` - } - e.Code = influxdb.ErrorCode(err) - if err, ok := err.(*influxdb.Error); ok { - e.Message = err.Error() - } else { - e.Message = "An internal error has occurred" + }{ + Code: code, + Message: msg, } b, _ := json.Marshal(e) _, _ = w.Write(b) diff --git a/pprof/http_server.go b/pprof/http_server.go index 7702178bb1..b7a4175ed9 100644 --- a/pprof/http_server.go +++ b/pprof/http_server.go @@ -1,8 +1,6 @@ package pprof import ( - "context" - "encoding/json" "fmt" "io" "net/http" @@ -37,23 +35,8 @@ func NewHTTPHandler(profilingEnabled bool) *Handler { return &Handler{r} } -func errResponse(ctx context.Context, w http.ResponseWriter, code string, message string) { - w.Header().Set(ihttp.PlatformErrorCodeHeader, code) - w.Header().Set("Content-Type", "application/json; charset=utf-8") - w.WriteHeader(ihttp.ErrorCodeToStatusCode(ctx, code)) - e := struct { - Code string `json:"code"` - Message string `json:"message"` - }{ - Code: code, - Message: message, - } - b, _ := json.Marshal(e) - _, _ = w.Write(b) -} - func profilingDisabledHandler(w http.ResponseWriter, r *http.Request) { - errResponse(r.Context(), w, influxdb.EForbidden, "profiling disabled") + ihttp.WriteErrorResponse(r.Context(), w, influxdb.EForbidden, "profiling disabled") } func archiveProfilesHandler(w http.ResponseWriter, r *http.Request) { @@ -65,7 +48,7 @@ func archiveProfilesHandler(w http.ResponseWriter, r *http.Request) { // distinguish between a form value that exists and has no value and one that // does not exist at all. if err := r.ParseForm(); err != nil { - errResponse(ctx, w, influxdb.EInternal, err.Error()) + ihttp.WriteErrorResponse(ctx, w, influxdb.EInternal, err.Error()) return } @@ -112,18 +95,18 @@ func archiveProfilesHandler(w http.ResponseWriter, r *http.Request) { // In this case it is a StatusBadRequest (400) since the problem is in the // supplied form data. if duration < 0 { - errResponse(ctx, w, influxdb.EInvalid, "negative trace durations not allowed") + ihttp.WriteErrorResponse(ctx, w, influxdb.EInvalid, "negative trace durations not allowed") return } if err != nil { - errResponse(ctx, w, influxdb.EInvalid, fmt.Sprintf("could not parse supplied duration for trace %q", val)) + ihttp.WriteErrorResponse(ctx, w, influxdb.EInvalid, fmt.Sprintf("could not parse supplied duration for trace %q", val)) return } // Trace files can get big. Lets clamp the maximum trace duration to 45s. if duration > 45*time.Second { - errResponse(ctx, w, influxdb.EInvalid, "cannot trace for longer than 45s") + ihttp.WriteErrorResponse(ctx, w, influxdb.EInvalid, "cannot trace for longer than 45s") return } @@ -172,7 +155,7 @@ func archiveProfilesHandler(w http.ResponseWriter, r *http.Request) { duration, err := getDuration() if err != nil { - errResponse(ctx, w, influxdb.EInvalid, fmt.Sprintf("could not parse supplied duration for cpu profile %q", val)) + ihttp.WriteErrorResponse(ctx, w, influxdb.EInvalid, fmt.Sprintf("could not parse supplied duration for cpu profile %q", val)) return } @@ -181,7 +164,7 @@ func archiveProfilesHandler(w http.ResponseWriter, r *http.Request) { tarstream, err := collectAllProfiles(ctx, traceDuration, cpuDuration) if err != nil { - errResponse(ctx, w, influxdb.EInternal, err.Error()) + ihttp.WriteErrorResponse(ctx, w, influxdb.EInternal, err.Error()) return } _, _ = io.Copy(w, tarstream)