From acaf4173a296cb5e62b984c448fc2bb0d72b88f0 Mon Sep 17 00:00:00 2001
From: Trevor Whitney <trevorjwhitney@gmail.com>
Date: Thu, 7 Oct 2021 15:57:58 -0600
Subject: [PATCH] change gprc server defaults

Signed-off-by: Trevor Whitney <trevorjwhitney@gmail.com>
---
 pkg/loki/loki.go      | 23 ++++++++++++++++-
 pkg/loki/loki_test.go | 58 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 1 deletion(-)
 create mode 100644 pkg/loki/loki_test.go

diff --git a/pkg/loki/loki.go b/pkg/loki/loki.go
index d6c9f8959526..e0d0651ce935 100644
--- a/pkg/loki/loki.go
+++ b/pkg/loki/loki.go
@@ -87,7 +87,7 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) {
 		"The alias 'all' can be used in the list to load a number of core modules and will enable single-binary mode. ")
 	f.BoolVar(&c.AuthEnabled, "auth.enabled", true, "Set to false to disable auth.")
 
-	c.Server.RegisterFlags(f)
+	c.registerServerFlagsWithChangedDefaultValues(f)
 	c.Distributor.RegisterFlags(f)
 	c.Querier.RegisterFlags(f)
 	c.IngesterClient.RegisterFlags(f)
@@ -108,6 +108,27 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) {
 	c.QueryScheduler.RegisterFlags(f)
 }
 
+func (c *Config) registerServerFlagsWithChangedDefaultValues(fs *flag.FlagSet) {
+	throwaway := flag.NewFlagSet("throwaway", flag.PanicOnError)
+
+	// Register to throwaway flags first. Default values are remembered during registration and cannot be changed,
+	// but we can take values from throwaway flag set and reregister into supplied flags with new default values.
+	c.Server.RegisterFlags(throwaway)
+
+	throwaway.VisitAll(func(f *flag.Flag) {
+		// Ignore errors when setting new values. We have a test to verify that it works.
+		switch f.Name {
+		case "server.grpc.keepalive.min-time-between-pings":
+			_ = f.Value.Set("10s")
+
+		case "server.grpc.keepalive.ping-without-stream-allowed":
+			_ = f.Value.Set("true")
+		}
+
+		fs.Var(f.Value, f.Name, f.Usage)
+	})
+}
+
 // Clone takes advantage of pass-by-value semantics to return a distinct *Config.
 // This is primarily used to parse a different flag set without mutating the original *Config.
 func (c *Config) Clone() flagext.Registerer {
diff --git a/pkg/loki/loki_test.go b/pkg/loki/loki_test.go
new file mode 100644
index 000000000000..7142d38d2712
--- /dev/null
+++ b/pkg/loki/loki_test.go
@@ -0,0 +1,58 @@
+package loki
+
+import (
+	"bytes"
+	"flag"
+	"io"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFlagDefaults(t *testing.T) {
+	c := Config{}
+
+	f := flag.NewFlagSet("test", flag.PanicOnError)
+	c.RegisterFlags(f)
+
+	buf := bytes.Buffer{}
+
+	f.SetOutput(&buf)
+	f.PrintDefaults()
+
+	const delim = '\n'
+
+	minTimeChecked := false
+	pingWithoutStreamChecked := false
+	for {
+		line, err := buf.ReadString(delim)
+		if err == io.EOF {
+			break
+		}
+
+		require.NoError(t, err)
+
+		if strings.Contains(line, "-server.grpc.keepalive.min-time-between-pings") {
+			nextLine, err := buf.ReadString(delim)
+			require.NoError(t, err)
+			assert.Contains(t, nextLine, "(default 10s)")
+			minTimeChecked = true
+		}
+
+		if strings.Contains(line, "-server.grpc.keepalive.ping-without-stream-allowed") {
+			nextLine, err := buf.ReadString(delim)
+			require.NoError(t, err)
+			assert.Contains(t, nextLine, "(default true)")
+			pingWithoutStreamChecked = true
+		}
+	}
+
+	require.True(t, minTimeChecked)
+	require.True(t, pingWithoutStreamChecked)
+
+	require.Equal(t, true, c.Server.GRPCServerPingWithoutStreamAllowed)
+	require.Equal(t, 10*time.Second, c.Server.GRPCServerMinTimeBetweenPings)
+}