feat(issues): add feature flags to issue stream search suggestions #83968
Conversation
github-actions
bot
added
the
Scope: Frontend
❌ 1 Tests Failed:
View the top 1 failed tests by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
|
@aliu39 Gate with this feature flag: https://github.com/getsentry/sentry-options-automator/pull/3051/files |
| (allTagsCollection[tag.key]!.totalValues ?? 0) + (tag.totalValues ?? 0); | ||
| } else { | ||
| // When a feature flag collides with a custom tag, we only suggest the tag. | ||
| // Searching for such a flag will filter by both tag and flag. |
There was a problem hiding this comment.
For tags that conflict with other search keys, we suggest tags[tag_name] instead. If the search backend supports that syntax, I'd do the same here (flags[flag_name]?)
There was a problem hiding this comment.
I've mentioned this to @cmanallen and @jas-kas and think we prefer to not use flags[. CC'ing here so we can align. The tags[ syntax seems (mostly?) deprecated - we don't use it in replays/feedback at least. On the backend, we check both columns (tags and flags) in an OR condition, so the actual filtering/search functionality is never broken.
The trade-off for not using flags[ syntax is we'd sacrifice flag suggestions when there's a conflict. The key would show up in the tag section, but not flags. It also wouldn't have the description
and would only suggest the tag values instead of true/false.
The reason there's this trade-off is because of the data structure for filterKeys (mapping[key -> metadata]). The way the search bar is currently built, if the key/name is the same, we can't case it into 2 different suggestions.
Is this trade-off ok for now?
There was a problem hiding this comment.
The reason there's this trade-off is because of the data structure for filterKeys (mapping[key -> metadata]). The way the search bar is currently built, if the key/name is the same, we can't case it into 2 different suggestions.
Even if we allowed more advanced mapping for filterKeys, we need to have that information in the underlying search query string. Without it, we can't know if it's a flag or a tag when you refresh the page. e.g. you load the URL /issues?query=flag_and_tag:value and you click into the filter, which suggestions do you show? And what if the user only wants to target the flag and not the tag?
I'm okay with punting this for now and dealing with it later, but I would suggest more thought into how we can differentiate tags and flags in the search syntax. Especially since once we run into conflicts it will be very difficult to modify the syntax in the future
There was a problem hiding this comment.
e.g. you load the URL /issues?query=flag_and_tag:value and you click into the filter, which suggestions do you show?
I think this PR deals with this by only suggesting values from the tag. Since conflicting flags are simply dropped from filterKeys. Not ideal, as it prevents the flag from being discovered, and confuses users who're looking for the flag.
There was a problem hiding this comment.
I'm sure you've probably already ruled this out, but this would fit into our current search syntax a lot better if it looked like:
feature_flag.on:["organizations:flag_1", "organizations_flag_2"]
feature_flag.off:["organizations:flag_3"]
There was a problem hiding this comment.
Oh we actually haven't considered that! Maybe the backend parser can use this as an alias for FF queries? cc @cmanallen
There was a problem hiding this comment.
@malwilley Feature flags are not just on/off values. They have the same properties as tags and we want to query them as though they were tags. They are only disambiguated on the back-end to prevent us from interfering with existing tag systems in ways that would be annoying to customers.
Bundle ReportChanges will increase total bundle size by 11.47kB (0.03%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: app-webpack-bundle-array-pushAssets Changed:
Files in
Files in
Files in
|
| // Strip quotes for feature flags, which may be used to escape special characters in the search bar. | ||
| const charsToStrip = '"'; | ||
| const key = | ||
| tag.kind === FieldKind.FEATURE_FLAG |
There was a problem hiding this comment.
This is questionable to me because tags could have colons too but its fine we're not breaking something. We could always extend this later.
There was a problem hiding this comment.
Sounds good, @malwilley specifically requested this just to be safe, so the changes are FF only
There was a problem hiding this comment.
I asked for this to be added because there's a risk that this new stripping logic might interfere with existing tags. Agree that the final solution should be generic
There was a problem hiding this comment.
Good to merge!
If you want to pair on #83968 (comment) just lmk
Closes https://github.com/getsentry/team-replay/issues/532
Adds FF suggestions as their own section in issues stream search bar.
To do this, we make additional requests to
/tagsand/tags/<key>/valueswith the?useFlagsBackendswitch. For details on the backend, see changes in #83639.Notes: