Skip to content

Commit 75f6df5

Browse files
EmilianoSanchezEmilianoSanchez
committed
Fixed some warning logs for flag sets, to properly print the method name when evaluating by flag sets
1 parent 709922e commit 75f6df5

File tree

4 files changed

+53
-51
lines changed

4 files changed

+53
-51
lines changed

‎src/logger/constants.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,8 @@ export const WARN_SPLITS_FILTER_EMPTY = 221;
9797
export const WARN_SDK_KEY = 222;
9898
export const STREAMING_PARSING_MY_SEGMENTS_UPDATE_V2 = 223;
9999
export const STREAMING_PARSING_SPLIT_UPDATE = 224;
100-
export const WARN_SPLITS_FILTER_INVALID_SET = 225;
101-
export const WARN_SPLITS_FILTER_LOWERCASE_SET = 226;
100+
export const WARN_INVALID_FLAGSET = 225;
101+
export const WARN_LOWERCASE_FLAGSET = 226;
102102
export const WARN_FLAGSET_NOT_CONFIGURED = 227;
103103
export const WARN_FLAGSET_WITHOUT_FLAGS = 228;
104104

‎src/logger/messages/warn.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export const codesWarn: [number, string][] = codesError.concat([
3434

3535
[c.STREAMING_PARSING_MY_SEGMENTS_UPDATE_V2, c.LOG_PREFIX_SYNC_STREAMING + 'Fetching MySegments due to an error processing %s notification: %s'],
3636
[c.STREAMING_PARSING_SPLIT_UPDATE, c.LOG_PREFIX_SYNC_STREAMING + 'Fetching SplitChanges due to an error processing SPLIT_UPDATE notification: %s'],
37-
[c.WARN_SPLITS_FILTER_INVALID_SET, c.LOG_PREFIX_SETTINGS + ': you passed %s, flag set must adhere to the regular expressions %s. This means a flag set must start with a letter or number, be in lowercase, alphanumeric and have a max length of 50 characters. %s was discarded.'],
38-
[c.WARN_SPLITS_FILTER_LOWERCASE_SET, c.LOG_PREFIX_SETTINGS + ': flag set %s should be all lowercase - converting string to lowercase.'],
37+
[c.WARN_INVALID_FLAGSET, '%s: you passed %s, flag set must adhere to the regular expressions %s. This means a flag set must start with a letter or number, be in lowercase, alphanumeric and have a max length of 50 characters. %s was discarded.'],
38+
[c.WARN_LOWERCASE_FLAGSET, '%s: flag set %s should be all lowercase - converting string to lowercase.'],
3939
[c.WARN_FLAGSET_WITHOUT_FLAGS, '%s: you passed %s flag set that does not contain cached feature flag names. Please double check what flag sets are in use in the Split user interface.'],
4040
]);

‎src/utils/settingsValidation/__tests__/splitFilters.spec.ts‎

Lines changed: 42 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { splitFilters, queryStrings, groupedFilters } from '../../../__tests__/m
66

77
// Test target
88
import { validateFlagSets, validateSplitFilters } from '../splitFilters';
9-
import { SETTINGS_SPLITS_FILTER, ERROR_INVALID, ERROR_EMPTY_ARRAY, ERROR_SETS_FILTER_EXCLUSIVE, WARN_SPLITS_FILTER_INVALID, WARN_SPLITS_FILTER_EMPTY, WARN_TRIMMING, WARN_SPLITS_FILTER_INVALID_SET, WARN_SPLITS_FILTER_LOWERCASE_SET, WARN_FLAGSET_NOT_CONFIGURED, WARN_SPLITS_FILTER_IGNORED } from '../../../logger/constants';
9+
import { SETTINGS_SPLITS_FILTER, ERROR_INVALID, ERROR_EMPTY_ARRAY, ERROR_SETS_FILTER_EXCLUSIVE, WARN_SPLITS_FILTER_INVALID, WARN_SPLITS_FILTER_EMPTY, WARN_TRIMMING, WARN_INVALID_FLAGSET, WARN_LOWERCASE_FLAGSET, WARN_FLAGSET_NOT_CONFIGURED, WARN_SPLITS_FILTER_IGNORED, LOG_PREFIX_SETTINGS } from '../../../logger/constants';
1010

