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

Enable UI Metric Collector #6203

Merged
merged 1 commit into from
Apr 30, 2024
Merged

Enable UI Metric Collector #6203

merged 1 commit into from
Apr 30, 2024

Conversation

LDrago27
Copy link
Collaborator

@LDrago27 LDrago27 commented Mar 19, 2024

Description

This changes is to enable UI Metric Collectors which would save the UI metrics and application usage information across the dashboard into the saved object.

Feature Flag: usageCollection.uiMetric.enabled
Default Value: false

Testing the changes

  • Enable the feature flag for this functionality by setting the usageCollection.uiMetric.enabled to true in the config/opensearch_dashboards.yml file
  • Upon interactions with the UI elements across dashboards (eg: Home , Console, Settings) you can see calls being made to /api/ui_metric/report endpoint in the network tab.
  • The API /api/stats?extended=true&legacy=true&exclude_usage=false now has ui_metric and application_usage fields populated with data.

Check List

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

Copy link

codecov bot commented Mar 19, 2024

Codecov Report

Attention: Patch coverage is 9.09091% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 67.68%. Comparing base (7aa7a74) to head (4ab1f80).

❗ Current head 4ab1f80 differs from pull request most recent head f2fef17. Consider uploading reports for the commit f2fef17 to get more accurate results

Files Patch % Lines
packages/osd-analytics/src/reporter.ts 0.00% 5 Missing ⚠️
...sage_collection/public/services/create_reporter.ts 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6203      +/-   ##
==========================================
- Coverage   67.69%   67.68%   -0.01%     
==========================================
  Files        3415     3415              
  Lines       66879    66858      -21     
  Branches    10882    10876       -6     
==========================================
- Hits        45273    45256      -17     
- Misses      18959    19000      +41     
+ Partials     2647     2602      -45     
Flag Coverage Δ
Linux_1 33.20% <9.09%> (+<0.01%) ⬆️
Linux_2 ?
Linux_3 45.21% <0.00%> (-0.03%) ⬇️
Linux_4 34.85% <0.00%> (-0.01%) ⬇️
Windows_1 33.22% <9.09%> (?)
Windows_2 55.56% <ø> (ø)
Windows_3 45.23% <0.00%> (-0.01%) ⬇️
Windows_4 34.85% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ashwin-pc
ashwin-pc previously approved these changes Mar 19, 2024
Copy link
Member

@ashwin-pc ashwin-pc left a comment

Choose a reason for hiding this comment

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

LGTM!

@seraphjiang
Copy link
Member

Do we have feature flag to disable ?

There used to be search telemetry feature which collect search query made from OSD and saved into saved object that cause significantly performance issue.

Cc: @Flyingliuhub @CCongWang @zengyan-amazon @wbeckler

@LDrago27
Copy link
Collaborator Author

LDrago27 commented Apr 9, 2024

Thanks @ananzh. I have fixed the failing test cases. Adding the screenshots for reference.
image

@LDrago27
Copy link
Collaborator Author

LDrago27 commented Apr 9, 2024

@LDrago27 A question not related to this PR:

Do we know what is the difference between usage_collection and opensearch_dashboards_usage_collection? I didn't find you mention opensearch_dashboards_usage_collection in ur previous doc PR, and I am curious why modify usage_collection only is enough.

Seems opensearch_dashboards_usage_collection plugin just extends the usage_collection service, specifically tailored for collecting usage data within the OSD. What is special here? Should we update ur previous doc PR for developers?

@ananzh usage_collection is the plugin that is responsible collecting and saving metrics. It contains a default collector called UI Metric and also defines the framework that can be used by other plugins to create custom versions of usage collector. The opensearch_dashboards_usage_collection is responsible for registering different usage collectors. Once registered these usage collectors can be used across the plugins.
I will raise a separate PR to update the documentation of telemetry with details regarding opensearch_dashboards_usage_collection.

ananzh
ananzh previously approved these changes Apr 10, 2024
Copy link
Member

@ananzh ananzh left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -313,3 +313,7 @@

# Set the value to true to enable workspace feature
# workspace.enabled: false

# Set the value to true to enable Ui Metric Collectors in Usage Collector
Copy link
Member

Choose a reason for hiding this comment

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

Let's call our this might cause permission issue to OpenSearch. turn on with caution

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated the comments

@seraphjiang
Copy link
Member

@LDrago27

What are the telemetry data has been stored into saved object, i didn't see any functional test and integration test. could we add a few ? if not planned for automation, could we add manual test result?

@LDrago27
Copy link
Collaborator Author

@seraphjiang Thanks for your suggestion. I have created this PR opensearch-project/opensearch-dashboards-functional-test#1223 to include a functional tests for the above change.

ananzh
ananzh previously approved these changes Apr 25, 2024
ashwin-pc
ashwin-pc previously approved these changes Apr 30, 2024
Signed-off-by: Suchit Sahoo <[email protected]>
@ananzh ananzh merged commit 5258d83 into opensearch-project:main Apr 30, 2024
53 of 54 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 30, 2024
Signed-off-by: Suchit Sahoo <[email protected]>
(cherry picked from commit 5258d83)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
ananzh pushed a commit that referenced this pull request Apr 30, 2024
Signed-off-by: Suchit Sahoo <[email protected]>
(cherry picked from commit 5258d83)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 30, 2024
Signed-off-by: Suchit Sahoo <[email protected]>
(cherry picked from commit 5258d83)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
(cherry picked from commit aac0ecb)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
ashwin-pc pushed a commit that referenced this pull request May 1, 2024
(cherry picked from commit 5258d83)


(cherry picked from commit aac0ecb)

Signed-off-by: Suchit Sahoo <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Anan Zhuang <[email protected]>
@seraphjiang
Copy link
Member

@seraphjiang Thanks for your suggestion. I have created this PR opensearch-project/opensearch-dashboards-functional-test#1223 to include a functional tests for the above change.

Thanks @LDrago27 for adding tests.

looks like the test PR is still open. are we also plan to get test merge in 2.14?

LDrago27 added a commit to LDrago27/OpenSearch-Dashboards that referenced this pull request Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants