From 684cd9375573f4324729dd2be17938b765f42a06 Mon Sep 17 00:00:00 2001 From: Christopher Speller Date: Tue, 27 Apr 2021 08:58:38 -0700 Subject: [PATCH] MM-34674 Adding config telemetry for feature flags. (#17456) * Adding config telemetry for feature flags. * Review fixes. --- config/client.go | 2 +- config/feature_flags.go | 23 ------------ config/feature_flags_test.go | 44 ----------------------- model/config.go | 2 +- model/feature_flags.go | 28 ++++++++++++++- model/feature_flags_test.go | 54 ++++++++++++++++++++++++++++ services/telemetry/telemetry.go | 9 +++++ services/telemetry/telemetry_test.go | 2 ++ 8 files changed, 94 insertions(+), 70 deletions(-) create mode 100644 model/feature_flags_test.go diff --git a/config/client.go b/config/client.go index 9154c4f266..ca6a29dcd7 100644 --- a/config/client.go +++ b/config/client.go @@ -353,7 +353,7 @@ func GenerateLimitedClientConfig(c *model.Config, telemetryID string, license *m } } - for key, value := range featureFlagsToMap(c.FeatureFlags) { + for key, value := range c.FeatureFlags.ToMap() { props["FeatureFlag"+key] = value } diff --git a/config/feature_flags.go b/config/feature_flags.go index f87d01ee84..4d8b1e13f8 100644 --- a/config/feature_flags.go +++ b/config/feature_flags.go @@ -100,29 +100,6 @@ func featureFlagsFromMap(featuresMap map[string]string, baseFeatureFlags model.F return baseFeatureFlags } -// featureFlagsToMap returns the feature flags as a map[string]string -// Supports boolean and string feature flags. -func featureFlagsToMap(featureFlags *model.FeatureFlags) map[string]string { - refStructVal := reflect.ValueOf(*featureFlags) - refStructType := reflect.TypeOf(*featureFlags) - ret := make(map[string]string) - for i := 0; i < refStructVal.NumField(); i++ { - refFieldVal := refStructVal.Field(i) - refFieldType := refStructType.Field(i) - if !refFieldVal.IsValid() { - continue - } - switch refFieldType.Type.Kind() { - case reflect.Bool: - ret[refFieldType.Name] = strconv.FormatBool(refFieldVal.Bool()) - default: - ret[refFieldType.Name] = refFieldVal.String() - } - } - - return ret -} - func getStructFields(s interface{}) []string { structType := reflect.TypeOf(s) fieldNames := make([]string, 0, structType.NumField()) diff --git a/config/feature_flags_test.go b/config/feature_flags_test.go index aa6bcaef10..cb1cf50471 100644 --- a/config/feature_flags_test.go +++ b/config/feature_flags_test.go @@ -109,47 +109,3 @@ func TestFeatureFlagsFromMap(t *testing.T) { }) } } - -func TestFeatureFlagsToMap(t *testing.T) { - for name, tc := range map[string]struct { - Flags model.FeatureFlags - TestFeatureValue string - }{ - "empty": { - TestFeatureValue: "", - Flags: model.FeatureFlags{}, - }, - "simple value": { - TestFeatureValue: "expectedvalue", - Flags: model.FeatureFlags{TestFeature: "expectedvalue"}, - }, - "empty value": { - TestFeatureValue: "", - Flags: model.FeatureFlags{TestFeature: ""}, - }, - } { - t.Run(name, func(t *testing.T) { - require.Equal(t, tc.TestFeatureValue, featureFlagsToMap(&tc.Flags)["TestFeature"]) - }) - } -} - -func TestFeatureFlagsToMapBool(t *testing.T) { - for name, tc := range map[string]struct { - Flags model.FeatureFlags - TestFeatureValue string - }{ - "false": { - TestFeatureValue: "false", - Flags: model.FeatureFlags{}, - }, - "true": { - TestFeatureValue: "true", - Flags: model.FeatureFlags{TestBoolFeature: true}, - }, - } { - t.Run(name, func(t *testing.T) { - require.Equal(t, tc.TestFeatureValue, featureFlagsToMap(&tc.Flags)["TestBoolFeature"]) - }) - } -} diff --git a/model/config.go b/model/config.go index 313574fe18..83501dd7c1 100644 --- a/model/config.go +++ b/model/config.go @@ -3143,7 +3143,7 @@ type Config struct { GuestAccountsSettings GuestAccountsSettings ImageProxySettings ImageProxySettings CloudSettings CloudSettings // telemetry: none - FeatureFlags *FeatureFlags `access:"*_read" json:",omitempty"` // telemetry: none + FeatureFlags *FeatureFlags `access:"*_read" json:",omitempty"` ImportSettings ImportSettings // telemetry: none ExportSettings ExportSettings } diff --git a/model/feature_flags.go b/model/feature_flags.go index 9828cfbddd..d8722333ca 100644 --- a/model/feature_flags.go +++ b/model/feature_flags.go @@ -3,7 +3,10 @@ package model -import "reflect" +import ( + "reflect" + "strconv" +) type FeatureFlags struct { // Exists only for unit and manual testing. @@ -72,3 +75,26 @@ func (f *FeatureFlags) Plugins() map[string]string { return pluginVersions } + +// ToMap returns the feature flags as a map[string]string +// Supports boolean and string feature flags. +func (f *FeatureFlags) ToMap() map[string]string { + refStructVal := reflect.ValueOf(*f) + refStructType := reflect.TypeOf(*f) + ret := make(map[string]string) + for i := 0; i < refStructVal.NumField(); i++ { + refFieldVal := refStructVal.Field(i) + if !refFieldVal.IsValid() { + continue + } + refFieldType := refStructType.Field(i) + switch refFieldType.Type.Kind() { + case reflect.Bool: + ret[refFieldType.Name] = strconv.FormatBool(refFieldVal.Bool()) + default: + ret[refFieldType.Name] = refFieldVal.String() + } + } + + return ret +} diff --git a/model/feature_flags_test.go b/model/feature_flags_test.go new file mode 100644 index 0000000000..991b3aaef5 --- /dev/null +++ b/model/feature_flags_test.go @@ -0,0 +1,54 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package model + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestFeatureFlagsToMap(t *testing.T) { + for name, tc := range map[string]struct { + Flags FeatureFlags + TestFeatureValue string + }{ + "empty": { + TestFeatureValue: "", + Flags: FeatureFlags{}, + }, + "simple value": { + TestFeatureValue: "expectedvalue", + Flags: FeatureFlags{TestFeature: "expectedvalue"}, + }, + "empty value": { + TestFeatureValue: "", + Flags: FeatureFlags{TestFeature: ""}, + }, + } { + t.Run(name, func(t *testing.T) { + require.Equal(t, tc.TestFeatureValue, tc.Flags.ToMap()["TestFeature"]) + }) + } +} + +func TestFeatureFlagsToMapBool(t *testing.T) { + for name, tc := range map[string]struct { + Flags FeatureFlags + TestFeatureValue string + }{ + "false": { + TestFeatureValue: "false", + Flags: FeatureFlags{}, + }, + "true": { + TestFeatureValue: "true", + Flags: FeatureFlags{TestBoolFeature: true}, + }, + } { + t.Run(name, func(t *testing.T) { + require.Equal(t, tc.TestFeatureValue, tc.Flags.ToMap()["TestBoolFeature"]) + }) + } +} diff --git a/services/telemetry/telemetry.go b/services/telemetry/telemetry.go index 339390838f..56bcf4c03c 100644 --- a/services/telemetry/telemetry.go +++ b/services/telemetry/telemetry.go @@ -66,6 +66,7 @@ const ( TrackConfigImageProxy = "config_image_proxy" TrackConfigBleve = "config_bleve" TrackConfigExport = "config_export" + TrackFeatureFlags = "config_feature_flags" TrackPermissionsGeneral = "permissions_general" TrackPermissionsSystemScheme = "permissions_system_scheme" TrackPermissionsTeamSchemes = "permissions_team_schemes" @@ -825,6 +826,14 @@ func (ts *TelemetryService) trackConfig() { ts.sendTelemetry(TrackConfigExport, map[string]interface{}{ "retention_days": *cfg.ExportSettings.RetentionDays, }) + + // Convert feature flags to map[string]interface{} for sending + flags := cfg.FeatureFlags.ToMap() + interfaceFlags := make(map[string]interface{}) + for k, v := range flags { + interfaceFlags[k] = v + } + ts.sendTelemetry(TrackFeatureFlags, interfaceFlags) } func (ts *TelemetryService) trackLicense() { diff --git a/services/telemetry/telemetry_test.go b/services/telemetry/telemetry_test.go index 2dee76524a..207ec8d5e1 100644 --- a/services/telemetry/telemetry_test.go +++ b/services/telemetry/telemetry_test.go @@ -369,6 +369,7 @@ func TestRudderTelemetry(t *testing.T) { TrackConfigExperimental, TrackConfigAnalytics, TrackConfigPlugin, + TrackFeatureFlags, TrackActivity, TrackServer, TrackConfigMessageExport, @@ -411,6 +412,7 @@ func TestRudderTelemetry(t *testing.T) { TrackConfigExperimental, TrackConfigAnalytics, TrackConfigPlugin, + TrackFeatureFlags, TrackActivity, TrackServer, TrackConfigMessageExport,