Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat(kit/feature): add feature flag package #17851

Merged
merged 3 commits into from
Apr 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,42 @@ jobs:
destination: raw-test-output
- store_test_results: # Upload test results for display in Test Summary: https://circleci.com/docs/2.0/collect-test-data/
path: /tmp/test-results

lint-feature-flags:
docker:
- image: circleci/golang:1.13
environment:
GOCACHE: /tmp/go-cache
GOFLAGS: "-mod=readonly -p=2" # Go on Circle thinks 32 CPUs are available, but there aren't.
working_directory: /go/src/github.com/influxdata/influxdb
steps:
- checkout
# Populate GOCACHE.
- restore_cache:
name: Restoring GOCACHE
keys:
- influxdb-gocache-{{ .Branch }}-{{ .Revision }} # Matches when retrying a single run.
- influxdb-gocache-{{ .Branch }}- # Matches a new commit on an existing branch.
- influxdb-gocache- # Matches a new branch.
# Populate GOPATH/pkg.
- restore_cache:
name: Restoring GOPATH/pkg/mod
keys:
- influxdb-gomod-{{ checksum "go.sum" }} # Matches based on go.sum checksum.
- run: ./scripts/ci/lint/flags.bash
- skip_if_not_master
- save_cache:
name: Saving GOCACHE
key: influxdb-gocache-{{ .Branch }}-{{ .Revision }}
paths:
- /tmp/go-cache
when: always
- save_cache:
name: Saving GOPATH/pkg/mod
key: influxdb-gomod-{{ checksum "go.sum" }}
paths:
- /go/pkg/mod
when: always
golint:
docker:
- image: circleci/golang:1.13
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

### Features