1111
describe('validateSplitFilters', () => {
1212

@@ -113,18 +113,18 @@ describe('validateSplitFilters', () => {
113113
expect(loggerMock.error.mock.calls[0]).toEqual([ERROR_SETS_FILTER_EXCLUSIVE]);
114114

115115
expect(validateSplitFilters(loggerMock, splitFilters[7], LOCALHOST_MODE)).toEqual(getOutput(7)); // lowercase and regexp
116-
expect(loggerMock.warn.mock.calls[3]).toEqual([WARN_SPLITS_FILTER_LOWERCASE_SET, ['seT_c']]); // lowercase
117-
expect(loggerMock.warn.mock.calls[4]).toEqual([WARN_SPLITS_FILTER_LOWERCASE_SET, ['set_B']]); // lowercase
118-
expect(loggerMock.warn.mock.calls[5]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set_ 1', regexp, 'set_ 1']]); // empty spaces
119-
expect(loggerMock.warn.mock.calls[6]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set _3', regexp, 'set _3']]); // empty spaces
120-
expect(loggerMock.warn.mock.calls[7]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['_set_2', regexp, '_set_2']]); // start with a letter
121-
expect(loggerMock.warn.mock.calls[8]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set_1234567890_1234567890_234567890_1234567890_1234567890', regexp, 'set_1234567890_1234567890_234567890_1234567890_1234567890']]); // max of 50 characters
116+
expect(loggerMock.warn.mock.calls[3]).toEqual([WARN_LOWERCASE_FLAGSET, [LOG_PREFIX_SETTINGS, 'seT_c']]); // lowercase
117+
expect(loggerMock.warn.mock.calls[4]).toEqual([WARN_LOWERCASE_FLAGSET, [LOG_PREFIX_SETTINGS, 'set_B']]); // lowercase
118+
expect(loggerMock.warn.mock.calls[5]).toEqual([WARN_INVALID_FLAGSET, [LOG_PREFIX_SETTINGS, 'set_ 1', regexp, 'set_ 1']]); // empty spaces
119+
expect(loggerMock.warn.mock.calls[6]).toEqual([WARN_INVALID_FLAGSET, [LOG_PREFIX_SETTINGS, 'set _3', regexp, 'set _3']]); // empty spaces
120+
expect(loggerMock.warn.mock.calls[7]).toEqual([WARN_INVALID_FLAGSET, [LOG_PREFIX_SETTINGS, '_set_2', regexp, '_set_2']]); // start with a letter
121+
expect(loggerMock.warn.mock.calls[8]).toEqual([WARN_INVALID_FLAGSET, [LOG_PREFIX_SETTINGS, 'set_1234567890_1234567890_234567890_1234567890_1234567890', regexp, 'set_1234567890_1234567890_234567890_1234567890_1234567890']]); // max of 50 characters
122122
expect(loggerMock.error.mock.calls[1]).toEqual([ERROR_SETS_FILTER_EXCLUSIVE]);
123123

124124
expect(validateSplitFilters(loggerMock, splitFilters[8], PRODUCER_MODE)).toEqual(getOutput(8)); // lowercase and dedupe
125-
expect(loggerMock.warn.mock.calls[9]).toEqual([WARN_SPLITS_FILTER_LOWERCASE_SET, ['SET_2']]); // lowercase
126-
expect(loggerMock.warn.mock.calls[10]).toEqual([WARN_SPLITS_FILTER_LOWERCASE_SET, ['set_B']]); // lowercase
127-
expect(loggerMock.warn.mock.calls[11]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set_3!', regexp, 'set_3!']]); // special character
125+
expect(loggerMock.warn.mock.calls[9]).toEqual([WARN_LOWERCASE_FLAGSET, [LOG_PREFIX_SETTINGS, 'SET_2']]); // lowercase
126+
expect(loggerMock.warn.mock.calls[10]).toEqual([WARN_LOWERCASE_FLAGSET, [LOG_PREFIX_SETTINGS, 'set_B']]); // lowercase
127+
expect(loggerMock.warn.mock.calls[11]).toEqual([WARN_INVALID_FLAGSET, [LOG_PREFIX_SETTINGS, 'set_3!', regexp, 'set_3!']]); // special character
128128
expect(loggerMock.error.mock.calls[2]).toEqual([ERROR_SETS_FILTER_EXCLUSIVE]);
129129

130130
expect(loggerMock.warn.mock.calls.length).toEqual(12);
@@ -133,69 +133,70 @@ describe('validateSplitFilters', () => {
133133

134134
test('validateFlagSets - Flag set validation for evaluations', () => {
135135

136-
let flagSetsFilter = ['set_1', 'set_2'];
136+
const flagSetsFilter = ['set_1', 'set_2'];
137+
const METHOD_NAME = 'test_method';
137138

138139
// empty array
139-
expect(validateFlagSets(loggerMock, 'test_method', [], flagSetsFilter)).toEqual([]);
140+
expect(validateFlagSets(loggerMock, METHOD_NAME, [], flagSetsFilter)).toEqual([]);
140141

141142
// must start with a letter or number
142-
expect(validateFlagSets(loggerMock, 'test_method', ['_set_1'], flagSetsFilter)).toEqual([]);
143-
expect(loggerMock.warn.mock.calls[0]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['_set_1', regexp, '_set_1']]);
143+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['_set_1'], flagSetsFilter)).toEqual([]);
144+
expect(loggerMock.warn.mock.calls[0]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, '_set_1', regexp, '_set_1']]);
144145

145146
// can contain _a-z0-9
146-
expect(validateFlagSets(loggerMock, 'test_method', ['set*1'], flagSetsFilter)).toEqual([]);
147-
expect(loggerMock.warn.mock.calls[1]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*1', regexp, 'set*1']]);
147+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set*1'], flagSetsFilter)).toEqual([]);
148+
expect(loggerMock.warn.mock.calls[1]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, 'set*1', regexp, 'set*1']]);
148149

149150
// have a max length of 50 characters
150151
const longName = '1234567890_1234567890_1234567890_1234567890_1234567890';
151-
expect(validateFlagSets(loggerMock, 'test_method', [longName], flagSetsFilter)).toEqual([]);
152-
expect(loggerMock.warn.mock.calls[2]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, [longName, regexp, longName]]);
152+
expect(validateFlagSets(loggerMock, METHOD_NAME, [longName], flagSetsFilter)).toEqual([]);
153+
expect(loggerMock.warn.mock.calls[2]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, longName, regexp, longName]]);
153154

154155
// both set names invalid -> empty list & warn
155-
expect(validateFlagSets(loggerMock, 'test_method', ['set*1', 'set*3'], flagSetsFilter)).toEqual([]);
156-
expect(loggerMock.warn.mock.calls[3]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*1', regexp, 'set*1']]);
157-
expect(loggerMock.warn.mock.calls[4]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*3', regexp, 'set*3']]);
156+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set*1', 'set*3'], flagSetsFilter)).toEqual([]);
157+
expect(loggerMock.warn.mock.calls[3]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, 'set*1', regexp, 'set*1']]);
158+
expect(loggerMock.warn.mock.calls[4]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, 'set*3', regexp, 'set*3']]);
158159

