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

ESLint Telemetry Rule #151173

Closed
wants to merge 58 commits into from
Closed

Conversation

CoenWarmer
Copy link
Contributor

@CoenWarmer CoenWarmer commented Feb 14, 2023

Resolves #144887

Summary

This PR adds an ESLint Plugin which checks Eui elements for the existence of a data-test-subj prop, and tries to come up with a suggestion based on the context in which the element is used.

Screen.Recording.2023-02-14.at.15.26.35.mov

Details

Why do this?
There is an increased push to move towards data driven feature development. In order to facilitate this, we need to have an increased focus on instrumenting user event generating elements in the Kibana codebase. This linting rule is an attempt to nudge Kibana engineers to not forget to add this property when writing frontend code. It also saves a bit of work for engineers by suggesting a value for the data-test-subj based on the location of the file in the codebase and any potential default values that might be present in the JSX node tree. Finally, because the suggestion is always of the same form, it can increase the consistency in the values given to these elements.

Shape of the suggestion
The suggestion for the value of data-test-subj is of the form: [app][componentName][intent][euiElementName].

For example, when working in a component in the location: x-pack/plugins/observability/public/pages/overview/containers/overview_page/header_actions.tsx, and having the code:

function HeaderActions() {
  return (
    <EuiButton>{i18n.translate('id', { defaultMessage: 'Submit Form' })}</EuiButton>
  )
}

the suggestion becomes: data-test-subj=o11yHeaderActionsSubmitFormButton.

Which elements are checked by the ESLint rule?
In its current iteration the rule checks these Eui elements:

  'EuiButton',
  'EuiButtonEmpty',
  'EuiLink',
  'EuiFieldText',
  'EuiFieldSearch',
  'EuiFieldNumber',
  'EuiSelect',
  'EuiRadioGroup',
  'EuiTextArea',
]

For elements that don't take a defaultMessage prop / translation, the suggestion takes the form: [app][componentName][euiElementName]

What types of function declarations does this rule support?

  • function Foo(){} (Named function)
  • const Foo = () => {} (Arrow function assigned to variable)
  • const Foo = memo(() => {}) (Arrow function assigned to variable wrapped in function)
  • const Foo = hoc(uponHoc(uponHoc(() => {}))) (Arrow function assigned to variable wrapped in infinite levels of functions)

Known limitations

  • the rule is set to warn, not to error. There can be legitimate reasons why an engineer does not want to add a data-test-subj and we don't want to block the CI in any way.
  • the suggestion is just that, a suggestion: the engineer can always adjust the value before committing.
  • If an element already has a value for data-test-subj the rule will not kick in as any existing instrumentation will depend on the value.

@CoenWarmer CoenWarmer requested review from a team as code owners February 14, 2023 16:38
@CoenWarmer CoenWarmer added the release_note:skip Skip the PR/issue when compiling release notes label Feb 14, 2023
@CoenWarmer CoenWarmer force-pushed the feature/telemetry-renewed branch from 6fa2a8d to 859cf68 Compare February 14, 2023 16:50
@CoenWarmer CoenWarmer force-pushed the feature/telemetry-renewed branch from 859cf68 to d1c9023 Compare February 14, 2023 16:50
@kibanamachine kibanamachine requested review from a team as code owners March 6, 2023 12:28
@kibanamachine kibanamachine requested a review from a team March 6, 2023 12:28
@kibanamachine kibanamachine requested review from a team as code owners March 6, 2023 12:28
@botelastic botelastic bot added Feature:Embedding Embedding content via iFrame Team:APM - DEPRECATED Use Team:obs-ux-infra_services. labels Mar 6, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:APM)

@CoenWarmer CoenWarmer closed this Mar 6, 2023
auto-merge was automatically disabled March 6, 2023 12:31

Pull request was closed

@kibana-ci
Copy link
Collaborator

kibana-ci commented Mar 6, 2023

⏳ Build in-progress, with failures

Failed CI Steps

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Embedding Embedding content via iFrame release_note:skip Skip the PR/issue when compiling release notes Team:APM - DEPRECATED Use Team:obs-ux-infra_services.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Actionable Observability] Create ESLint rule to help raise awareness about element instrumentation
8 participants