1. [17851](https://github.com/influxdata/influxdb/pull/17851): Add feature flag package capability and flags endpoint

### Bug Fixes

1. [17618](https://github.com/influxdata/influxdb/pull/17618): Add index for URM by user ID to improve lookup performance
Expand Down
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -206,5 +206,9 @@ protoc:
unzip -o -d /go /tmp/protoc.zip
chmod +x /go/bin/protoc

# generate feature flags
flags:
$(GO_GENERATE) ./kit/feature

# .PHONY targets represent actions that do not create an actual file.
.PHONY: all $(SUBDIRS) run fmt checkfmt tidy checktidy checkgenerate test test-go test-js test-go-race bench clean node_modules vet nightly chronogiraffe dist ping protoc e2e run-e2e influxd libflux
.PHONY: all $(SUBDIRS) run fmt checkfmt tidy checktidy checkgenerate test test-go test-js test-go-race bench clean node_modules vet nightly chronogiraffe dist ping protoc e2e run-e2e influxd libflux flags
23 changes: 22 additions & 1 deletion cmd/influxd/launcher/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
"github.com/influxdata/influxdb/v2/inmem"
"github.com/influxdata/influxdb/v2/internal/fs"
"github.com/influxdata/influxdb/v2/kit/cli"
"github.com/influxdata/influxdb/v2/kit/feature"
overrideflagger "github.com/influxdata/influxdb/v2/kit/feature/override"
"github.com/influxdata/influxdb/v2/kit/prom"
"github.com/influxdata/influxdb/v2/kit/signals"
"github.com/influxdata/influxdb/v2/kit/tracing"
Expand Down Expand Up @@ -303,8 +305,12 @@ func buildLauncherCommand(l *Launcher, cmd *cobra.Command) {
Default: 10,
Desc: "the number of queries that are allowed to be awaiting execution before new queries are rejected",
},
{
DestP: &l.featureFlags,
Flag: "feature-flags",
Desc: "feature flag overrides",
},
}

cli.BindOptions(cmd, opts)
cmd.AddCommand(inspect.NewCommand())
}
Expand Down Expand Up @@ -332,6 +338,8 @@ type Launcher struct {

enableNewMetaStore bool

featureFlags map[string]string

// Query options.
concurrencyQuota int
initialMemoryBytesQuotaPerQuery int
Expand Down Expand Up @@ -843,6 +851,18 @@ func (m *Launcher) run(ctx context.Context) (err error) {
Addr: m.httpBindAddress,
}

flagger := feature.DefaultFlagger()
if len(m.featureFlags) > 0 {
f, err := overrideflagger.Make(m.featureFlags)
if err != nil {
m.log.Error("Failed to configure feature flag overrides",
zap.Error(err), zap.Any("overrides", m.featureFlags))
return err
}
m.log.Info("Running with feature flag overrides", zap.Any("config", m.featureFlags))
flagger = f
}

m.apibackend = &http.APIBackend{
AssetsPath: m.assetsPath,
HTTPErrorHandler: kithttp.ErrorHandler(0),
Expand Down Expand Up @@ -885,6 +905,7 @@ func (m *Launcher) run(ctx context.Context) (err error) {
OrgLookupService: m.kvService,
WriteEventRecorder: infprom.NewEventRecorder("write"),
QueryEventRecorder: infprom.NewEventRecorder("query"),
Flagger: flagger,
}

m.reg.MustRegister(m.apibackend.PrometheusCollectors()...)
Expand Down
28 changes: 28 additions & 0 deletions flags.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# This file defines feature flags.
#
# It is used for code generation in the ./kit/feature package.
# If you change this file, run `make flags` to regenerate.
#
# Format details:
#
# - name: Human-readable name
# description: Human-readable description
# key: Programmatic name
# default: Used when unable to reach server and to infer flag type
# contact: Contact for information or issues regarding the flag
# lifetime: Expected lifetime of the flag; temporary or permanent, default temporary
# expose: Boolean indicating whether the flag should be exposed to callers; default false

- name: Backend Example
description: A permanent backend example boolean flag
key: backendExample
default: false
contact: Gavin Cabbage
lifetime: permanent

- name: Frontend Example
description: A temporary frontend example integer flag
key: frontendExample
default: 42
contact: Gavin Cabbage
expose: true
17 changes: 17 additions & 0 deletions http/api_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/influxdata/influxdb/v2/authorizer"
"github.com/influxdata/influxdb/v2/chronograf/server"
"github.com/influxdata/influxdb/v2/http/metric"
"github.com/influxdata/influxdb/v2/kit/feature"
"github.com/influxdata/influxdb/v2/kit/prom"
kithttp "github.com/influxdata/influxdb/v2/kit/transport/http"
"github.com/influxdata/influxdb/v2/query"
Expand Down Expand Up @@ -82,6 +83,7 @@ type APIBackend struct {
DocumentService influxdb.DocumentService
NotificationRuleStore influxdb.NotificationRuleStore
NotificationEndpointService influxdb.NotificationEndpointService
Flagger feature.Flagger
}

// PrometheusCollectors exposes the prometheus collectors associated with an APIBackend.
Expand Down Expand Up @@ -203,6 +205,7 @@ func NewAPIHandler(b *APIBackend, opts ...APIHandlerOptFn) *APIHandler {
userHandler := NewUserHandler(b.Logger, userBackend)
h.Mount(prefixMe, userHandler)
h.Mount(prefixUsers, userHandler)
h.Mount("/api/v2/flags", serveFlagsHandler(b.HTTPErrorHandler))

variableBackend := NewVariableBackend(b.Logger.With(zap.String("handler", "variable")), b)
variableBackend.VariableService = authorizer.NewVariableService(b.VariableService)
Expand Down Expand Up @@ -236,6 +239,7 @@ var apiLinks = map[string]interface{}{
"external": map[string]string{
"statusFeed": "https://www.influxdata.com/feed/json",
},
"flags": "/api/v2/flags",
"labels": "/api/v2/labels",
"variables": "/api/v2/variables",
"me": "/api/v2/me",
Expand Down Expand Up @@ -277,3 +281,16 @@ func serveLinksHandler(errorHandler influxdb.HTTPErrorHandler) http.Handler {
}
return http.HandlerFunc(fn)
}

func serveFlagsHandler(errorHandler influxdb.HTTPErrorHandler) http.Handler {
fn := func(w http.ResponseWriter, r *http.Request) {
var (
ctx = r.Context()
flags = feature.ExposedFlagsFromContext(ctx)
)
if err := encodeResponse(ctx, w, http.StatusOK, flags); err != nil {
errorHandler.HandleHTTPError(ctx, err, w)
}
}
return http.HandlerFunc(fn)
}
3 changes: 2 additions & 1 deletion http/platform_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"net/http"
"strings"

"github.com/influxdata/influxdb/v2/kit/feature"
kithttp "github.com/influxdata/influxdb/v2/kit/transport/http"
)

Expand All @@ -17,7 +18,7 @@ type PlatformHandler struct {
// NewPlatformHandler returns a platform handler that serves the API and associated assets.
func NewPlatformHandler(b *APIBackend, opts ...APIHandlerOptFn) *PlatformHandler {
h := NewAuthenticationHandler(b.Logger, b.HTTPErrorHandler)
h.Handler = NewAPIHandler(b, opts...)
h.Handler = feature.NewHandler(b.Logger, b.Flagger, feature.Flags(), NewAPIHandler(b, opts...))
h.AuthorizationService = b.AuthorizationService
h.SessionService = b.SessionService
h.SessionRenewDisabled = b.SessionRenewDisabled
Expand Down
27 changes: 27 additions & 0 deletions http/swagger.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4936,6 +4936,27 @@ paths:
application/json:
schema:
$ref: "#/components/schemas/Error"
/flags:
get:
operationId: GetFlags
tags:
- Users
summary: Return the feature flags for the currently authenticated user
parameters:
- $ref: '#/components/parameters/TraceSpan'
responses:
'200':
description: Feature flags for the currently authenticated user
content:
application/json:
schema:
$ref: "#/components/schemas/Flags"
default:
description: Unexpected error
content:
application/json:
schema:
$ref: "#/components/schemas/Error"
/me:
get:
operationId: GetMe
Expand Down Expand Up @@ -8138,6 +8159,9 @@ components:
type: array
items:
$ref: "#/components/schemas/User"
Flags:
type: object
additionalProperties: true
ResourceMember:
allOf:
- $ref: "#/components/schemas/User"
Expand Down Expand Up @@ -8223,6 +8247,9 @@ components:
me:
type: string
format: uri
flags:
type: string
format: uri
orgs:
type: string
format: uri
Expand Down
12 changes: 12 additions & 0 deletions kit/cli/viper.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,18 @@ func BindOptions(cmd *cobra.Command, opts []Opt) {
}
mustBindPFlag(o.Flag, flagset)
*destP = viper.GetStringSlice(envVar)
case *map[string]string:
var d map[string]string
if o.Default != nil {
d = o.Default.(map[string]string)
}
if hasShort {
flagset.StringToStringVarP(destP, o.Flag, string(o.Short), d, o.Desc)
Copy link
Contributor

Choose a reason for hiding this comment

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

Will d being nil cause a panic here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested with this flag both set and not and didn't run into any panics. It's just used as a default, and in launcher.go we don't use this map if its nil or empty anyway.

} else {
flagset.StringToStringVar(destP, o.Flag, d, o.Desc)
}
mustBindPFlag(o.Flag, flagset)
*destP = viper.GetStringMapString(envVar)
case pflag.Value:
if hasShort {
flagset.VarP(destP, o.Flag, string(o.Short), o.Desc)
Expand Down
Loading