159160
// only set_1 is valid => [set_1] & warn
160-
expect(validateFlagSets(loggerMock, 'test_method', ['set_1', 'set*3'], flagSetsFilter)).toEqual(['set_1']);
161-
expect(loggerMock.warn.mock.calls[5]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*3', regexp, 'set*3']]);
161+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set_1', 'set*3'], flagSetsFilter)).toEqual(['set_1']);
162+
expect(loggerMock.warn.mock.calls[5]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, 'set*3', regexp, 'set*3']]);
162163

163164
// set_3 not included in configuration but set_1 included => [set_1] & warn
164-
expect(validateFlagSets(loggerMock, 'test_method', ['set_1', 'set_3'], flagSetsFilter)).toEqual(['set_1']);
165-
expect(loggerMock.warn.mock.calls[6]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, ['test_method', 'set_3']]);
165+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set_1', 'set_3'], flagSetsFilter)).toEqual(['set_1']);
166+
expect(loggerMock.warn.mock.calls[6]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, [METHOD_NAME, 'set_3']]);
166167

167168
// set_3 not included in configuration => [] & warn
168-
expect(validateFlagSets(loggerMock, 'test_method', ['set_3'], flagSetsFilter)).toEqual([]);
169-
expect(loggerMock.warn.mock.calls[7]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, ['test_method', 'set_3']]);
169+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set_3'], flagSetsFilter)).toEqual([]);
170+
expect(loggerMock.warn.mock.calls[7]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, [METHOD_NAME, 'set_3']]);
170171

