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

Fixes search usage telemetry #1427

Merged
merged 5 commits into from
Apr 7, 2022
Merged

Fixes search usage telemetry #1427

merged 5 commits into from
Apr 7, 2022

Conversation

Flyingliuhub
Copy link
Member

@Flyingliuhub Flyingliuhub commented Apr 5, 2022

Signed-off-by: Tao liu liutaoaz@amazon.com

Description

Search Telemetry is used to analyze performance for sucess/failed search request in Dashboards. It will save telemetry data into .kibana_1 index, there are thousands of concurrent search request from Dashboards. It causes significant load in OpenSearch cluster. We have been seen some case 1/3 Search/Update request of total are come from this. This PR will suppress the search usage telemetry based on the switch defined on opensearch_dashboard.yml or data plugin config.

Issues Resolved

resolves #928

Check List

  • New functionality includes testing.
    • All tests pass
      • yarn test:jest
      • yarn test:jest_integration
      • yarn test:ftr
  • New functionality has been documented.
  • Commits are signed per the DCO using --signoff
yarn test:jest

Test Suites: 1 skipped, 1432 passed, 1432 of 1433 total
Tests:       24 skipped, 9 todo, 10721 passed, 10754 total
Snapshots:   2452 passed, 2452 total
Time:        241.591 s
Ran all test suites.
Done in 243.82s.
yarn test:jest_integration

Test Suites: 50 passed, 50 total
Tests:       5 skipped, 438 passed, 443 total
Snapshots:   77 passed, 77 total
Time:        294.497 s
Ran all test suites.
         │ info [opensearch] cleanup complete
Done in 296.78s.
opensearch_dashboard.yml enabled value data plugin config enabled value Results
TRUE FALSE Passed
FALSE FALSE Passed
TRUE TRUE Passed
FALSE TRUE Passed
NONE TRUE Passed
NONE FALSE Passed

Signed-off-by: Tao liu <liutaoaz@amazon.com>
@Flyingliuhub Flyingliuhub requested a review from a team as a code owner April 5, 2022 18:57
@Flyingliuhub Flyingliuhub changed the title fix search usage telemetry Fixes search usage telemetry Apr 5, 2022
@seraphjiang
Copy link
Member

I like the this table, explain how to use this very well. what's our recommendation to make it a user manual or doc
@kavilla @seanneumann

opensearch_dashboard.yml enabled value data plugin config enabled value Results
TRUE FALSE Passed
FALSE FALSE Passed
TRUE TRUE Passed
FALSE TRUE Passed
NONE TRUE Passed
NONE FALSE Passed

@mihirsoni
Copy link
Contributor

@Flyingliuhub Thanks this is super nice, it would be great to add some UTs to ensure the config works in expected behavior.

@kavilla
Copy link
Member

kavilla commented Apr 5, 2022

@Flyingliuhub Thanks this is super nice, it would be great to add some UTs to ensure the config works in expected behavior.

+1

@Flyingliuhub Flyingliuhub reopened this Apr 6, 2022
Signed-off-by: Tao liu <liutaoaz@amazon.com>
@Flyingliuhub
Copy link
Member Author

Thanks, I added some unit tests.

Signed-off-by: Tao liu <liutaoaz@amazon.com>
Signed-off-by: Tao liu <liutaoaz@amazon.com>
seraphjiang
seraphjiang previously approved these changes Apr 6, 2022
Copy link
Member

@seraphjiang seraphjiang left a comment

Choose a reason for hiding this comment

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

LGTM

tianleh
tianleh previously approved these changes Apr 7, 2022
mihirsoni
mihirsoni previously approved these changes Apr 7, 2022
Copy link
Contributor

@mihirsoni mihirsoni left a comment

Choose a reason for hiding this comment

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

LGTM !! Thanks for UT.

@Flyingliuhub
Copy link
Member Author

Thanks Mihir.

@kavilla
Copy link
Member

kavilla commented Apr 7, 2022

Is this still valid: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/data/server/server.api.md?plain=1#L1104

I know we don't currently use these docs but I think @ashwin-pc just made a proposal here: opensearch-project/documentation-website#492 to start including this docs which I agree with. So might be worth to update this doc here.

@kavilla
Copy link
Member

kavilla commented Apr 7, 2022

I like the this table, explain how to use this very well. what's our recommendation to make it a user manual or doc @kavilla @seanneumann
opensearch_dashboard.yml enabled value data plugin config enabled value Results
TRUE FALSE Passed
FALSE FALSE Passed
TRUE TRUE Passed
FALSE TRUE Passed
NONE TRUE Passed
NONE FALSE Passed

You mean like how to let the public know? Yeah right now the current strategy has been create an issue with the technical details in this repo: https://github.com/opensearch-project/documentation-website/issues/new?assignees=&labels=enhancement%2C+untriaged&template=FEATURE_REQUEST_TEMPLATE.md&title=%5BFEATURE%5D

kavilla
kavilla previously approved these changes Apr 7, 2022
Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

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

I have 1 nit about the default value and 1 comment I think you should address about the potentially undefined case but I'm still ecstatic that this is in here!

So don't want to block this. I think if you just make a commit on top of this should not wipe out my approval. Just don't rebase, that will definitely clear everyone's approval.

Signed-off-by: Tao liu <liutaoaz@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhance search telemetry to avoid high load to OpenSearch cluster
7 participants