Skip to content

Commit

Permalink
chore: Add JSObject creation flow metrics (#37064)
Browse files Browse the repository at this point in the history
## Description

These changes add monitoring spans to analyze the JS action creation
flow.

Fixes #37065

## Test the changes
| Case | Screenshot  |
|---|---|
| newRelic metrics for create flow |
![image](https://github.com/user-attachments/assets/314cc1d6-9def-49fc-afe9-4c77e84eaf72)
|


## 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/11500044480>
> Commit: ebf826f
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11500044480&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Sanity`
> Spec:
> <hr>Thu, 24 Oct 2024 13:46:02 UTC
<!-- end of auto-generated comment: Cypress test results  -->


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


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

- **New Features**
- Introduced new constants to enhance action validation and repository
operations.
- Added functionality for improved observability in action creation and
validation processes.
- New constants related to data sources and pages for better tracking
and logging.

- **Bug Fixes**
- Updated existing constants for better alignment with method names,
enhancing clarity and functionality.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
Rishabh Rathod authored Oct 28, 2024
1 parent 836bf20 commit 5bca179
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,9 @@ public class ActionSpanCE {
public static final String DELETE_ACTION = APPSMITH_SPAN_PREFIX + "delete.action";
public static final String FILL_SELF_REFERENCING_PATHS_ACTION =
APPSMITH_SPAN_PREFIX + "action.fillSelfReferencingPaths";

public static final String VALIDATE_AND_GENERATE_ACTION_DOMAIN_BASED_ON_CONTEXT =
APPSMITH_SPAN_PREFIX + ACTIONS + "validateAndGenerateActionDomainBasedOnContext";
public static final String VALIDATE_AND_SAVE_ACTION_TO_REPOSITORY =
APPSMITH_SPAN_PREFIX + ACTIONS + "validateAndSaveActionToRepository";
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
import static com.appsmith.external.constants.spans.BaseSpan.APPSMITH_SPAN_PREFIX;

public class DatasourceSpanCE {
public static final String DATASOURCES = "datasources.";
public static final String FETCH_ALL_DATASOURCES_WITH_STORAGES =
APPSMITH_SPAN_PREFIX + "get_all_datasource_storage";
public static final String FETCH_ALL_PLUGINS_IN_WORKSPACE = APPSMITH_SPAN_PREFIX + "get_all_plugins_in_workspace";

// datasource service spans
public static final String DATASOURCE_SERVICE = "datasourceService";
public static final String FIND_DATASOURCE_BY_ID = APPSMITH_SPAN_PREFIX + DATASOURCE_SERVICE + ".findById";
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ public class PageSpanCE {
public static final String PREPARE_APPLICATION_PAGES_DTO_FROM_PAGES = PAGES + "generate_app_pages_dto";
public static final String MIGRATE_DSL = PAGES + "migrate_dsl";

public static final String GET_PAGE_BY_ID = APPSMITH_SPAN_PREFIX + "get.page.unpublished";
public static final String GET_PAGE_BY_ID_AND_LAYOUTS_ID = APPSMITH_SPAN_PREFIX + "getPageByIdAndLayoutsId";
public static final String GET_PAGE_BY_ID = APPSMITH_SPAN_PREFIX + PAGES + "newPageService.findById";
public static final String GET_PAGE_BY_ID_AND_LAYOUTS_ID = APPSMITH_SPAN_PREFIX + PAGES + "getPageByIdAndLayoutsId";

// page level method added here
public static final String IS_NAME_ALLOWED = APPSMITH_SPAN_PREFIX + PAGES + "refactoringService.isNameAllowed";
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@
import static com.appsmith.external.constants.spans.ActionSpan.GET_ACTION_BY_ID;
import static com.appsmith.external.constants.spans.ActionSpan.UPDATE_ACTION_BASED_ON_CONTEXT;
import static com.appsmith.external.constants.spans.ActionSpan.UPDATE_SINGLE_ACTION;
import static com.appsmith.external.constants.spans.ActionSpan.VALIDATE_AND_GENERATE_ACTION_DOMAIN_BASED_ON_CONTEXT;
import static com.appsmith.external.constants.spans.ActionSpan.VALIDATE_AND_SAVE_ACTION_TO_REPOSITORY;
import static com.appsmith.external.constants.spans.DatasourceSpan.FIND_DATASOURCE_BY_ID;
import static com.appsmith.external.constants.spans.LayoutSpan.UPDATE_PAGE_LAYOUT_BY_PAGE_ID;
import static com.appsmith.external.constants.spans.PageSpan.GET_PAGE_BY_ID;
import static com.appsmith.external.constants.spans.PageSpan.IS_NAME_ALLOWED;
import static java.util.stream.Collectors.toSet;

@Slf4j
Expand Down Expand Up @@ -347,6 +352,8 @@ protected Mono<ActionDTO> createAction(ActionDTO actionDTO, Boolean isJsAction)
public Mono<ActionDTO> createAction(ActionDTO actionDTO, AppsmithEventContext eventContext, Boolean isJsAction) {

return validateAndGenerateActionDomainBasedOnContext(actionDTO, isJsAction)
.name(VALIDATE_AND_GENERATE_ACTION_DOMAIN_BASED_ON_CONTEXT)
.tap(Micrometer.observation(observationRegistry))
.flatMap(newAction -> {
// If the datasource is embedded, check for workspaceId and set it in action
if (actionDTO.getDatasource() != null
Expand All @@ -371,13 +378,17 @@ public Mono<ActionDTO> createAction(ActionDTO actionDTO, AppsmithEventContext ev
})
.flatMap(savedNewAction -> newActionService
.validateAndSaveActionToRepository(savedNewAction)
.name(VALIDATE_AND_SAVE_ACTION_TO_REPOSITORY)
.tap(Micrometer.observation(observationRegistry))
.zipWith(Mono.just(savedNewAction)))
.zipWhen(zippedActions -> {
ActionDTO savedActionDTO = zippedActions.getT1();
if (savedActionDTO.getDatasource() != null
&& savedActionDTO.getDatasource().getId() != null) {
return datasourceService.findById(
savedActionDTO.getDatasource().getId());
return datasourceService
.findById(savedActionDTO.getDatasource().getId())
.name(FIND_DATASOURCE_BY_ID)
.tap(Micrometer.observation(observationRegistry));
} else {
return Mono.justOrEmpty(savedActionDTO.getDatasource());
}
Expand Down Expand Up @@ -408,6 +419,8 @@ protected Mono<NewAction> validateAndGenerateActionDomainBasedOnContext(ActionDT

Mono<NewPage> pageMono = newPageService
.findById(action.getPageId(), aclPermission)
.name(GET_PAGE_BY_ID)
.tap(Micrometer.observation(observationRegistry))
.switchIfEmpty(Mono.error(
new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.PAGE, action.getPageId())))
.cache();
Expand All @@ -419,7 +432,10 @@ protected Mono<NewAction> validateAndGenerateActionDomainBasedOnContext(ActionDT
String name = action.getValidName();
CreatorContextType contextType =
action.getContextType() == null ? CreatorContextType.PAGE : action.getContextType();
return refactoringService.isNameAllowed(page.getId(), contextType, layout.getId(), name);
return refactoringService
.isNameAllowed(page.getId(), contextType, layout.getId(), name)
.name(IS_NAME_ALLOWED)
.tap(Micrometer.observation(observationRegistry));
})
.flatMap(nameAllowed -> {
// If the name is allowed, return pageMono for further processing
Expand Down

0 comments on commit 5bca179

Please sign in to comment.