171172
// empty config
172173

173174

174175
// must start with a letter or number
175-
expect(validateFlagSets(loggerMock, 'test_method', ['_set_1'], [])).toEqual([]);
176-
expect(loggerMock.warn.mock.calls[8]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['_set_1', regexp, '_set_1']]);
176+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['_set_1'], [])).toEqual([]);
177+
expect(loggerMock.warn.mock.calls[8]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, '_set_1', regexp, '_set_1']]);
177178

178179
// can contain _a-z0-9
179-
expect(validateFlagSets(loggerMock, 'test_method', ['set*1'], [])).toEqual([]);
180-
expect(loggerMock.warn.mock.calls[9]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*1', regexp, 'set*1']]);
180+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set*1'], [])).toEqual([]);
181+
expect(loggerMock.warn.mock.calls[9]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, 'set*1', regexp, 'set*1']]);
181182

182183
// have a max length of 50 characters
183-
expect(validateFlagSets(loggerMock, 'test_method', [longName], [])).toEqual([]);
184-
expect(loggerMock.warn.mock.calls[10]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, [longName, regexp, longName]]);
184+
expect(validateFlagSets(loggerMock, METHOD_NAME, [longName], [])).toEqual([]);
185+
expect(loggerMock.warn.mock.calls[10]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, longName, regexp, longName]]);
185186

186187
// both set names invalid -> empty list & warn
187-
expect(validateFlagSets(loggerMock, 'test_method', ['set*1', 'set*3'], [])).toEqual([]);
188-
expect(loggerMock.warn.mock.calls[11]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*1', regexp, 'set*1']]);
189-
expect(loggerMock.warn.mock.calls[12]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*3', regexp, 'set*3']]);
188+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set*1', 'set*3'], [])).toEqual([]);
189+
expect(loggerMock.warn.mock.calls[11]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, 'set*1', regexp, 'set*1']]);
190+
expect(loggerMock.warn.mock.calls[12]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, 'set*3', regexp, 'set*3']]);
190191

191192
// only set_1 is valid => [set_1] & warn
192-
expect(validateFlagSets(loggerMock, 'test_method', ['set_1', 'set*3'], [])).toEqual(['set_1']);
193-
expect(loggerMock.warn.mock.calls[13]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*3', regexp, 'set*3']]);
193+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set_1', 'set*3'], [])).toEqual(['set_1']);
194+
expect(loggerMock.warn.mock.calls[13]).toEqual([WARN_INVALID_FLAGSET, [METHOD_NAME, 'set*3', regexp, 'set*3']]);
194195

195196
// any set should be returned if there isn't flag sets in filter
196-
expect(validateFlagSets(loggerMock, 'test_method', ['set_1'], [])).toEqual(['set_1']);
197-
expect(validateFlagSets(loggerMock, 'test_method', ['set_1', 'set_2'], [])).toEqual(['set_1', 'set_2']);
198-
expect(validateFlagSets(loggerMock, 'test_method', ['set_3'], [])).toEqual(['set_3']);
197+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set_1'], [])).toEqual(['set_1']);
198+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set_1', 'set_2'], [])).toEqual(['set_1', 'set_2']);
199+
expect(validateFlagSets(loggerMock, METHOD_NAME, ['set_3'], [])).toEqual(['set_3']);
199200

200201
});
201202

