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.

Sign up for GitHub

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

Jump to bottom

[Task]: Move filtering of js object actions to DB #35815

Closed
sneha122 opened this issue Aug 21, 2024 · 3 comments · Fixed by #35826
Closed

[Task]: Move filtering of js object actions to DB #35815

sneha122 opened this issue Aug 21, 2024 · 3 comments · Fixed by #35826
Assignees
@sneha122
Labels
Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. Integrations Product Issues related to a specific integration Query & JS Pod Issues related to the query & JS Pod Task A simple Todo

Comments

@sneha122
Copy link
Contributor

SubTasks

One optimisation that can be done after observing the new relic metrics pushed in this PR, is that the part where we filter out JS object actions can be done on the DB layer itself. This task is to make this improvement and observe the metrics locally for heavy application.
Screenshot 2024-08-21 at 5 03 17 PM
@sneha122 sneha122 added the Task A simple Todo label Aug 21, 2024
@sneha122 sneha122 self-assigned this Aug 21, 2024
@sneha122 sneha122 added the Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. label Aug 21, 2024
@github-actions github-actions bot added Integrations Product Issues related to a specific integration Query & JS Pod Issues related to the query & JS Pod labels Aug 21, 2024
@sneha122
Copy link
Contributor Author

Spans before moving filtering to DB

@sneha122
Copy link
Contributor Author

Spans after moving filtering to DB

@sneha122
Copy link
Contributor Author

Spans after moving filtering to DB

@sneha122 sneha122 mentioned this issue Aug 21, 2024
Merged
2 tasks
sneha122 added a commit that referenced this issue Aug 23, 2024
@sneha122
fix: moved js filter to mongoDB (#35826)
9b40629 
## Description
This PR adds an improvement to the actions part of the consolidated view
API, where by it moves the filtering that we do to check if the action
of pluginType JSObject has been moved to MongoDB. This way the mongoDB
query that we do contains pluginTypes to include and we won't have to do
additional filtering at the code layer, there by saving some time that
is spend on further filtering and sanitising each action.

Here are some of the observations for before and after making this
change:

### Case 1:
Before (Total consolidated view api duration ~ 1.72s)
Actions span totally took ~ 1.12s
<img width="1171" alt="Screenshot 2024-08-22 at 9 26 15 PM"
src="https://github.com/user-attachments/assets/2dc69e4f-619a-4b86-9e9b-f1aede63d868">

After (Total consolidated view api duration ~ 1.72s)
Actions span totally took ~ 566ms
<img width="1180" alt="Screenshot 2024-08-22 at 9 33 16 PM"
src="https://github.com/user-attachments/assets/2947da7e-34ae-4933-9adf-7f7433c89c4e">


### Case 2:
Before (Total consolidated view api duration ~ 2.66s)
Actions span totally took ~ 1.67s
<img width="1170" alt="Screenshot 2024-08-22 at 9 34 05 PM"
src="https://github.com/user-attachments/assets/27b046d9-c066-4282-92c3-d1053b820d33">


After (Total consolidated view api duration ~ 2.88s)
Actions span totally took ~ 935ms
<img width="1180" alt="Screenshot 2024-08-22 at 9 34 28 PM"
src="https://github.com/user-attachments/assets/6bf3c47a-27a3-497d-b4ee-debdd5e6f25e">

Note: These numbers are obtained by testing the app with many actions
and js objects [Xolair App]

Fixes #`Issue Number`  
_or_  
Fixes [`Issue
URL`](#35815)
> [!WARNING]  
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

/ok-to-test tags="@tag.Datasource"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/10522148136>
> Commit: f0cf9b2
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10522148136&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Datasource`
> Spec:
> <hr>Fri, 23 Aug 2024 08:34:57 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit


- **New Features**
- Introduced enhanced action retrieval capabilities based on specific
plugin types, allowing for more tailored responses.
- Added new methods to filter actions by application ID and plugin type,
improving granularity in action queries.

- **Bug Fixes**
- Updated existing action retrieval methods to support additional
filtering options, improving the accuracy of returned results.

- **Chores**
- Streamlined query logic in the repository to enhance performance and
maintainability.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: “sneha122” <“sneha@appsmith.com”>
@sneha122 sneha122 mentioned this issue Sep 4, 2024
Merged
2 tasks
@coderabbitai coderabbitai bot mentioned this issue Nov 15, 2024
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sneha122 sneha122

Labels
Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. Integrations Product Issues related to a specific integration Query & JS Pod Issues related to the query & JS Pod Task A simple Todo
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: moved js filter to mongoDB appsmithorg/appsmith
1 participant
@sneha122