chore: added classes for ce and ee spans #34484
Conversation
WalkthroughThe refactoring tasks involve reorganizing span-related classes in the Appsmith codebase, following a cleaner structure where common spans are defined in new Changes
Sequence Diagram(s)No sequence diagrams necessary as the changes are structural and do not modify control flows. Assessment against linked issues
Poem
Warning Review ran into problemsProblems (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
github-actions
bot
added
Integrations Product
| @@ -1,23 +1,9 @@ | |||
| package com.appsmith.external.constants.spans; | |||
|
|
|||
| import static com.appsmith.external.constants.spans.BaseSpan.APPSMITH_SPAN_PREFIX; | |||
| import com.appsmith.external.constants.spans.ce.ActionSpanCE; | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
This could also be copied to CE classes, please make sure it's consistent across.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ActionSpan.java (1 hunks)
- app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/DatasourceSpan.java (1 hunks)
- app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/TenantSpan.java (1 hunks)
- app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ActionSpanCE.java (1 hunks)
- app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/DatasourceSpanCE.java (1 hunks)
- app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/TenantSpanCE.java (1 hunks)
Files skipped from review due to trivial changes (3)
- app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ActionSpan.java
- app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/ActionSpanCE.java
- app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/TenantSpanCE.java
Additional comments not posted (3)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/TenantSpan.java (1)
3-3: Inheritance fromTenantSpanCEis correctly implemented.The change aligns with the PR's objective to modularize span constants. Ensure that this change is tested thoroughly to prevent any runtime issues due to the new inheritance structure.
#!/bin/bash # Description: Verify if the change in inheritance affects other parts of the project. # Test: Search for references to TenantSpan constants across the project. rg --type java "TenantSpan."Also applies to: 5-5
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/DatasourceSpan.java (1)
3-3: Inheritance fromDatasourceSpanCEis correctly implemented.This change is consistent with the PR's goals to better organize span constants. It is crucial to verify that all functionalities relying on the constants previously declared in
DatasourceSpancontinue to operate correctly.#!/bin/bash # Description: Verify if the change in inheritance affects other parts of the project. # Test: Search for references to DatasourceSpan constants across the project. rg --type java "DatasourceSpan."Also applies to: 5-5
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/DatasourceSpanCE.java (1)
6-8: Constant declarations are well-defined.The use of
APPSMITH_SPAN_PREFIXensures consistency in naming spans. Verify that the actual values of these constants match the intended spans in the application.#!/bin/bash # Description: Verify the correctness of span constant values. # Test: Check if the span constants are used appropriately within the application. rg --type java "get_all_datasource_storage|get_all_plugins_in_workspace"
## Description The spans directory currently did not have a way to add EE-specific spans. <img width="340" alt="SCR-20240626-j4l" src="https://github.com/appsmithorg/appsmith/assets/25542733/610dd7d4-2309-4fed-8dce-7faf1c02b6a7"> **This PR fixes the span directory to add ce and ee spans support.** Fixes appsmithorg#34483 ## Automation /ok-to-test tags="@tag.Sanity" ### 🔍 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/9676199406> > Commit: 45830b0 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9676199406&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` <!-- 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 - **Refactor** - Improved code structure by extending specialized classes (`ActionSpanCE`, `DatasourceSpanCE`, `TenantSpanCE`) for `ActionSpan`, `DatasourceSpan`, and `TenantSpan`. - **New Features** - Introduced new constants for tracing various action executions and data fetching related to datasources and tenants. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
The spans directory currently did not have a way to add EE-specific spans.
This PR fixes the span directory to add ce and ee spans support.
Fixes #34483
Automation
/ok-to-test tags="@tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9676199406
Commit: 45830b0
Cypress dashboard.
Tags:
@tag.SanityCommunication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Refactor
ActionSpanCE,DatasourceSpanCE,TenantSpanCE) forActionSpan,DatasourceSpan, andTenantSpan.New Features