‎src/utils/settingsValidation/splitFilters.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { validateSplits } from '../inputValidation/splits';
33
import { ISplitFiltersValidation } from '../../dtos/types';
44
import { SplitIO } from '../../types';
55
import { ILogger } from '../../logger/types';
6-
import { WARN_SPLITS_FILTER_IGNORED, WARN_SPLITS_FILTER_EMPTY, WARN_SPLITS_FILTER_INVALID, SETTINGS_SPLITS_FILTER, LOG_PREFIX_SETTINGS, ERROR_SETS_FILTER_EXCLUSIVE, WARN_SPLITS_FILTER_LOWERCASE_SET, WARN_SPLITS_FILTER_INVALID_SET, WARN_FLAGSET_NOT_CONFIGURED } from '../../logger/constants';
6+
import { WARN_SPLITS_FILTER_IGNORED, WARN_SPLITS_FILTER_EMPTY, WARN_SPLITS_FILTER_INVALID, SETTINGS_SPLITS_FILTER, LOG_PREFIX_SETTINGS, ERROR_SETS_FILTER_EXCLUSIVE, WARN_LOWERCASE_FLAGSET, WARN_INVALID_FLAGSET, WARN_FLAGSET_NOT_CONFIGURED } from '../../logger/constants';
77
import { objectAssign } from '../lang/objectAssign';
88
import { find, uniq } from '../lang';
99

@@ -54,7 +54,7 @@ function validateSplitFilter(log: ILogger, type: SplitIO.SplitFilterType, values
5454
if (result) {
5555

5656
if (type === 'bySet') {
57-
result = sanitizeFlagSets(log, result);
57+
result = sanitizeFlagSets(log, result, LOG_PREFIX_SETTINGS);
5858
}
5959

6060
// check max length
@@ -98,20 +98,21 @@ function queryStringBuilder(groupedFilters: Record<SplitIO.SplitFilterType, stri
9898
*
9999
* @param {ILogger} log
100100
* @param {string[]} flagSets
101+
* @param {string} method
101102
* @returns sanitized list of set names
102103
*/
103-
function sanitizeFlagSets(log: ILogger, flagSets: string[]) {
104+
function sanitizeFlagSets(log: ILogger, flagSets: string[], method: string) {
104105
let sanitizedSets = flagSets
105106
.map(flagSet => {
106107
if (CAPITAL_LETTERS_REGEX.test(flagSet)) {
107-
log.warn(WARN_SPLITS_FILTER_LOWERCASE_SET, [flagSet]);
108+
log.warn(WARN_LOWERCASE_FLAGSET, [method, flagSet]);
108109
flagSet = flagSet.toLowerCase();
109110
}
110111
return flagSet;
111112
})
112113
.filter(flagSet => {
113114
if (!VALID_FLAGSET_REGEX.test(flagSet)) {
114-
log.warn(WARN_SPLITS_FILTER_INVALID_SET, [flagSet, VALID_FLAGSET_REGEX, flagSet]);
115+
log.warn(WARN_INVALID_FLAGSET, [method, flagSet, VALID_FLAGSET_REGEX, flagSet]);
115116
return false;
116117
}
117118
if (typeof flagSet !== 'string') return false;
@@ -190,7 +191,7 @@ export function validateSplitFilters(log: ILogger, maybeSplitFilters: any, mode:
190191

191192
export function validateFlagSets(log: ILogger, method: string, flagSets: string[], flagSetsInConfig: string[]): string[] {
192193
const sets = validateSplits(log, flagSets, method, 'flag sets', 'flag set');
193-
let toReturn = sets ? sanitizeFlagSets(log, sets) : [];
194+
let toReturn = sets ? sanitizeFlagSets(log, sets, method) : [];
194195
if (flagSetsInConfig.length > 0) {
195196
toReturn = toReturn.filter(flagSet => {
196197
if (flagSetsInConfig.indexOf(flagSet) > -1) {

0 commit comments

Comments
 (0)