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

Add data summary panel in discover #8186

Merged
merged 8 commits into from
Sep 20, 2024

Conversation

chishui
Copy link
Contributor

@chishui chishui commented Sep 13, 2024

Description

UI:

  1. add a button on searchbar to display or hide query assist
  2. add a summary panel to display sample data summary
    They are part of query assist feature, so only when query assist is available, they could display.

Changes

Data plugin
  1. add a new observable variant in search service, to provide search data result from discover, others can get notified for search result change.
  2. add a new interface getSearchBarButton in QueryEditorExtensionConfig so that different query editor can display their customized button on search bar.
query_enhancements plugin
  1. provide summary panel and query assist button UI.
  2. add configPath "queryEnhancements" in opensearch_dashboards.json so that the feature can be control from opensearch_dashboards.yml
  3. add usage_collection plugin dependency to emit metrics.

Issues Resolved

#8177

Screenshot

Screenshot 2024-09-13 at 10 02 31

Testing the changes

Step 1. enable this summary feature in opensearch_dashboards.yml
queryEnhancements.queryAssist.summary.enabled: true
Step 2. Enable query enhancement from dashboard

OSD left sidebar -> management -> Dashboards Managements -> Advanced settings -> Enable query enhancements -> On

Step 3. configure OpenSearch ML agents
query assist agent

Follow #2 form this doc to configure query assist agent to enable query assist feature

summary agent
POST .plugins-ml-config/_doc/os_data2summary
{
    "type":"os_summary_root_agent",
    "configuration":{
        "agent_id": "{{ _.summary_agent_id }}"
    }
}
Step 4. Navigate to Discover page
  1. Change language to PPL
  2. Click expand button to show natural language input and summary panel
  3. Type a command to query your data then click submit button

Changelog

Add a data summary panel on discover page under query assist function.

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

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
Copy link
Contributor

❌ Changelog Entry Missing Hyphen

Changelog entries must begin with a hyphen (-).

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
@kavilla kavilla added discover for discover reinvent v2.18.0 labels Sep 13, 2024
@kavilla kavilla linked an issue Sep 13, 2024 that may be closed by this pull request
@kavilla
Copy link
Member

kavilla commented Sep 13, 2024

pretty awesome! i have comments and questions but i like how it's looking.

@kavilla
Copy link
Member

kavilla commented Sep 13, 2024

can you make sure to include a changelog? you might need to give the changelog bot permissions though. just will need to follow the directions given in the link

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
@ashwin-pc ashwin-pc added 2.17.1 and removed v2.18.0 labels Sep 17, 2024
@chishui
Copy link
Contributor Author

chishui commented Sep 18, 2024

@joshuali925 @kavilla I've addressed your comments, could you help review again?

joshuali925
joshuali925 previously approved these changes Sep 18, 2024
Copy link
Member

@joshuali925 joshuali925 left a comment

Choose a reason for hiding this comment

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

lgtm

src/plugins/data/public/ui/query_editor/query_editor.tsx Outdated Show resolved Hide resolved
const RESPONSE_TEXT = 'response';
httpMock.post.mockResolvedValue(RESPONSE_TEXT);
renderQueryAssistSummary(COLLAPSED.NO);
await sleep(2000);
Copy link
Member

Choose a reason for hiding this comment

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

curious does it use jest fake timers or is this actually sleeping 2 seconds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's actual sleep, I'll change it to 100ms.

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
setSummary(response);
reportCountMetric(SUCCESS_METRIC, 1);
} catch (error) {
reportCountMetric(SUCCESS_METRIC, 0);
Copy link
Member

Choose a reason for hiding this comment

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

What about having another event FAILED_METRIC? It doesn't seem METRIC_TYPE.COUNT is intend to be used in this way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this way we can get success rate easily only using one metric

<QueryAssistContext.Provider
value={{
question,
question$,
Copy link
Member

Choose a reason for hiding this comment

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

question$ is possibly undefined, should you tweaks the type QueryAssistContextValue to question$?: BehaviorSubject<string>;?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, question$ is always created, it could be undefined because for QueryAssistBanner, it doesn't use this variable, so I think we don't need to pass question$ to it thus it's undefined

src/plugins/data/public/ui/query_editor/query_editor.tsx Outdated Show resolved Hide resolved
@ruanyl ruanyl removed the 2.17.1 label Sep 20, 2024
@ruanyl
Copy link
Member

ruanyl commented Sep 20, 2024

@ashwin-pc FYI, saw you add 2.17.1 tag, since this is new feature which is not supposed to be included in 2.17.1, so removed that tag from it, let me know if you have any concern, thanks:)

@ruanyl ruanyl merged commit 8d52134 into opensearch-project:main Sep 20, 2024
66 of 67 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 20, 2024
* Add data summary panel in discover

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Fix UTs

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Add changelog

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Fix UTs

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Address comments

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Address comments

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Address comments

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

---------

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
(cherry picked from commit 8d52134)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
kavilla pushed a commit that referenced this pull request Sep 23, 2024
* Add data summary panel in discover



* Fix UTs



* Add changelog



* Fix UTs



* Address comments



* Address comments



* Address comments



---------


(cherry picked from commit 8d52134)

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
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>
virajsanghvi pushed a commit to virajsanghvi/OpenSearch-Dashboards that referenced this pull request Sep 24, 2024
* Add data summary panel in discover

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Fix UTs

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Add changelog

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Fix UTs

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Address comments

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Address comments

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Address comments

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

---------

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
@chishui chishui deleted the discover-summary branch September 25, 2024 08:05
SuZhou-Joe pushed a commit to SuZhou-Joe/OpenSearch-Dashboards that referenced this pull request Oct 3, 2024
…rch-project#8264)

* Add data summary panel in discover



* Fix UTs



* Add changelog



* Fix UTs



* Address comments



* Address comments



* Address comments



---------


(cherry picked from commit 8d52134)

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
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>
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.

[FEAT] Display LLM generated summary on sample data from discover
7 participants