From 7c47bd00c237e1cc6cbfb4c018ebdca9abc52147 Mon Sep 17 00:00:00 2001 From: Brandur Date: Thu, 7 Mar 2024 18:42:39 -0800 Subject: [PATCH] Generate default client IDs using host and client start time Here, move away from ULIDs for generating client IDs, and over to a custom ID generator that uses a combination of host and client start time. This has two benefits: * Host/start time are more descriptive for humans. It should give an operator a rough idea as to where the name of a client originated from, and if it's a client that's likely long dead (if the timestamp is very old). * Lets us drop our ULID package dependency, which we don't use for anything else. Generated IDs look like this for example: miniml2_local_2024_03_07T04_39_12 I've tried to arrange them so that on most systems you'll be able to double click on an ID to select the whole thing, which might be helpful while copying information or debugging a production problem. Characters like colons (`:`) are less friendly for double-click-to-select, so I've avoided them. Hyphens and dots aren't universally friendly for double click either (depends on program and configuration), so I've avoided those as well. --- CHANGELOG.md | 4 ++++ client.go | 30 +++++++++++++++----------- client_test.go | 11 ++++++++++ go.mod | 1 - go.sum | 3 --- internal/util/valutil/val_util.go | 10 +++++++++ internal/util/valutil/val_util_test.go | 10 +++++++++ 7 files changed, 52 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7ef0398..97a6602b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Changed default client IDs to be a combination of hostname and the time which the client started. This can still be changed by specifying `Config.ID`. [PR #255](https://github.com/riverqueue/river/pull/255). + ## [0.0.25] - 2024-03-01 ### Fixed diff --git a/client.go b/client.go index 1f88ba16..4a9b7d58 100644 --- a/client.go +++ b/client.go @@ -2,18 +2,16 @@ package river import ( "context" - "crypto/rand" "encoding/json" "errors" "fmt" "log/slog" "os" "regexp" + "strings" "sync" "time" - "github.com/oklog/ulid/v2" - "github.com/riverqueue/river/internal/baseservice" "github.com/riverqueue/river/internal/componentstatus" "github.com/riverqueue/river/internal/dblist" @@ -405,7 +403,7 @@ func NewClient[TTx any](driver riverdriver.Driver[TTx], config *Config) (*Client ErrorHandler: config.ErrorHandler, FetchCooldown: valutil.ValOrDefault(config.FetchCooldown, FetchCooldownDefault), FetchPollInterval: valutil.ValOrDefault(config.FetchPollInterval, FetchPollIntervalDefault), - ID: config.ID, + ID: valutil.ValOrDefaultFunc(config.ID, func() string { return defaultClientID(time.Now().UTC()) }), JobTimeout: valutil.ValOrDefault(config.JobTimeout, JobTimeoutDefault), Logger: logger, PeriodicJobs: config.PeriodicJobs, @@ -418,15 +416,6 @@ func NewClient[TTx any](driver riverdriver.Driver[TTx], config *Config) (*Client schedulerInterval: valutil.ValOrDefault(config.schedulerInterval, maintenance.JobSchedulerIntervalDefault), } - if config.ID == "" { - // Generate a random ULID for the client ID. - clientID, err := ulid.New(ulid.Now(), rand.Reader) - if err != nil { - return nil, err - } - config.ID = clientID.String() - } - if err := config.validate(); err != nil { return nil, err } @@ -566,6 +555,21 @@ func NewClient[TTx any](driver riverdriver.Driver[TTx], config *Config) (*Client return client, nil } +// Generates a default client ID using the current hostname and time. +func defaultClientID(startedAt time.Time) string { + // Dots, hyphens, and colons aren't particularly friendly for double click + // to select (depends on application and configuration), so avoid them all + // in favor of underscores. + const rfc3339Compact = "2006_01_02T15_04_05" + + host, _ := os.Hostname() + if host == "" { + host = "unknown_host" + } + + return strings.ReplaceAll(host, ".", "_") + "_" + startedAt.Format(rfc3339Compact) +} + // Start starts the client's job fetching and working loops. Once this is called, // the client will run in a background goroutine until stopped. All jobs are // run with a context inheriting from the provided context, but with a timeout diff --git a/client_test.go b/client_test.go index 067c1cc9..eb2d1b58 100644 --- a/client_test.go +++ b/client_test.go @@ -3388,6 +3388,17 @@ func TestID(t *testing.T) { }) } +func TestDefaultClientID(t *testing.T) { + t.Parallel() + + host, _ := os.Hostname() + require.NotEmpty(t, host) + + startedAt := time.Date(2024, time.March, 7, 4, 39, 12, 0, time.UTC) + + require.Equal(t, strings.ReplaceAll(host, ".", "_")+"_2024_03_07T04_39_12", defaultClientID(startedAt)) +} + type customInsertOptsJobArgs struct{} func (w *customInsertOptsJobArgs) Kind() string { return "customInsertOpts" } diff --git a/go.mod b/go.mod index 1f6ade55..1afff268 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,6 @@ require ( github.com/jackc/pgerrcode v0.0.0-20220416144525-469b46aa5efa github.com/jackc/pgx/v5 v5.5.3 github.com/jackc/puddle/v2 v2.2.1 - github.com/oklog/ulid/v2 v2.1.0 github.com/riverqueue/river/riverdriver v0.0.25 github.com/riverqueue/river/riverdriver/riverdatabasesql v0.0.25 github.com/riverqueue/river/riverdriver/riverpgxv5 v0.0.25 diff --git a/go.sum b/go.sum index 159f558d..9c9ed016 100644 --- a/go.sum +++ b/go.sum @@ -20,9 +20,6 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw= github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= -github.com/oklog/ulid/v2 v2.1.0 h1:+9lhoxAP56we25tyYETBBY1YLA2SaoLvUFgrP2miPJU= -github.com/oklog/ulid/v2 v2.1.0/go.mod h1:rcEKHmBBKfef9DhnvX7y1HZBYxjXb0cP5ExxNsTT1QQ= -github.com/pborman/getopt v0.0.0-20170112200414-7148bc3a4c30/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs= diff --git a/internal/util/valutil/val_util.go b/internal/util/valutil/val_util.go index 79aecda7..f4c52178 100644 --- a/internal/util/valutil/val_util.go +++ b/internal/util/valutil/val_util.go @@ -12,6 +12,16 @@ func ValOrDefault[T constraints.Integer | string](val, defaultVal T) T { return defaultVal } +// ValOrDefault returns the given value if it's non-zero, and otherwise invokes +// defaultFunc to produce a default value. +func ValOrDefaultFunc[T constraints.Integer | string](val T, defaultFunc func() T) T { + var zero T + if val != zero { + return val + } + return defaultFunc() +} + // FirstNonZero returns the first argument that is non-zero, or the zero value if // all are zero. func FirstNonZero[T constraints.Integer | ~string](values ...T) T { diff --git a/internal/util/valutil/val_util_test.go b/internal/util/valutil/val_util_test.go index 60ade8ee..ec7d0dc8 100644 --- a/internal/util/valutil/val_util_test.go +++ b/internal/util/valutil/val_util_test.go @@ -15,3 +15,13 @@ func TestValOrDefault(t *testing.T) { require.Equal(t, "default", ValOrDefault("", "default")) require.Equal(t, "hello", ValOrDefault("hello", "default")) } + +func TestValOrDefaultFunc(t *testing.T) { + t.Parallel() + + require.Equal(t, 1, ValOrDefaultFunc(0, func() int { return 1 })) + require.Equal(t, 5, ValOrDefaultFunc(5, func() int { return 1 })) + + require.Equal(t, "default", ValOrDefaultFunc("", func() string { return "default" })) + require.Equal(t, "hello", ValOrDefaultFunc("hello", func() string { return "default" })) +}