Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RAM] Feature flagging for triggers actions UI plugin #126957

Merged

Conversation

JiaweiWu
Copy link
Contributor

@JiaweiWu JiaweiWu commented Mar 4, 2022

Summary

For the upcoming feature to add logs in the rules details page (#126624), we would like to take a more incremental approach when it comes to committing code and PRs. Therefore, this PR borrows the feature flagging work done by @Zacqary, in this PR: #124428 so we can start using feature flags in triggers actions UI.

usage:

for react components:

import { getIsExperimentalFeatureEnabled } from 'triggers_actions_ui/public/common/get_experimental_features';
...
const isEnabled = getIsExperimentalFeatureEnabled('rulesListDatagrid');

for server side:

enable configs in plugin:

export class TriggersActionsPlugin implements Plugin<void, PluginStartContract> {
  private readonly logger: Logger;
  private readonly data: PluginStartContract['data'];
  private config: ConfigType; <---

  constructor(ctx: PluginInitializerContext) {
    this.logger = ctx.logger.get();
    this.data = getService();
    this.config = createConfig(ctx); <--
  }

and then it'll be accessible via:

const experimentalFeatures = this.config.experimentalFeatures;

Co-authored-by: Zacqary Adam Xeper zacqary.xeper@elastic.co

Co-authored-by: Zacqary Adam Xeper <zacqary.xeper@elastic.co>
@JiaweiWu JiaweiWu added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Alerting/RulesManagement Issues related to the Rules Management UX v8.2.0 labels Mar 4, 2022
@JiaweiWu JiaweiWu requested a review from a team as a code owner March 4, 2022 20:38
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@JiaweiWu JiaweiWu marked this pull request as draft March 4, 2022 20:40
@JiaweiWu JiaweiWu linked an issue Mar 4, 2022 that may be closed by this pull request
@JiaweiWu
Copy link
Contributor Author

@elasticmachine merge upstream

@JiaweiWu JiaweiWu marked this pull request as ready for review March 15, 2022 15:26
@JiaweiWu
Copy link
Contributor Author

@elasticmachine merge upstream


private static throwUninitializedError(): never {
throw new Error(
'Experimental features services not initialized - are you trying to import this module from outside of the Security Solution app?'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Solution? :)


export const useExperimentalFeatures = () => {
const [featuresCache, setFeaturesCache] = useState<ExperimentalFeatures>({
rulesListDatagrid: false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is where we set the list of experiment features in use?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would these be used in the UI code exactly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We set the experiments in the service file, or from kibana.dev.yml. This looks to be a default of sorts

@Zacqary just out of curiosity, Do you know the purpose of keeping these defaults as opposed to reading directly from the service? I'm not too sure what the use case would be.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, if these defaults aren't required, I'll make this a util function that reads from the service as opposed to a hook

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@XavierM
Copy link
Contributor

XavierM commented Mar 16, 2022

@elasticmachine merge upstream

Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
triggersActionsUi 331 333 +2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
triggersActionsUi 242 244 +2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
triggersActionsUi 700.1KB 700.1KB +2.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
triggersActionsUi 57.3KB 58.2KB +934.0B
Unknown metric groups

API count

id before after diff
triggersActionsUi 254 256 +2

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@JiaweiWu JiaweiWu merged commit 6084c97 into elastic:main Mar 16, 2022
maksimkovalev pushed a commit to maksimkovalev/kibana that referenced this pull request Mar 18, 2022
* Feature flagging for triggers actions UI

Co-authored-by: Zacqary Adam Xeper <zacqary.xeper@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 18, 2022
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 126957 or prevent reminders by adding the backport:skip label.

@JiaweiWu JiaweiWu added the auto-backport Deprecated - use backport:version if exact versions are needed label Mar 18, 2022
@kibanamachine
Copy link
Contributor

⚪ Backport skipped

The pull request was not backported as there were no branches to backport to. If this is a mistake, please apply the desired version labels or run the backport tool manually.

Manual backport

To create the backport manually run:

node scripts/backport --pr 126957

Questions ?

Please refer to the Backport tool documentation

@JiaweiWu JiaweiWu added backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. auto-backport Deprecated - use backport:version if exact versions are needed labels Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Alerting/RulesManagement Issues related to the Rules Management UX release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RAM] Add feature flagging capabilities
6 participants