Update feature gate logging to include default on #4029
Conversation
After the 5.7 release, when AutoUserSchemaCreate was graduated to default on/true, we discovered that our current system (and the underlying featuregate implementation) treats features explicitly turned on by the user differently than features turned on by default. This PR updates that logging to make clear that the features are those specifically requested by the user (kept as a string to help debugging) and revises the 'ShowGates' function to include those set through defaults. Issues: [PGO-1824]
internal/feature/features.go
Outdated
| @@ -52,6 +55,7 @@ type Feature = featuregate.Feature | |||
| type Gate interface { | |||
| Enabled(Feature) bool | |||
| String() string | |||
There was a problem hiding this comment.
🔧 I misunderstood what upstream's String() does. Let's explain it here.
| String() string | |
| // String returns the features enabled or disabled by Set and SetFromMap. | |
| String() string |
🤔 Maybe we drop String from the interface and pivot fully to context functions?
// ShowAssigned returns the features enabled or disabled by Set and SetFromMap
// in the Gate contained in ctx.
func ShowAssigned(ctx context.Context) string
// ShowEnabled returns the features enabled in the Gate contained in ctx.
func ShowEnabled(ctx context.Context) stringThere was a problem hiding this comment.
The only place we use gate.String() is in main.go when we report what the user set -- but if we're just telling the user what they set, why not just print the env var?
There was a problem hiding this comment.
Whatever the shape on the way in, we have a canonical representation in the logs.
| @@ -69,7 +69,7 @@ func TestCheckForUpgrades(t *testing.T) { | |||
| assert.Equal(t, data.RegistrationToken, "speakFriend") | |||
| assert.Equal(t, data.BridgeClustersTotal, 2) | |||
| assert.Equal(t, data.PGOClustersTotal, 2) | |||
| assert.Equal(t, data.FeatureGatesEnabled, "TablespaceVolumes=true") | |||
| assert.Equal(t, data.FeatureGatesEnabled, "AutoCreateUserSchema=true,TablespaceVolumes=true") | |||
There was a problem hiding this comment.
👍🏻 This isn't Contains, but I don't mind seeing the format. 🌱 We can change the test if it is ever a problem.
internal/feature/features.go
Outdated
| @@ -52,6 +55,7 @@ type Feature = featuregate.Feature | |||
| type Gate interface { | |||
| Enabled(Feature) bool | |||
| String() string | |||
There was a problem hiding this comment.
Whatever the shape on the way in, we have a canonical representation in the logs.
Checklist:
Type of Changes:
What is the current behavior (link to any open issues here)?
After the 5.7 release, when AutoUserSchemaCreate was graduated to default on/true, we discovered that our current system (and the underlying featuregate implementation) treats features explicitly turned on by the user differently than features turned on by default.
What is the new behavior (if this is a feature change)?
This PR updates that logging to make clear that the features are those specifically requested by the user (kept as a string to help debugging) and revises the 'ShowGates' function to include those set through defaults.
Other Information:
Issues: [PGO-1824]