chore: Add spans attributes for no of lines and action count (#37001)

## Description Fixes #36995 ## 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/11455841055> > Commit: 7285f3f > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11455841055&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` > Spec: > <hr>Tue, 22 Oct 2024 08:35:48 UTC  ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No  ## Summary by CodeRabbit - **New Features** - Added new constants for layout operations and data extraction to improve functionality. - Introduced observability tracking for various methods to enhance monitoring and performance insights. - **Bug Fixes** - Enhanced error handling and logging for action updates and layout modifications. - **Refactor** - Improved clarity and maintainability of methods related to action collections and layout updates. - **Chores** - Updated method signatures to include new observability parameters for better tracking. - Added metrics for line and action counts in action collection updates.
appsmithorg · Oct 22, 2024 · dc5fbed · dc5fbed
1 parent 4f55f52
commit dc5fbed
Show file tree

Hide file tree

Showing 8 changed files with 142 additions and 29 deletions.
diff --git a/...r/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/OnLoadSpan.java b/...r/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/OnLoadSpan.java
@@ -0,0 +1,5 @@
+package com.appsmith.external.constants.spans;
+
+import com.appsmith.external.constants.spans.ce.OnLoadSpanCE;
+
+public class OnLoadSpan extends OnLoadSpanCE {}
diff --git a/...smith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/LayoutSpanCE.java b/...smith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/LayoutSpanCE.java
@@ -15,7 +15,9 @@ public class LayoutSpanCE {
             APPSMITH_SPAN_PREFIX + "onLoadExecutablesUtil.updateExecutablesExecuteOnLoad";
     public static final String FIND_AND_UPDATE_LAYOUT =
             APPSMITH_SPAN_PREFIX + "onLoadExecutablesUtil.findAndUpdateLayout";
-
+    public static final String UNESCAPE_MONGO_SPECIAL_CHARS = APPSMITH_SPAN_PREFIX + "unescapeMongoSpecialCharacters";
+    public static final String EXTRACT_ALL_WIDGET_NAMES_AND_DYNAMIC_BINDINGS_FROM_DSL =
+            APPSMITH_SPAN_PREFIX + "extractAllWidgetNamesAndDynamicBindingsFromDSL";
     public static final String EXTRACT_AND_SET_EXECUTABLE_BINDINGS_IN_GRAPH_EDGES =
             APPSMITH_SPAN_PREFIX + "extractAndSetExecutableBindingsInGraphEdges";
     public static final String RECURSIVELY_ADD_EXECUTABLES_AND_THEIR_DEPENDENTS_TO_GRAPH_FROM_BINDINGS =

diff --git a/...smith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/OnLoadSpanCE.java b/...smith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/OnLoadSpanCE.java
@@ -0,0 +1,22 @@
+package com.appsmith.external.constants.spans.ce;
+
+import static com.appsmith.external.constants.spans.BaseSpan.APPSMITH_SPAN_PREFIX;
+
+public class OnLoadSpanCE {
+
+    public static final String GET_ALL_EXECUTABLES_BY_CREATOR_ID =
+            APPSMITH_SPAN_PREFIX + "getAllExecutablesByCreatorIdFlux";
+    public static final String EXECUTABLE_NAME_TO_EXECUTABLE_MAP =
+            APPSMITH_SPAN_PREFIX + "executableNameToExecutableMap";
+    public static final String EXECUTABLE_IN_CREATOR_CONTEXT = APPSMITH_SPAN_PREFIX + "executablesInCreatorContext";
+    public static final String ADD_DIRECTLY_REFERENCED_EXECUTABLES_TO_GRAPH =
+            APPSMITH_SPAN_PREFIX + "addDirectlyReferencedExecutablesToGraph";
+    public static final String GET_POSSIBLE_ENTITY_REFERENCES = APPSMITH_SPAN_PREFIX + "getPossibleEntityReferences";
+    public static final String UPDATE_EXECUTABLE_SELF_REFERENCING_PATHS =
+            APPSMITH_SPAN_PREFIX + "updateExecutableSelfReferencingPaths";
+    public static final String GET_POSSIBLE_ENTITY_PARENTS_MAP = APPSMITH_SPAN_PREFIX + "getPossibleEntityParentsMap";
+    public static final String ADD_EXPLICIT_USER_SET_ON_LOAD_EXECUTABLES_TO_GRAPH =
+            APPSMITH_SPAN_PREFIX + "addExplicitUserSetOnLoadExecutablesToGraph";
+    public static final String GET_UNPUBLISHED_ON_LOAD_EXECUTABLES_EXPLICIT_SET_BY_USER_IN_CREATOR_CONTEXT =
+            APPSMITH_SPAN_PREFIX + "getUnpublishedOnLoadExecutablesExplicitSetByUserInCreatorContext";
+}
diff --git a/.../appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceCEImpl.java b/.../appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceCEImpl.java
@@ -17,6 +17,7 @@
 import com.appsmith.server.dtos.UpdateMultiplePageLayoutDTO;
 import com.appsmith.server.exceptions.AppsmithError;
 import com.appsmith.server.exceptions.AppsmithException;
+import com.appsmith.server.helpers.ObservationHelperImpl;
 import com.appsmith.server.helpers.WidgetSpecificUtils;
 import com.appsmith.server.newpages.base.NewPageService;
 import com.appsmith.server.onload.internal.OnLoadExecutablesUtil;
@@ -26,6 +27,7 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import io.micrometer.observation.ObservationRegistry;
+import io.micrometer.tracing.Span;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import net.minidev.json.JSONObject;
@@ -48,6 +50,7 @@
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
+import static com.appsmith.external.constants.spans.LayoutSpan.EXTRACT_ALL_WIDGET_NAMES_AND_DYNAMIC_BINDINGS_FROM_DSL;
 import static com.appsmith.external.constants.spans.LayoutSpan.FIND_ALL_ON_LOAD_EXECUTABLES;
 import static com.appsmith.external.constants.spans.LayoutSpan.FIND_AND_UPDATE_LAYOUT;
 import static com.appsmith.external.constants.spans.LayoutSpan.UPDATE_EXECUTABLES_EXECUTE_ONLOAD;
@@ -70,6 +73,7 @@ public class UpdateLayoutServiceCEImpl implements UpdateLayoutServiceCE {
     private final ApplicationService applicationService;
     private final ObjectMapper objectMapper;
     private final ObservationRegistry observationRegistry;
+    private final ObservationHelperImpl observationHelper;
 
     private final String layoutOnLoadActionErrorToastMessage =
             "A cyclic dependency error has been encountered on current page, \nqueries on page load will not run. \n Please check debugger and Appsmith documentation for more information";
@@ -127,6 +131,12 @@ private Mono<LayoutDTO> updateLayoutDsl(
         Set<String> widgetNames = new HashSet<>();
         Map<String, Set<String>> widgetDynamicBindingsMap = new HashMap<>();
         Set<String> escapedWidgetNames = new HashSet<>();
+
+        Span extractAllWidgetNamesAndDynamicBindingsFromDSLSpan =
+                observationHelper.createSpan(EXTRACT_ALL_WIDGET_NAMES_AND_DYNAMIC_BINDINGS_FROM_DSL);
+
+        observationHelper.startSpan(extractAllWidgetNamesAndDynamicBindingsFromDSLSpan, true);
+
         try {
             dsl = extractAllWidgetNamesAndDynamicBindingsFromDSL(
                     dsl, widgetNames, widgetDynamicBindingsMap, creatorId, layoutId, escapedWidgetNames, creatorType);
@@ -136,6 +146,8 @@ private Mono<LayoutDTO> updateLayoutDsl(
                     .then(Mono.error(t));
         }
 
+        observationHelper.endSpan(extractAllWidgetNamesAndDynamicBindingsFromDSLSpan, true);
+
         layout.setWidgetNames(widgetNames);
 
         if (!escapedWidgetNames.isEmpty()) {
@@ -151,7 +163,8 @@ private Mono<LayoutDTO> updateLayoutDsl(
 
         AtomicReference<Boolean> validOnLoadExecutables = new AtomicReference<>(Boolean.TRUE);
 
-        // setting the layoutOnLoadActionActionErrors to empty to remove the existing errors before new DAG calculation.
+        // setting the layoutOnLoadActionActionErrors to empty to remove the existing
+        // errors before new DAG calculation.
         layout.setLayoutOnLoadActionErrors(new ArrayList<>());
 
         Mono<List<Set<DslExecutableDTO>>> allOnLoadExecutablesMono = onLoadExecutablesUtil
@@ -180,14 +193,18 @@ private Mono<LayoutDTO> updateLayoutDsl(
 
         // First update the actions and set execute on load to true
         JSONObject finalDsl = dsl;
-        return allOnLoadExecutablesMono
+
+        Mono<LayoutDTO> layoutDTOMono = allOnLoadExecutablesMono
                 .flatMap(allOnLoadExecutables -> {
-                    // If there has been an error (e.g. cyclical dependency), then don't update any actions.
-                    // This is so that unnecessary updates don't happen to actions while the page is in invalid state.
+                    // If there has been an error (e.g. cyclical dependency), then don't update any
+                    // actions.
+                    // This is so that unnecessary updates don't happen to actions while the page is
+                    // in invalid state.
                     if (!validOnLoadExecutables.get()) {
                         return Mono.just(allOnLoadExecutables);
                     }
-                    // Update these executables to be executed on load, unless the user has touched the executeOnLoad
+                    // Update these executables to be executed on load, unless the user has touched
+                    // the executeOnLoad
                     // setting for this
                     return onLoadExecutablesUtil
                             .updateExecutablesExecuteOnLoad(
@@ -201,12 +218,15 @@ private Mono<LayoutDTO> updateLayoutDsl(
                     layout.setLayoutOnLoadActions(onLoadExecutables);
                     layout.setAllOnPageLoadActionNames(executableNames);
                     layout.setActionsUsedInDynamicBindings(executablesUsedInDSL);
-                    // The below field is to ensure that we record if the page load actions computation was
+                    // The below field is to ensure that we record if the page load actions
+                    // computation was
                     // valid when last stored in the database.
                     layout.setValidOnPageLoadActions(validOnLoadExecutables.get());
 
                     return onLoadExecutablesUtil
                             .findAndUpdateLayout(creatorId, creatorType, layoutId, layout)
+                            .tag("no_of_widgets", String.valueOf(widgetNames.size()))
+                            .tag("no_of_executables", String.valueOf(executableNames.size()))
                             .name(FIND_AND_UPDATE_LAYOUT)
                             .tap(Micrometer.observation(observationRegistry));
                 })
@@ -222,6 +242,8 @@ private Mono<LayoutDTO> updateLayoutDsl(
                     return sendUpdateLayoutAnalyticsEvent(creatorId, layoutId, finalDsl, true, null, creatorType)
                             .thenReturn(layoutDTO);
                 });
+
+        return layoutDTOMono;
     }
 
     @Override
@@ -326,6 +348,7 @@ public Mono<List<Set<DslExecutableDTO>>> getOnPageLoadActions(
         Set<String> widgetNames = new HashSet<>();
         Map<String, Set<String>> widgetDynamicBindingsMap = new HashMap<>();
         Set<String> escapedWidgetNames = new HashSet<>();
+        // observationHelper.createSpan()
         try {
             dsl = extractAllWidgetNamesAndDynamicBindingsFromDSL(
                     dsl, widgetNames, widgetDynamicBindingsMap, creatorId, layoutId, escapedWidgetNames, creatorType);

diff --git a/...er/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceImpl.java b/...er/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceImpl.java
@@ -1,6 +1,7 @@
 package com.appsmith.server.layouts;
 
 import com.appsmith.server.applications.base.ApplicationService;
+import com.appsmith.server.helpers.ObservationHelperImpl;
 import com.appsmith.server.newpages.base.NewPageService;
 import com.appsmith.server.onload.internal.OnLoadExecutablesUtil;
 import com.appsmith.server.services.AnalyticsService;
@@ -21,7 +22,8 @@ public UpdateLayoutServiceImpl(
             PagePermission pagePermission,
             ApplicationService applicationService,
             ObjectMapper objectMapper,
-            ObservationRegistry observationRegistry) {
+            ObservationRegistry observationRegistry,
+            ObservationHelperImpl observationHelper) {
         super(
                 onLoadExecutablesUtil,
                 sessionUserService,
@@ -30,6 +32,7 @@ public UpdateLayoutServiceImpl(
                 pagePermission,
                 applicationService,
                 objectMapper,
-                observationRegistry);
+                observationRegistry,
+                observationHelper);
     }
 }