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

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

Merged
merged 7 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.appsmith.external.constants.spans;

import com.appsmith.external.constants.spans.ce.OnLoadSpanCE;

public class OnLoadSpan extends OnLoadSpanCE {}
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
@@ -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";
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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";
Expand Down Expand Up @@ -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);
Expand All @@ -136,6 +146,8 @@ private Mono<LayoutDTO> updateLayoutDsl(
.then(Mono.error(t));
}

observationHelper.endSpan(extractAllWidgetNamesAndDynamicBindingsFromDSLSpan, true);

layout.setWidgetNames(widgetNames);

if (!escapedWidgetNames.isEmpty()) {
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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));
})
Expand All @@ -222,6 +242,8 @@ private Mono<LayoutDTO> updateLayoutDsl(
return sendUpdateLayoutAnalyticsEvent(creatorId, layoutId, finalDsl, true, null, creatorType)
.thenReturn(layoutDTO);
});

return layoutDTOMono;
}

@Override
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -21,7 +22,8 @@ public UpdateLayoutServiceImpl(
PagePermission pagePermission,
ApplicationService applicationService,
ObjectMapper objectMapper,
ObservationRegistry observationRegistry) {
ObservationRegistry observationRegistry,
ObservationHelperImpl observationHelper) {
super(
onLoadExecutablesUtil,
sessionUserService,
Expand All @@ -30,6 +32,7 @@ public UpdateLayoutServiceImpl(
pagePermission,
applicationService,
objectMapper,
observationRegistry);
observationRegistry,
observationHelper);
}
}
Loading
Loading