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

[Fleet] Disable TSBD setting switch when enabled #154048

Merged
merged 3 commits into from
Apr 12, 2023

Conversation

jillguyonnet
Copy link
Contributor

@jillguyonnet jillguyonnet commented Mar 30, 2023

Summary

Disable Time-series indexing (TSDB) switch in experimental datastream settings if TSDB is enabled in the package itself.

As part of this fix, the following changes are included in compliance with UX rules:

  • Rename toggle label from Time-series indexing (TSDB) to Time-series database (TSDB) indexing.
  • When the toggle is disabled, add a question mark icon with a tooltip with text TSDB indexing is enabled by the integration.

Closes #152778

Screenshots

Before

Screenshot 2023-04-11 at 17 54 45

After

When the integration does NOT enable TSDB:
Screenshot 2023-04-11 at 17 26 04

When the integration DOES enable TSDB:
Screenshot 2023-04-11 at 17 28 12

Steps to test locally

Refer to the original PR for extended context.

Testing a package without TSDB

  1. Run Kibana on this branch.
  2. Add the System integration (version 1.5.2 at the time of writing); under the Collect metrics from System instances -> System cpu metrics -> Advanced options settings, the Time-series indexing (TSDB) switch should be off and enabled (you should be able to toggle it on and off).

Testing a package with TSDB

  1. Update the system integration to use TSDB and run a local package registry:
    1. In the integrations repo, bump the patch version of the system integration: add a changelog entry (1.25.3) and update the version accordingly in the package manifest.
    2. In the manifest of the cpu data stream, add the following to enable TSDB (see this doc for context):
      elasticsearch:
        index_mode: "time_series"
      
    3. Build the system integration: cd packages/system && elastic-package build
    4. Run a local package registry: elastic-package stack up -d -v --services package-registry
    5. Check at https://localhost:8080/search?package=system that the version is correct (1.5.3).
  2. Run Kibana on this branch to test the change:
    1. In your kibana.dev.yml, add xpack.fleet.registryUrl: https://localhost:8080 if not there already.
    2. Also in kibana.dev.yml, add xpack.fleet.enableExperimental: ['experimentalDataStreamSettings'] if not there already.
    3. Before running yarn start, run export NODE_EXTRA_CA_CERTS=$HOME/.elastic-package/profiles/default/certs/kibana/ca-cert.pem in the same shell. This is to set up the certificate needed to access EPR with https.
    4. In Kibana, add the System integration (check that the version is correct); under the Collect metrics from System instances -> System cpu metrics -> Advanced options settings, the Time-series indexing (TSDB) switch should be on and disabled. There should be a question mark icon with a tooltip.

@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@jillguyonnet jillguyonnet added Team:Fleet Team label for Observability Data Collection Fleet team release_note:skip Skip the PR/issue when compiling release notes v8.8.0 labels Apr 5, 2023
@jillguyonnet jillguyonnet marked this pull request as ready for review April 5, 2023 15:03
@jillguyonnet jillguyonnet requested a review from a team as a code owner April 5, 2023 15:03
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@jillguyonnet
Copy link
Contributor Author

@elasticmachine merge upstream

@hop-dev
Copy link
Contributor

hop-dev commented Apr 5, 2023

Should we add a tooltip or similar to let the user know why the checkbox is disabled? I'm not 100% sure on this, just interested in other peoples thoughts

@jlind23
Copy link
Contributor

jlind23 commented Apr 6, 2023

Should we add a tooltip or similar to let the user know why the checkbox is disabled? I'm not 100% sure on this, just interested in other peoples thoughts

👍🏼 Let's add something quick and easy.

@jillguyonnet
Copy link
Contributor Author

@jlind23 Who should I consult with to validate the tooltip message?

@jlind23
Copy link
Contributor

jlind23 commented Apr 6, 2023

@karenzone might be our best asset here.

@jillguyonnet jillguyonnet self-assigned this Apr 6, 2023
@karenzone
Copy link
Contributor

A comment on the option itself... The description and the abbreviation should be aligned.
So instead of Time-series indexing (TSDB), I would expect to see something like Time-series database (TSDB) indexing.

Similarly for the tooltip:
"TSDB indexing is enabled by the integration"

@jillguyonnet
Copy link
Contributor Author

jillguyonnet commented Apr 11, 2023

@karenzone I have made the changes and uploaded updated screenshots to the PR description based on your feedback.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
fleet 942.2KB 942.9KB +736.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 433 436 +3

Total ESLint disabled count

id before after diff
securitySolution 513 516 +3

History

  • 💚 Build #118030 succeeded 5ddecbed573f32aeadc367dae6a8fd339b44606f
  • 💚 Build #116795 succeeded e48197e9b718f14832a3d9ed5ea12478a71ce6af
  • 💚 Build #116750 succeeded c857d26a5779e52c80a3483579796c419451b6d3

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

cc @jillguyonnet

@jillguyonnet jillguyonnet merged commit 7d946ff into elastic:main Apr 12, 2023
@jillguyonnet jillguyonnet deleted the fleet/tsdb-switch-fix branch April 12, 2023 15:09
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Apr 12, 2023
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 release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fleet]: tsdb settings are not greyed out for system-metrics-cpu when the button is not editable by user.
9 participants