Skip to content
This repository was archived by the owner on Apr 2, 2024. It is now read-only.

Commit 4ed7edc

Browse files
antekresicantekresic
committed
Add logging usage of deprecated flags with recommendations
In version 0.11.0 we will be removing a lot of old CLI flags. Users will have to update their configuration to new flag names when that happens. This is the first step in handling this transition.
1 parent 2ba6586 commit 4ed7edc

File tree

6 files changed

+181
-116
lines changed

6 files changed

+181
-116
lines changed

‎cmd/promscale/main.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
)
1616

1717
func main() {
18+
log.InitDefault()
1819
args := os.Args[1:]
1920
if shouldProceed := runner.ParseArgs(args); !shouldProceed {
2021
os.Exit(0)

‎pkg/log/log.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,17 @@ func ParseFlags(fs *flag.FlagSet, cfg *Config) *Config {
4343
return cfg
4444
}
4545

46+
// InitDefault is used to start the logger with sane defaults before we can configure it.
47+
// It's useful in instances where we want to log stuff before the configuration
48+
// has been successfully parsed. Calling Init function later on overrides this.
49+
func InitDefault() {
50+
l := level.NewFilter(
51+
log.NewLogfmtLogger(log.NewSyncWriter(os.Stderr)),
52+
level.AllowInfo(),
53+
)
54+
logger = log.With(l, "ts", timestampFormat, "caller", log.Caller(4))
55+
}
56+
4657
// Init starts logging given the configuration. By default, it uses logfmt format
4758
// and minimum logging level.
4859
func Init(cfg Config) error {

‎pkg/runner/flags.go

Lines changed: 124 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -46,58 +46,62 @@ type Config struct {
4646
StartupOnly bool
4747
}
4848

49-
var flagAliases = map[string][]string{
50-
"auth.tls-cert-file": {"tls-cert-file"},
51-
"auth.tls-key-file": {"tls-key-file"},
52-
"cache.memory-target": {"memory-target"},
53-
"db.app": {"app"},
54-
"db.connection-timeout": {"db-connection-timeout"},
55-
"db.connections-max": {"db-connections-max"},
56-
"db.host": {"db-host"},
57-
"db.name": {"db-name"},
58-
"db.num-writer-connections": {"db-writer-connection-concurrency"},
59-
"db.password": {"db-password"},
60-
"db.port": {"db-port"},
61-
"db.read-only": {"read-only"},
62-
"db.ssl-mode": {"db-ssl-mode"},
63-
"db.statements-cache": {"db-statements-cache"},
64-
"db.uri": {"db-uri"},
65-
"db.user": {"db-user"},
66-
"metrics.async-acks": {"async-acks"},
67-
"metrics.cache.exemplar.size": {"exemplar-cache-size"},
68-
"metrics.cache.labels.size": {"labels-cache-size"},
69-
"metrics.cache.metrics.size": {"metrics-cache-size"},
70-
"metrics.cache.series.initial-size": {"series-cache-initial-size"},
71-
"metrics.cache.series.max-bytes": {"series-cache-max-bytes"},
72-
"metrics.high-availability": {"high-availability"},
73-
"metrics.ignore-samples-written-to-compressed-chunks": {"ignore-samples-written-to-compressed-chunks"},
74-
"metrics.multi-tenancy": {"multi-tenancy"},
75-
"metrics.multi-tenancy.allow-non-tenants": {"multi-tenancy-allow-non-tenants"},
76-
"metrics.multi-tenancy.valid-tenants": {"multi-tenancy-valid-tenants"},
77-
"metrics.promql.default-subquery-step-interval": {"promql-default-subquery-step-interval"},
78-
"metrics.promql.lookback-delta": {"promql-lookback-delta"},
79-
"metrics.promql.max-points-per-ts": {"promql-max-points-per-ts"},
80-
"metrics.promql.max-samples": {"promql-max-samples"},
81-
"metrics.promql.query-timeout": {"promql-query-timeout"},
82-
"startup.install-extensions": {"install-extensions"},
83-
"startup.upgrade-extensions": {"upgrade-extensions"},
84-
"startup.upgrade-prerelease-extensions": {"upgrade-prerelease-extensions"},
85-
"startup.use-schema-version-lease": {"use-schema-version-lease"},
86-
"telemetry.log.format": {"log-format"},
87-
"telemetry.log.level": {"log-level"},
88-
"telemetry.log.throughput-report-interval": {"tput-report"},
89-
"thanos.store-api.server-address": {"thanos-store-api-listen-address"},
90-
"tracing.otlp.server-address": {"otlp-grpc-server-listen-address"},
91-
"web.auth.bearer-token": {"bearer-token"},
92-
"web.auth.bearer-token-file": {"bearer-token-file"},
93-
"web.auth.password": {"auth-password"},
94-
"web.auth.password-file": {"auth-password-file"},
95-
"web.auth.username": {"auth-username"},
96-
"web.cors-origin": {"web-cors-origin"},
97-
"web.enable-admin-api": {"web-enable-admin-api"},
98-
"web.listen-address": {"web-listen-address"},
99-
"web.telemetry-path": {"web-telemetry-path"},
100-
}
49+
var (
50+
aliasDescTemplate = "Alias for -%s flag. "
51+
deprecatedFlagSuffix = "(DEPRECATED) Will be removed in version 0.11.0"
52+
flagAliases = map[string][]string{
53+
"auth.tls-cert-file": {"tls-cert-file"},
54+
"auth.tls-key-file": {"tls-key-file"},
55+
"cache.memory-target": {"memory-target"},
56+
"db.app": {"app"},
57+
"db.connection-timeout": {"db-connection-timeout"},
58+
"db.connections-max": {"db-connections-max"},
59+
"db.host": {"db-host"},
60+
"db.name": {"db-name"},
61+
"db.num-writer-connections": {"db-writer-connection-concurrency"},
62+
"db.password": {"db-password"},
63+
"db.port": {"db-port"},
64+
"db.read-only": {"read-only"},
65+
"db.ssl-mode": {"db-ssl-mode"},
66+
"db.statements-cache": {"db-statements-cache"},
67+
"db.uri": {"db-uri"},
68+
"db.user": {"db-user"},
69+
"metrics.async-acks": {"async-acks"},
70+
"metrics.cache.exemplar.size": {"exemplar-cache-size"},
71+
"metrics.cache.labels.size": {"labels-cache-size"},
72+
"metrics.cache.metrics.size": {"metrics-cache-size"},
73+
"metrics.cache.series.initial-size": {"series-cache-initial-size"},
74+
"metrics.cache.series.max-bytes": {"series-cache-max-bytes"},
75+
"metrics.high-availability": {"high-availability"},
76+
"metrics.ignore-samples-written-to-compressed-chunks": {"ignore-samples-written-to-compressed-chunks"},
77+
"metrics.multi-tenancy": {"multi-tenancy"},
78+
"metrics.multi-tenancy.allow-non-tenants": {"multi-tenancy-allow-non-tenants"},
79+
"metrics.multi-tenancy.valid-tenants": {"multi-tenancy-valid-tenants"},
80+
"metrics.promql.default-subquery-step-interval": {"promql-default-subquery-step-interval"},
81+
"metrics.promql.lookback-delta": {"promql-lookback-delta"},
82+
"metrics.promql.max-points-per-ts": {"promql-max-points-per-ts"},
83+
"metrics.promql.max-samples": {"promql-max-samples"},
84+
"metrics.promql.query-timeout": {"promql-query-timeout"},
85+
"startup.install-extensions": {"install-extensions"},
86+
"startup.upgrade-extensions": {"upgrade-extensions"},
87+
"startup.upgrade-prerelease-extensions": {"upgrade-prerelease-extensions"},
88+
"startup.use-schema-version-lease": {"use-schema-version-lease"},
89+
"telemetry.log.format": {"log-format"},
90+
"telemetry.log.level": {"log-level"},
91+
"telemetry.log.throughput-report-interval": {"tput-report"},
92+
"thanos.store-api.server-address": {"thanos-store-api-listen-address"},
93+
"tracing.otlp.server-address": {"otlp-grpc-server-listen-address"},
94+
"web.auth.bearer-token": {"bearer-token"},
95+
"web.auth.bearer-token-file": {"bearer-token-file"},
96+
"web.auth.password": {"auth-password"},
97+
"web.auth.password-file": {"auth-password-file"},
98+
"web.auth.username": {"auth-username"},
99+
"web.cors-origin": {"web-cors-origin"},
100+
"web.enable-admin-api": {"web-enable-admin-api"},
101+
"web.listen-address": {"web-listen-address"},
102+
"web.telemetry-path": {"web-telemetry-path"},
103+
}
104+
)
101105

102106
func ParseFlags(cfg *Config, args []string) (*Config, error) {
103107
var (
@@ -121,7 +125,7 @@ func ParseFlags(cfg *Config, args []string) (*Config, error) {
121125
fs.StringVar(&cfg.OTLPGRPCListenAddr, "tracing.otlp.server-address", "", "Address to listen on for OTLP GRPC server.")
122126
fs.StringVar(&corsOriginFlag, "web.cors-origin", ".*", `Regex for CORS origin. It is fully anchored. Example: 'https?://(domain1|domain2)\.com'`)
123127
fs.DurationVar(&cfg.ThroughputInterval, "telemetry.log.throughput-report-interval", time.Second, "Duration interval at which throughput should be reported. Setting duration to `0` will disable reporting throughput, otherwise, an interval with unit must be provided, e.g. `10s` or `3m`.")
124-
fs.StringVar(&migrateOption, "migrate", "true", "Update the Prometheus SQL schema to the latest version. Valid options are: [true, false, only]. (DEPRECATED) Will be removed in version 0.11.0")
128+
fs.StringVar(&migrateOption, "migrate", "true", fmt.Sprintf("Update the Prometheus SQL schema to the latest version. Valid options are: [true, false, only]. %s", deprecatedFlagSuffix))
125129
fs.StringVar(&cfg.DatasetConfig, "startup.dataset.config", "", "Dataset configuration in YAML format for Promscale. It is used for setting various dataset configuration like default metric chunk interval")
126130
fs.BoolVar(&cfg.StartupOnly, "startup.only", false, "Only run startup configuration with Promscale (i.e. migrate) and exit. Can be used to run promscale as an init container for HA setups.")
127131
fs.BoolVar(&skipMigrate, "startup.skip-migrate", false, "Skip migrating Promscale SQL schema to latest version on startup.")
@@ -133,7 +137,7 @@ func ParseFlags(cfg *Config, args []string) (*Config, error) {
133137
fs.StringVar(&cfg.TLSCertFile, "auth.tls-cert-file", "", "TLS Certificate file used for server authentication, leave blank to disable TLS. NOTE: this option is used for all servers that Promscale runs (web and GRPC).")
134138
fs.StringVar(&cfg.TLSKeyFile, "auth.tls-key-file", "", "TLS Key file for server authentication, leave blank to disable TLS. NOTE: this option is used for all servers that Promscale runs (web and GRPC).")
135139

136-
util.AddAliases(fs, flagAliases, "(DEPRECATED) Will be removed in version 0.11.0")
140+
addAliases(fs, flagAliases, deprecatedFlagSuffix)
137141

138142
if err := util.ParseEnv("PROMSCALE", fs); err != nil {
139143
return nil, fmt.Errorf("error parsing env variables: %w", err)
@@ -147,6 +151,23 @@ func ParseFlags(cfg *Config, args []string) (*Config, error) {
147151
return nil, fmt.Errorf("configuration error: %w", err)
148152
}
149153

154+
// Warn about deprecated flags.
155+
if flagsForUpdate := deprecatedFlagsUsed(fs); len(flagsForUpdate) > 0 {
156+
log.Warn("msg", "Using deprecated flags that will be dropped in a future version. Please update to new flag names.")
157+
for oldFlag, newFlag := range flagsForUpdate {
158+
log.Warn("msg", fmt.Sprintf("Update deprecated flag %s to new flag %s", oldFlag, newFlag))
159+
}
160+
}
161+
// Migrate flag is a special case that needs to be logged separately.
162+
switch migrateOption {
163+
case "false":
164+
log.Warn("msg", "Using deprecated flag `migrate` set to `false` which will be dropped in a future version. "+
165+
"Please update you configuration by using `startup.skip-migrate` flag.")
166+
case "only":
167+
log.Warn("msg", "Using deprecated flag `migrate` set to `only` which will be dropped in a future version. "+
168+
"Please update you configuration by using `startup.only` flag.")
169+
}
170+
150171
// Checking if TLS files are not both set or both empty.
151172
if (cfg.TLSCertFile != "") != (cfg.TLSKeyFile != "") {
152173
return nil, fmt.Errorf("both TLS Ceriticate File and TLS Key File need to be provided for a valid TLS configuration")
@@ -225,3 +246,52 @@ func validate(cfg *Config) error {
225246
}
226247
return nil
227248
}
249+
250+
func addAliases(fs *flag.FlagSet, aliases map[string][]string, descSuffix string) {
251+
aliasDescFormat := aliasDescTemplate + descSuffix
252+
set := false
253+
fs.VisitAll(func(f *flag.Flag) {
254+
if flagAliases, ok := aliases[f.Name]; ok {
255+
set = false
256+
for _, alias := range flagAliases {
257+
set = true
258+
fs.Var(f.Value, alias, fmt.Sprintf(aliasDescFormat, f.Name))
259+
}
260+
261+
if !set {
262+
panic(fmt.Sprintf("trying to set an flag alias for a flag that is missing: %s", f.Name))
263+
}
264+
}
265+
})
266+
}
267+
268+
func deprecatedFlagsUsed(fs *flag.FlagSet) map[string]string {
269+
var (
270+
result = make(map[string]string)
271+
newFlagName string
272+
count int
273+
)
274+
275+
fs.Visit(func(f *flag.Flag) {
276+
if strings.Contains(f.Usage, deprecatedFlagSuffix) {
277+
i, err := fmt.Sscanf(f.Usage, aliasDescTemplate, &newFlagName)
278+
279+
switch {
280+
case f.Name == "migrate":
281+
// Migrate flag is a special case since it's not just a rename
282+
// of an old flag. We handle it separately when logging
283+
// use of deprecated flags.
284+
return
285+
case err != nil:
286+
fallthrough
287+
case i != 1:
288+
panic("deprecated flag usage not set in the correct format")
289+
}
290+
291+
result[f.Name] = newFlagName
292+
count++
293+
}
294+
})
295+
296+
return result
297+
}

‎pkg/runner/flags_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,14 @@
55
package runner
66

77
import (
8+
"flag"
9+
"fmt"
810
"io/ioutil"
911
"os"
1012
"reflect"
1113
"testing"
14+
15+
"github.com/stretchr/testify/require"
1216
)
1317

1418
func TestParseFlags(t *testing.T) {
@@ -340,3 +344,43 @@ func TestParseFlagsConfigPrecedence(t *testing.T) {
340344
})
341345
}
342346
}
347+
348+
func TestAddAlias(t *testing.T) {
349+
aliases := map[string][]string{
350+
"first_flag": {"first_flag_alias", "first_flag_another_alias"},
351+
"second_flag": {"second_flag_alias"},
352+
}
353+
354+
fs := flag.NewFlagSet("test", flag.ContinueOnError)
355+
fs.Bool("first_flag", true, "First flag desc")
356+
fs.String("second_flag", "empty", "Second flag desc")
357+
358+
descSuffix := "Test description."
359+
360+
addAliases(fs, aliases, descSuffix)
361+
362+
ff := fs.Lookup("first_flag")
363+
sf := fs.Lookup("second_flag")
364+
365+
require.NotNil(t, ff)
366+
require.NotNil(t, sf)
367+
368+
alias := fs.Lookup("first_flag_alias")
369+
require.NotNil(t, alias)
370+
require.Equal(t, ff.Value, alias.Value)
371+
require.Equal(t, ff.DefValue, alias.DefValue)
372+
require.Equal(t, alias.Usage, fmt.Sprintf(aliasDescTemplate+descSuffix, ff.Name))
373+
374+
alias = fs.Lookup("first_flag_another_alias")
375+
require.NotNil(t, alias)
376+
require.Equal(t, ff.Value, alias.Value)
377+
require.Equal(t, ff.DefValue, alias.DefValue)
378+
require.Equal(t, alias.Usage, fmt.Sprintf(aliasDescTemplate+descSuffix, ff.Name))
379+
380+
alias = fs.Lookup("second_flag_alias")
381+
require.NotNil(t, alias)
382+
require.Equal(t, sf.Value, alias.Value)
383+
require.Equal(t, sf.DefValue, alias.DefValue)
384+
require.Equal(t, alias.Usage, fmt.Sprintf(aliasDescTemplate+descSuffix, sf.Name))
385+
386+
}

‎pkg/util/util.go

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,7 @@ import (
1111
"strings"
1212
)
1313

14-
const (
15-
PromNamespace = "promscale"
16-
aliasDescTemplate = "Alias for -%s flag. "
17-
)
14+
const PromNamespace = "promscale"
1815

1916
// ParseEnv takes a prefix string p and *flag.FlagSet. Each flag
2017
// in the FlagSet is exposed as an upper case environment variable
@@ -66,21 +63,3 @@ func ParseEnv(p string, fs *flag.FlagSet) error {
6663

6764
return err
6865
}
69-
70-
func AddAliases(fs *flag.FlagSet, aliases map[string][]string, descSuffix string) {
71-
aliasDescFormat := aliasDescTemplate + descSuffix
72-
set := false
73-
fs.VisitAll(func(f *flag.Flag) {
74-
if flagAliases, ok := aliases[f.Name]; ok {
75-
set = false
76-
for _, alias := range flagAliases {
77-
set = true
78-
fs.Var(f.Value, alias, fmt.Sprintf(aliasDescFormat, f.Name))
79-
}
80-
81-
if !set {
82-
panic(fmt.Sprintf("trying to set an flag alias for a flag that is missing: %s", f.Name))
83-
}
84-
}
85-
})
86-
}

‎pkg/util/util_test.go

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -129,46 +129,6 @@ func TestParseEnv(t *testing.T) {
129129

130130
}
131131

132-
func TestAddAlias(t *testing.T) {
133-
aliases := map[string][]string{
134-
"first_flag": {"first_flag_alias", "first_flag_another_alias"},
135-
"second_flag": {"second_flag_alias"},
136-
}
137-
138-
fs := flag.NewFlagSet("test", flag.ContinueOnError)
139-
fs.Bool("first_flag", true, "First flag desc")
140-
fs.String("second_flag", "empty", "Second flag desc")
141-
142-
descSuffix := "Test description."
143-
144-
AddAliases(fs, aliases, descSuffix)
145-
146-
ff := fs.Lookup("first_flag")
147-
sf := fs.Lookup("second_flag")
148-
149-
require.NotNil(t, ff)
150-
require.NotNil(t, sf)
151-
152-
alias := fs.Lookup("first_flag_alias")
153-
require.NotNil(t, alias)
154-
require.Equal(t, ff.Value, alias.Value)
155-
require.Equal(t, ff.DefValue, alias.DefValue)
156-
require.Equal(t, alias.Usage, fmt.Sprintf(aliasDescTemplate+descSuffix, ff.Name))
157-
158-
alias = fs.Lookup("first_flag_another_alias")
159-
require.NotNil(t, alias)
160-
require.Equal(t, ff.Value, alias.Value)
161-
require.Equal(t, ff.DefValue, alias.DefValue)
162-
require.Equal(t, alias.Usage, fmt.Sprintf(aliasDescTemplate+descSuffix, ff.Name))
163-
164-
alias = fs.Lookup("second_flag_alias")
165-
require.NotNil(t, alias)
166-
require.Equal(t, sf.Value, alias.Value)
167-
require.Equal(t, sf.DefValue, alias.DefValue)
168-
require.Equal(t, alias.Usage, fmt.Sprintf(aliasDescTemplate+descSuffix, sf.Name))
169-
170-
}
171-
172132
func TestExtractMetricValue(t *testing.T) {
173133
metric := prometheus.NewGauge(prometheus.GaugeOpts{Namespace: "test", Name: "extraction"})
174134

0 commit comments

Comments
 (0)