From df5d50a0de187d6f1d808894b79e34851591308c Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Wed, 17 Jun 2026 08:01:05 +0200 Subject: [PATCH 1/6] UNOMI-873: Add request tracing and web-servlets extraction Introduce tracing-api/tracing-impl for the Explain feature, wire TracerService across services, evaluators, actions, and REST endpoints, and extract legacy HTTP servlets into the web-servlets module while preserving master review fixes. --- api/pom.xml | 6 + .../org/apache/unomi/api/ContextResponse.java | 11 + bom/artifacts/pom.xml | 15 + extensions/geonames/services/pom.xml | 8 +- extensions/groovy-actions/services/pom.xml | 13 + .../actions/GroovyActionDispatcher.java | 62 ++-- extensions/json-schema/services/pom.xml | 13 + .../unomi/schema/impl/SchemaServiceImpl.java | 15 + .../OSGI-INF/blueprint/blueprint.xml | 2 + extensions/router/router-core/pom.xml | 8 +- itests/pom.xml | 8 +- kar/src/main/feature/feature.xml | 5 + persistence-spi/pom.xml | 7 + .../conditions/ConditionContextHelper.java | 87 +++++- .../ConditionEvaluatorDispatcherImpl.java | 50 +++- plugins/baseplugin/pom.xml | 6 + .../AllEventToProfilePropertiesAction.java | 76 +++-- .../actions/CopyPropertiesAction.java | 117 ++++++-- .../actions/EvaluateProfileAgeAction.java | 80 ++++- .../EvaluateProfileSegmentsAction.java | 90 +++++- .../EvaluateVisitPropertiesAction.java | 100 +++++-- .../actions/EventToProfilePropertyAction.java | 51 +++- .../actions/IncrementPropertyAction.java | 84 ++++-- .../MergeProfilesOnPropertyAction.java | 61 ++++ .../actions/ModifyConsentAction.java | 74 ++++- .../baseplugin/actions/SendEventAction.java | 66 +++- .../actions/SetEventOccurenceCountAction.java | 174 ++++++----- .../baseplugin/actions/SetPropertyAction.java | 86 ++++-- .../actions/UpdatePropertiesAction.java | 119 +++++--- .../conditions/BooleanConditionEvaluator.java | 76 +++-- .../conditions/IdsConditionEvaluator.java | 45 ++- .../conditions/NotConditionEvaluator.java | 38 ++- .../PropertyConditionEvaluator.java | 41 +++ .../OSGI-INF/blueprint/blueprint.xml | 47 ++- pom.xml | 2 + rest/pom.xml | 7 + .../rest/endpoints/ContextJsonEndpoint.java | 40 ++- .../endpoints/EventsCollectorEndpoint.java | 33 +- .../rest/models/EventCollectorResponse.java | 32 +- services/pom.xml | 13 + .../impl/ActionExecutorDispatcherImpl.java | 131 ++++++-- .../definitions/DefinitionsServiceImpl.java | 23 ++ .../impl/events/EventServiceImpl.java | 57 ++++ .../services/impl/goals/GoalsServiceImpl.java | 190 +++++++++--- .../services/impl/rules/RulesServiceImpl.java | 58 +++- .../impl/segments/SegmentServiceImpl.java | 189 ++++++++---- .../OSGI-INF/blueprint/blueprint.xml | 27 +- tracing/pom.xml | 36 +++ tracing/tracing-api/pom.xml | 68 +++++ .../unomi/tracing/api/RequestTracer.java | 84 ++++++ .../apache/unomi/tracing/api/TraceNode.java | 109 +++++++ .../unomi/tracing/api/TracerService.java | 52 ++++ tracing/tracing-impl/pom.xml | 94 ++++++ .../tracing/impl/DefaultRequestTracer.java | 146 +++++++++ .../tracing/impl/DefaultTracerService.java | 69 +++++ .../tracing/impl/LoggingRequestTracer.java | 160 ++++++++++ .../impl/DefaultTracerServiceTest.java | 281 ++++++++++++++++++ wab/pom.xml | 110 ------- web-servlets/pom.xml | 189 ++++++++++++ .../unomi/web/servlets}/ClientServlet.java | 14 +- .../unomi/web/servlets}/ContextServlet.java | 15 +- .../web/servlets}/EventsCollectorServlet.java | 14 +- .../HttpServletRequestForwardWrapper.java | 2 +- .../apache/unomi/web/servlets}/HttpUtils.java | 2 +- .../apache/unomi/web/servlets}/WebConfig.java | 2 +- .../main/resources/org.apache.unomi.web.cfg | 0 66 files changed, 3343 insertions(+), 647 deletions(-) create mode 100644 tracing/pom.xml create mode 100644 tracing/tracing-api/pom.xml create mode 100644 tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/RequestTracer.java create mode 100644 tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/TraceNode.java create mode 100644 tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/TracerService.java create mode 100644 tracing/tracing-impl/pom.xml create mode 100644 tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultRequestTracer.java create mode 100644 tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultTracerService.java create mode 100644 tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/LoggingRequestTracer.java create mode 100644 tracing/tracing-impl/src/test/java/org/apache/unomi/tracing/impl/DefaultTracerServiceTest.java create mode 100644 web-servlets/pom.xml rename {wab/src/main/java/org/apache/unomi/web => web-servlets/src/main/java/org/apache/unomi/web/servlets}/ClientServlet.java (84%) rename {wab/src/main/java/org/apache/unomi/web => web-servlets/src/main/java/org/apache/unomi/web/servlets}/ContextServlet.java (82%) rename {wab/src/main/java/org/apache/unomi/web => web-servlets/src/main/java/org/apache/unomi/web/servlets}/EventsCollectorServlet.java (83%) rename {wab/src/main/java/org/apache/unomi/web => web-servlets/src/main/java/org/apache/unomi/web/servlets}/HttpServletRequestForwardWrapper.java (98%) rename {wab/src/main/java/org/apache/unomi/web => web-servlets/src/main/java/org/apache/unomi/web/servlets}/HttpUtils.java (98%) rename {wab/src/main/java/org/apache/unomi/web => web-servlets/src/main/java/org/apache/unomi/web/servlets}/WebConfig.java (99%) rename {wab => web-servlets}/src/main/resources/org.apache.unomi.web.cfg (100%) diff --git a/api/pom.xml b/api/pom.xml index b1933fbed9..93e1603ee1 100644 --- a/api/pom.xml +++ b/api/pom.xml @@ -43,6 +43,12 @@ + + org.apache.unomi + unomi-tracing-api + ${project.version} + provided + jakarta.xml.bind jakarta.xml.bind-api diff --git a/api/src/main/java/org/apache/unomi/api/ContextResponse.java b/api/src/main/java/org/apache/unomi/api/ContextResponse.java index 8c158ec768..de2335390d 100644 --- a/api/src/main/java/org/apache/unomi/api/ContextResponse.java +++ b/api/src/main/java/org/apache/unomi/api/ContextResponse.java @@ -19,6 +19,7 @@ import org.apache.unomi.api.conditions.Condition; import org.apache.unomi.api.services.RulesService; +import org.apache.unomi.tracing.api.TraceNode; import java.io.Serializable; import java.util.*; @@ -59,6 +60,8 @@ public class ContextResponse implements Serializable { private Map consents = new LinkedHashMap<>(); + private TraceNode requestTracing; + /** * Retrieves the profile identifier associated with the profile of the user on behalf of which the client performed the context request. * @@ -285,4 +288,12 @@ public Map getConsents() { public void setConsents(Map consents) { this.consents = consents; } + + public TraceNode getRequestTracing() { + return requestTracing; + } + + public void setRequestTracing(TraceNode requestTracing) { + this.requestTracing = requestTracing; + } } diff --git a/bom/artifacts/pom.xml b/bom/artifacts/pom.xml index 5a739b1017..07c372c3fa 100644 --- a/bom/artifacts/pom.xml +++ b/bom/artifacts/pom.xml @@ -245,6 +245,21 @@ graphql-providers-sample ${project.version} + + org.apache.unomi + unomi-tracing-api + ${project.version} + + + org.apache.unomi + unomi-tracing-impl + ${project.version} + + + org.apache.unomi + unomi-web-servlets + ${project.version} + org.apache.unomi unomi-plugins-base diff --git a/extensions/geonames/services/pom.xml b/extensions/geonames/services/pom.xml index c83b1299a2..a44f3131c9 100644 --- a/extensions/geonames/services/pom.xml +++ b/extensions/geonames/services/pom.xml @@ -37,7 +37,13 @@ pom import - + + org.apache.unomi + unomi-tracing-impl + ${project.version} + test + + diff --git a/extensions/groovy-actions/services/pom.xml b/extensions/groovy-actions/services/pom.xml index 751baf13e0..d4070f3e6a 100644 --- a/extensions/groovy-actions/services/pom.xml +++ b/extensions/groovy-actions/services/pom.xml @@ -46,6 +46,19 @@ unomi-api provided + + org.apache.unomi + unomi-tracing-api + ${project.version} + provided + + + org.apache.unomi + unomi-tracing-impl + ${project.version} + test + + org.apache.unomi unomi-metrics diff --git a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java index 093a91d6f9..17932568ae 100644 --- a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java +++ b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java @@ -21,15 +21,20 @@ import org.apache.unomi.api.actions.Action; import org.apache.unomi.api.actions.ActionDispatcher; import org.apache.unomi.api.services.DefinitionsService; +import org.apache.unomi.api.services.EventService; import org.apache.unomi.groovy.actions.services.GroovyActionsService; import org.apache.unomi.metrics.MetricAdapter; import org.apache.unomi.metrics.MetricsService; import org.apache.unomi.services.actions.ActionExecutorDispatcher; +import org.apache.unomi.tracing.api.TracerService; +import org.apache.unomi.tracing.api.RequestTracer; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Reference; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.HashMap; + /** * High-performance ActionDispatcher for pre-compiled Groovy scripts. * Executes scripts without GroovyShell overhead using isolated instances. @@ -46,6 +51,7 @@ public class GroovyActionDispatcher implements ActionDispatcher { private GroovyActionsService groovyActionsService; private DefinitionsService definitionsService; private ActionExecutorDispatcher actionExecutorDispatcher; + private TracerService tracerService; @Reference public void setMetricsService(MetricsService metricsService) { @@ -67,32 +73,54 @@ public void setActionExecutorDispatcher(ActionExecutorDispatcher actionExecutorD this.actionExecutorDispatcher = actionExecutorDispatcher; } + @Reference + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + public String getPrefix() { return GROOVY_PREFIX; } public Integer execute(Action action, Event event, String actionName) { - Class scriptClass = groovyActionsService.getCompiledScript(actionName); - if (scriptClass == null) { - LOGGER.warn("Couldn't find a Groovy action with name {}, action will not execute!", actionName); - return 0; + RequestTracer tracer = tracerService.getCurrentTracer(); + if (!tracer.isEnabled()) { + tracer.setEnabled(true); } - + + tracer.startOperation("groovy-action", "Executing Groovy action", new HashMap() {{ + put("action.name", actionName); + put("action.type", action.getActionTypeId()); + put("event.type", event.getEventType()); + }}); + try { - Script script = scriptClass.getDeclaredConstructor().newInstance(); - setScriptVariables(script, action, event); - - return new MetricAdapter(metricsService, this.getClass().getName() + ".action.groovy." + actionName) { - @Override - public Integer execute(Object... args) throws Exception { - return (Integer) script.invokeMethod("execute", null); - } - }.runWithTimer(); + Class scriptClass = groovyActionsService.getCompiledScript(actionName); + if (scriptClass == null) { + LOGGER.warn("Couldn't find a Groovy action with name {}, action will not execute!", actionName); + tracer.trace("Action not found", null); + return EventService.NO_CHANGE; + } - } catch (Exception e) { - LOGGER.error("Error executing Groovy action with key={}", actionName, e); + try { + Script script = scriptClass.getDeclaredConstructor().newInstance(); + setScriptVariables(script, action, event); + + return new MetricAdapter(metricsService, this.getClass().getName() + ".action.groovy." + actionName) { + @Override + public Integer execute(Object... args) throws Exception { + return (Integer) script.invokeMethod("execute", null); + } + }.runWithTimer(); + + } catch (Exception e) { + LOGGER.error("Error executing Groovy action with key={}", actionName, e); + tracer.trace("Error executing action", e); + return EventService.NO_CHANGE; + } + } finally { + tracer.endOperation(null, "Completed Groovy action execution"); } - return 0; } /** diff --git a/extensions/json-schema/services/pom.xml b/extensions/json-schema/services/pom.xml index 710a7d1a01..65c96a3882 100644 --- a/extensions/json-schema/services/pom.xml +++ b/extensions/json-schema/services/pom.xml @@ -52,6 +52,19 @@ unomi-api provided + + org.apache.unomi + unomi-tracing-api + ${project.version} + provided + + + org.apache.unomi + unomi-tracing-impl + ${project.version} + test + + org.apache.unomi unomi-persistence-spi diff --git a/extensions/json-schema/services/src/main/java/org/apache/unomi/schema/impl/SchemaServiceImpl.java b/extensions/json-schema/services/src/main/java/org/apache/unomi/schema/impl/SchemaServiceImpl.java index 5a2cd09265..5a7068ac1f 100644 --- a/extensions/json-schema/services/src/main/java/org/apache/unomi/schema/impl/SchemaServiceImpl.java +++ b/extensions/json-schema/services/src/main/java/org/apache/unomi/schema/impl/SchemaServiceImpl.java @@ -40,6 +40,8 @@ import org.apache.unomi.schema.api.ValidationError; import org.apache.unomi.schema.api.ValidationException; import org.apache.unomi.schema.keyword.ScopeKeyword; +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TracerService; import org.apache.unomi.services.common.cache.AbstractMultiTypeCachingService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -78,6 +80,7 @@ public class SchemaServiceImpl extends AbstractMultiTypeCachingService implement private Integer jsonSchemaRefreshInterval = 1000; private ScopeService scopeService; + private TracerService tracerService; // Map to store tenant-specific JsonSchemaFactory instances private final ConcurrentMap tenantJsonSchemaFactories = new ConcurrentHashMap<>(); @@ -375,6 +378,14 @@ private Set validate(JsonNode jsonNode, JsonSchema jsonSchema) } } + // Add validation info to trace if tracing is enabled + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + tracer.addValidationInfo(validationMessages, jsonSchema.getCurrentUri().toString()); + } + } + return validationMessages != null ? validationMessages.stream() .map(validationMessage -> new ValidationError(validationMessage.getMessage())) @@ -594,6 +605,10 @@ public void setJsonSchemaRefreshInterval(Integer jsonSchemaRefreshInterval) { this.jsonSchemaRefreshInterval = jsonSchemaRefreshInterval; } + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + /** * Refreshes schema extensions and factories with the provided schemas map. * This method encapsulates the common logic needed after schema changes. diff --git a/extensions/json-schema/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/extensions/json-schema/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml index d73eca09cc..b89e3d0036 100644 --- a/extensions/json-schema/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml +++ b/extensions/json-schema/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml @@ -27,6 +27,7 @@ + @@ -44,6 +45,7 @@ + diff --git a/extensions/router/router-core/pom.xml b/extensions/router/router-core/pom.xml index c28a7e1b0e..0ec3cfc1bc 100644 --- a/extensions/router/router-core/pom.xml +++ b/extensions/router/router-core/pom.xml @@ -36,7 +36,13 @@ pom import - + + org.apache.unomi + unomi-tracing-impl + ${project.version} + test + + diff --git a/itests/pom.xml b/itests/pom.xml index 55b19b39d6..3e8c79ee05 100644 --- a/itests/pom.xml +++ b/itests/pom.xml @@ -54,7 +54,13 @@ awaitility 3.1.6 - + + org.apache.unomi + unomi-tracing-api + ${project.version} + test + + diff --git a/kar/src/main/feature/feature.xml b/kar/src/main/feature/feature.xml index 2a764410fc..9d59b30229 100644 --- a/kar/src/main/feature/feature.xml +++ b/kar/src/main/feature/feature.xml @@ -68,6 +68,8 @@ mvn:org.apache.unomi/unomi-lifecycle-watcher/${project.version} mvn:org.apache.unomi/unomi-api/${project.version} mvn:org.apache.unomi/unomi-common/${project.version} + mvn:org.apache.unomi/unomi-tracing-api/${project.version} + mvn:org.apache.unomi/unomi-tracing-impl/${project.version} mvn:org.apache.unomi/unomi-scripting/${project.version} mvn:org.apache.unomi/unomi-metrics/${project.version} mvn:org.apache.unomi/unomi-persistence-spi/${project.version} @@ -130,10 +132,13 @@ unomi-cxs-privacy-extension-services mvn:org.apache.unomi/unomi-json-schema-services/${project.version}/cfg/schemacfg mvn:org.apache.unomi/unomi-rest/${project.version}/cfg/restauth + + mvn:org.apache.unomi/unomi-web-servlets/${project.version}/cfg/unomicfg mvn:org.apache.unomi/unomi-json-schema-services/${project.version} mvn:org.apache.unomi/unomi-rest/${project.version} mvn:org.apache.unomi/unomi-json-schema-rest/${project.version} mvn:org.apache.unomi/cxs-lists-extension-actions/${project.version} + mvn:org.apache.unomi/unomi-web-servlets/${project.version} diff --git a/persistence-spi/pom.xml b/persistence-spi/pom.xml index a50c555afd..54cb4608cd 100644 --- a/persistence-spi/pom.xml +++ b/persistence-spi/pom.xml @@ -47,6 +47,13 @@ unomi-api provided + + org.apache.unomi + unomi-tracing-api + ${project.version} + provided + + org.apache.unomi unomi-scripting diff --git a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelper.java b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelper.java index d8c974af70..8e0b083138 100644 --- a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelper.java +++ b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelper.java @@ -23,6 +23,8 @@ import org.apache.unomi.api.conditions.ConditionType; import org.apache.unomi.api.services.DefinitionsService; import org.apache.unomi.scripting.ScriptExecutor; +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TracerService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -114,7 +116,7 @@ private static void loadMappingFile() throws IOException { } public static Condition getContextualCondition(Condition condition, Map context, ScriptExecutor scriptExecutor) { - return getContextualCondition(condition, context, scriptExecutor, null); + return getContextualCondition(condition, context, scriptExecutor, null, null); } /** @@ -125,13 +127,24 @@ public static Condition getContextualCondition(Condition condition, Map context, ScriptExecutor scriptExecutor, DefinitionsService definitionsService) { + return getContextualCondition(condition, context, scriptExecutor, definitionsService, null); + } + + public static Condition getContextualCondition( + Condition condition, + Map context, + ScriptExecutor scriptExecutor, + DefinitionsService definitionsService, + TracerService tracerService) { // Debug logging @@ -166,7 +179,7 @@ public static Condition getContextualCondition( Object rawValues = parseParameterWithValidation( context, condition.getParameterValues(), scriptExecutor, - parameterDefs, condition.getConditionTypeId()); + parameterDefs, tracerService, condition.getConditionTypeId()); if (rawValues == null || rawValues == RESOLUTION_ERROR) { return null; @@ -181,7 +194,7 @@ public static Condition getContextualCondition( @SuppressWarnings("unchecked") private static Object parseParameter(Map context, Object value, ScriptExecutor scriptExecutor) { - return parseParameterWithValidation(context, value, scriptExecutor, null, null); + return parseParameterWithValidation(context, value, scriptExecutor, null, null, null); } @SuppressWarnings("unchecked") @@ -190,10 +203,12 @@ private static Object parseParameterWithValidation( Object value, ScriptExecutor scriptExecutor, Map parameterDefs, + TracerService tracerService, String conditionTypeId) { return parseParameterWithValidationRecursive( - context, value, scriptExecutor, parameterDefs, conditionTypeId, new HashSet<>(), 0); + context, value, scriptExecutor, parameterDefs, + tracerService, conditionTypeId, new HashSet<>(), 0); } /** @@ -204,10 +219,11 @@ private static Object parseParameterWithValidation( * @param value the value to resolve * @param scriptExecutor executor for script expressions * @param parameterDefs optional parameter definitions for validation + * @param tracerService optional tracer service for warnings * @param conditionTypeId condition type ID for context * @param resolutionChain set of parameter keys/scripts already being resolved (for cycle detection) * @param depth current resolution depth - * @return resolved value, or {@code RESOLUTION_ERROR} sentinel if cycle detected or max depth exceeded + * @return resolved value, or null if cycle detected or max depth exceeded */ @SuppressWarnings("unchecked") private static Object parseParameterWithValidationRecursive( @@ -215,6 +231,7 @@ private static Object parseParameterWithValidationRecursive( Object value, ScriptExecutor scriptExecutor, Map parameterDefs, + TracerService tracerService, String conditionTypeId, Set resolutionChain, int depth) { @@ -226,6 +243,16 @@ private static Object parseParameterWithValidationRecursive( "Possible infinite chain or very deep nesting. Resolution chain: %s", MAX_RESOLUTION_DEPTH, resolutionChain); LOGGER.error(message); + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + Map traceContext = new HashMap<>(); + traceContext.put("maxDepth", MAX_RESOLUTION_DEPTH); + traceContext.put("resolutionChain", new ArrayList<>(resolutionChain)); + traceContext.put("conditionTypeId", conditionTypeId); + tracer.trace("Parameter resolution depth exceeded", traceContext); + } + } return RESOLUTION_ERROR; } @@ -239,6 +266,16 @@ private static Object parseParameterWithValidationRecursive( "Circular parameter reference detected: %s. Resolution chain: %s", referenceKey, resolutionChain); LOGGER.warn(message); + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + Map traceContext = new HashMap<>(); + traceContext.put("circularReference", referenceKey); + traceContext.put("resolutionChain", new ArrayList<>(resolutionChain)); + traceContext.put("conditionTypeId", conditionTypeId); + tracer.trace("Circular parameter reference detected", traceContext); + } + } // Return a special marker to indicate cycle (we'll check for this in Map processing) return RESOLUTION_ERROR; } @@ -268,8 +305,10 @@ private static Object parseParameterWithValidationRecursive( // If resolved value is itself a parameter reference, continue resolving if (resolvedValue != null && isParameterReference(resolvedValue)) { Object furtherResolved = parseParameterWithValidationRecursive( - context, resolvedValue, scriptExecutor, parameterDefs, conditionTypeId, resolutionChain, depth + 1); - // Propagate RESOLUTION_ERROR if cycle/max-depth was detected, or resolved value otherwise + context, resolvedValue, scriptExecutor, parameterDefs, + tracerService, conditionTypeId, resolutionChain, depth + 1); + // If further resolution returns null due to cycle or max depth, propagate it + // But if it's just a missing parameter, return null (not a cycle) return furtherResolved; } @@ -287,17 +326,18 @@ private static Object parseParameterWithValidationRecursive( Parameter paramDef = parameterDefs != null ? parameterDefs.get(paramName) : null; Object parameter = parseParameterWithValidationRecursive( - context, paramValue, scriptExecutor, parameterDefs, conditionTypeId, resolutionChain, depth); + context, paramValue, scriptExecutor, parameterDefs, + tracerService, conditionTypeId, resolutionChain, depth); - // If resolution returned an error marker, propagate failure + // If resolution returned an error marker, return null for entire map if (parameter == RESOLUTION_ERROR) { - return RESOLUTION_ERROR; + return null; } // Validate type if parameter definition is available and value is not null if (parameter != null && paramDef != null && paramDef.getType() != null) { validateParameterType(paramName, parameter, paramDef, - conditionTypeId); + conditionTypeId, tracerService); } // Always put the value, even if null (missing parameter is valid) @@ -308,10 +348,11 @@ private static Object parseParameterWithValidationRecursive( List values = new ArrayList(); for (Object o : ((List) value)) { Object parameter = parseParameterWithValidationRecursive( - context, o, scriptExecutor, parameterDefs, conditionTypeId, resolutionChain, depth); - // If resolution returned an error marker, propagate failure (consistent with Map behavior) + context, o, scriptExecutor, parameterDefs, + tracerService, conditionTypeId, resolutionChain, depth); + // If resolution returned an error marker, skip this element if (parameter == RESOLUTION_ERROR) { - return RESOLUTION_ERROR; + continue; } // Add the value, even if null (missing parameter is valid in lists) values.add(parameter); @@ -323,18 +364,20 @@ private static Object parseParameterWithValidationRecursive( /** * Validates that a resolved parameter value matches the expected type. - * Logs warnings if type mismatch is detected. + * Logs warnings via tracer if type mismatch is detected. * * @param paramName parameter name * @param resolvedValue resolved parameter value * @param paramDef parameter definition * @param conditionTypeId condition type ID for context + * @param tracerService optional tracer service */ private static void validateParameterType( String paramName, Object resolvedValue, Parameter paramDef, - String conditionTypeId) { + String conditionTypeId, + TracerService tracerService) { if (resolvedValue == null || paramDef == null || paramDef.getType() == null) { return; @@ -350,6 +393,18 @@ private static void validateParameterType( LOGGER.warn(message + " (value: {})", resolvedValue); + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + Map traceContext = new HashMap<>(); + traceContext.put("parameter", paramName); + traceContext.put("expectedType", expectedType); + traceContext.put("actualType", actualType); + traceContext.put("value", resolvedValue); + traceContext.put("conditionTypeId", conditionTypeId); + tracer.trace("Parameter type mismatch detected", traceContext); + } + } } } diff --git a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/evaluator/impl/ConditionEvaluatorDispatcherImpl.java b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/evaluator/impl/ConditionEvaluatorDispatcherImpl.java index d63719c801..8c5b34cd01 100644 --- a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/evaluator/impl/ConditionEvaluatorDispatcherImpl.java +++ b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/evaluator/impl/ConditionEvaluatorDispatcherImpl.java @@ -28,6 +28,8 @@ import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluator; import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluatorDispatcher; import org.apache.unomi.scripting.ScriptExecutor; +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TracerService; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Reference; import org.osgi.service.component.annotations.ReferenceCardinality; @@ -52,6 +54,7 @@ public class ConditionEvaluatorDispatcherImpl private MetricsService metricsService; private ScriptExecutor scriptExecutor; + private TracerService tracerService; private DefinitionsService definitionsService; public ConditionEvaluatorDispatcherImpl() { @@ -85,6 +88,11 @@ public void unbindEvaluator(ConditionEvaluator evaluator, Map pr evaluators.remove((String) props.get("conditionEvaluatorId")); } + @Reference + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + @Override public void addEvaluator(String name, ConditionEvaluator evaluator) { evaluators.put(name, evaluator); @@ -106,6 +114,13 @@ public boolean eval(Condition condition, Item item, Map context) throw new UnsupportedOperationException("Null condition passed for item : " + item); } + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.startOperation("condition-evaluation", + "Evaluating condition: " + condition.getConditionTypeId(), condition); + } + try { // Resolve condition type if needed - try to resolve first if definitionsService is available if (condition.getConditionType() == null) { @@ -120,6 +135,9 @@ public boolean eval(Condition condition, Item item, Map context) // If still null after attempting resolution (or definitionsService was null), return false gracefully if (condition.getConditionType() == null) { LOGGER.warn("Condition type is null for condition typeID={}, returning false gracefully", condition.getConditionTypeId()); + if (tracer != null) { + tracer.endOperation(false, "Condition type not resolved"); + } return false; } } @@ -132,6 +150,9 @@ public boolean eval(Condition condition, Item item, Map context) if (resolved == null) { LOGGER.warn("Could not resolve effective condition for typeID={} on item={} (cycle or max depth), returning false", condition.getConditionTypeId(), item != null ? item.getItemId() : "null"); + if (tracer != null) { + tracer.endOperation(false, "Could not resolve effective condition"); + } return false; } effectiveCondition = resolved; @@ -145,14 +166,18 @@ public boolean eval(Condition condition, Item item, Map context) if (effectiveCondition.getConditionType() == null) { LOGGER.debug("Effective condition type is null for condition typeID={}, returning false gracefully", effectiveCondition.getConditionTypeId()); + if (tracer != null) { + tracer.endOperation(false, "Effective condition type not resolved"); + } return false; } String conditionEvaluatorKey = effectiveCondition.getConditionType().getConditionEvaluator(); if (conditionEvaluatorKey == null) { - // resolveEffectiveCondition() already resolved to the root parent above, - // so a null evaluator key here means the chain genuinely has no evaluator. LOGGER.warn("No evaluator defined for condition type: {}", effectiveCondition.getConditionTypeId()); + if (tracer != null) { + tracer.endOperation(false, "No evaluator defined for condition type"); + } return false; } @@ -161,8 +186,11 @@ public boolean eval(Condition condition, Item item, Map context) final ConditionEvaluatorDispatcher dispatcher = this; try { // Use effective condition for evaluation - Condition contextualCondition = ConditionContextHelper.getContextualCondition(effectiveCondition, context, scriptExecutor); + Condition contextualCondition = ConditionContextHelper.getContextualCondition(effectiveCondition, context, scriptExecutor, definitionsService, tracerService); if (contextualCondition == null) { + if (tracer != null) { + tracer.endOperation(false, "Contextual condition is null"); + } return false; } final Condition finalContextualCondition = contextualCondition; @@ -172,18 +200,30 @@ public Boolean execute(Object... args) throws Exception { return evaluator.eval(finalContextualCondition, item, context, dispatcher); } }.runWithTimer(); + + if (tracer != null) { + tracer.endOperation(result, "Condition evaluation completed"); + } return result; } catch (Exception e) { LOGGER.error("Error executing condition evaluator with key={}", conditionEvaluatorKey, e); + if (tracer != null) { + tracer.endOperation(false, "Error during condition evaluation: " + e.getMessage()); + } return false; } } else { LOGGER.error("Couldn't find evaluator with key={}", conditionEvaluatorKey); + if (tracer != null) { + tracer.endOperation(false, "No evaluator found for condition type: " + conditionEvaluatorKey); + } return false; } } catch (Exception e) { - LOGGER.error("Infrastructure error during condition evaluation for typeID={} on item={}", - condition.getConditionTypeId(), item != null ? item.getItemId() : "null", e); + LOGGER.error("Error during condition evaluation", e); + if (tracer != null) { + tracer.endOperation(false, "Error during condition evaluation: " + e.getMessage()); + } return false; } } diff --git a/plugins/baseplugin/pom.xml b/plugins/baseplugin/pom.xml index 68e36ca865..b31483a3f4 100644 --- a/plugins/baseplugin/pom.xml +++ b/plugins/baseplugin/pom.xml @@ -61,6 +61,12 @@ unomi-scripting provided + + org.apache.unomi + unomi-tracing-api + ${project.version} + provided + javax.servlet diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/AllEventToProfilePropertiesAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/AllEventToProfilePropertiesAction.java index b24bb96744..7289f03983 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/AllEventToProfilePropertiesAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/AllEventToProfilePropertiesAction.java @@ -23,6 +23,8 @@ import org.apache.unomi.api.actions.ActionExecutor; import org.apache.unomi.api.services.EventService; import org.apache.unomi.api.services.ProfileService; +import org.apache.unomi.tracing.api.TracerService; +import org.apache.unomi.tracing.api.RequestTracer; import java.util.HashMap; import java.util.Map; @@ -34,36 +36,76 @@ public class AllEventToProfilePropertiesAction implements ActionExecutor { private ProfileService profileService; + private TracerService tracerService; public void setProfileService(ProfileService profileService) { this.profileService = profileService; } + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + @SuppressWarnings({"unchecked", "rawtypes"}) public int execute(Action action, Event event) { - boolean changed = false; - Map properties = new HashMap(); - if (event.getProperties() != null) { - properties.putAll(event.getProperties()); + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.startOperation("sync-event-properties", + "Synchronizing event properties to profile", action); } - try { - Object targetProperties = BeanUtilsBean.getInstance().getPropertyUtils().getProperty(event.getTarget(), "properties"); - if (targetProperties instanceof Map) { - properties.putAll((Map) targetProperties); + boolean changed = false; + Map properties = new HashMap(); + if (event.getProperties() != null) { + properties.putAll(event.getProperties()); + } + + if (tracer != null) { + tracer.trace("Processing properties", Map.of( + "propertiesCount", properties.size(), + "hasTarget", event.getTarget() != null + )); + } + + try { + Object targetProperties = BeanUtilsBean.getInstance().getPropertyUtils().getProperty(event.getTarget(), "properties"); + if (targetProperties instanceof Map) { + properties.putAll((Map) targetProperties); + } + } catch (Exception e) { + if (tracer != null) { + tracer.trace("Target properties not available", Map.of()); + } + // Ignore + } + + int updatedCount = 0; + for (Map.Entry entry : properties.entrySet()) { + if (event.getProfile().getProperty(entry.getKey()) == null || !event.getProfile().getProperty(entry.getKey()).equals(event.getProperty(entry.getKey()))) { + String propertyMapping = profileService.getPropertyTypeMapping(entry.getKey()); + String propertyName = (propertyMapping != null) ? propertyMapping : entry.getKey(); + event.getProfile().setProperty(propertyName, entry.getValue()); + changed = true; + updatedCount++; + } } + + if (tracer != null) { + tracer.trace("Properties synchronized", Map.of( + "updatedCount", updatedCount, + "isChanged", changed + )); + tracer.endOperation(changed, + changed ? "Properties synchronized successfully" : "No changes needed"); + } + return changed ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; } catch (Exception e) { - // Ignore - } - for (Map.Entry entry : properties.entrySet()) { - if (event.getProfile().getProperty(entry.getKey()) == null || !event.getProfile().getProperty(entry.getKey()).equals(event.getProperty(entry.getKey()))) { - String propertyMapping = profileService.getPropertyTypeMapping(entry.getKey()); - String propertyName = (propertyMapping != null) ? propertyMapping : entry.getKey(); - event.getProfile().setProperty(propertyName, entry.getValue()); - changed = true; + if (tracer != null) { + tracer.endOperation(false, "Error synchronizing properties: " + e.getMessage()); } + throw e; } - return changed ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; } } diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/CopyPropertiesAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/CopyPropertiesAction.java index 343d8997c7..2b7225d7bc 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/CopyPropertiesAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/CopyPropertiesAction.java @@ -26,6 +26,8 @@ import org.apache.unomi.api.services.EventService; import org.apache.unomi.api.services.ProfileService; import org.apache.unomi.persistence.spi.PropertyHelper; +import org.apache.unomi.tracing.api.TracerService; +import org.apache.unomi.tracing.api.RequestTracer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,49 +39,98 @@ public class CopyPropertiesAction implements ActionExecutor { private static final Logger LOGGER = LoggerFactory.getLogger(CopyPropertiesAction.class); private ProfileService profileService; + private TracerService tracerService; public void setProfileService(ProfileService profileService) { this.profileService = profileService; } + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + @SuppressWarnings({ "unchecked", "rawtypes" }) public int execute(Action action, Event event) { - boolean atLeastOnechanged = false; - List mandatoryPropTypeSystemTags = (List) action.getParameterValues().get("mandatoryPropTypeSystemTag"); - String singleValueStrategy = (String) action.getParameterValues().get("singleValueStrategy"); - for (Map.Entry entry : getEventPropsToCopy(action, event).entrySet()) { - String mappedProperty = resolvePropertyName(entry.getKey()); - - // propType Check - PropertyType propertyType = profileService.getPropertyType(mappedProperty); - Object previousValue = event.getProfile().getProperty(mappedProperty); - if (mandatoryPropTypeSystemTags != null && mandatoryPropTypeSystemTags.size() > 0) { - if (propertyType == null || propertyType.getMetadata() == null || propertyType.getMetadata().getSystemTags() == null - || !propertyType.getMetadata().getSystemTags().containsAll(mandatoryPropTypeSystemTags)) { - continue; - } - } - String propertyName = "properties." + mappedProperty; - boolean changed = false; - if (previousValue == null && propertyType == null) { - changed = PropertyHelper.setProperty(event.getProfile(), propertyName, entry.getValue(), "alwaysSet"); - } else { - boolean propertyTypeIsMultiValued = - propertyType != null && propertyType.isMultivalued() != null && propertyType.isMultivalued(); - boolean multipleIsExpected = previousValue != null ? previousValue instanceof List : propertyTypeIsMultiValued; - - if (multipleIsExpected) { - changed = PropertyHelper.setProperty(event.getProfile(), propertyName, entry.getValue(), "addValues"); - } else if (entry.getValue() instanceof List) { - LOGGER.error("Impossible to copy the property of type List to the profile, either a single value already exist on the profile or the property type is declared as a single value property. Enable debug log level for more information"); - LOGGER.debug("cannot copy property {}, because it's a List", mappedProperty); - } else { - changed = PropertyHelper.setProperty(event.getProfile(), propertyName, entry.getValue(), singleValueStrategy); + RequestTracer tracer = tracerService.getCurrentTracer(); + if (!tracer.isEnabled()) { + tracer.setEnabled(true); + } + + tracer.startOperation("copy-properties", "Copying properties from event to profile", new HashMap() {{ + put("action.id", action.getActionTypeId()); + put("event.type", event.getEventType()); + }}); + + try { + boolean atLeastOnechanged = false; + List mandatoryPropTypeSystemTags = (List) action.getParameterValues().get("mandatoryPropTypeSystemTag"); + String singleValueStrategy = (String) action.getParameterValues().get("singleValueStrategy"); + + Map propsToCopy = getEventPropsToCopy(action, event); + tracer.trace("Found properties to copy", new HashMap() {{ + put("properties.count", propsToCopy.size()); + }}); + + for (Map.Entry entry : propsToCopy.entrySet()) { + String mappedProperty = resolvePropertyName(entry.getKey()); + tracer.startOperation("copy-property", "Copying single property", new HashMap() {{ + put("property.name", mappedProperty); + }}); + + try { + // propType Check + PropertyType propertyType = profileService.getPropertyType(mappedProperty); + Object previousValue = event.getProfile().getProperty(mappedProperty); + + if (mandatoryPropTypeSystemTags != null && mandatoryPropTypeSystemTags.size() > 0) { + if (propertyType == null || propertyType.getMetadata() == null || propertyType.getMetadata().getSystemTags() == null + || !propertyType.getMetadata().getSystemTags().containsAll(mandatoryPropTypeSystemTags)) { + tracer.trace("Skipping property due to missing required system tags", null); + continue; + } + } + + String propertyName = "properties." + mappedProperty; + final boolean changed; + if (previousValue == null && propertyType == null) { + changed = PropertyHelper.setProperty(event.getProfile(), propertyName, entry.getValue(), "alwaysSet"); + } else { + boolean propertyTypeIsMultiValued = + propertyType != null && propertyType.isMultivalued() != null && propertyType.isMultivalued(); + boolean multipleIsExpected = previousValue != null ? previousValue instanceof List : propertyTypeIsMultiValued; + + if (multipleIsExpected) { + changed = PropertyHelper.setProperty(event.getProfile(), propertyName, entry.getValue(), "addValues"); + } else if (entry.getValue() instanceof List) { + LOGGER.error("Impossible to copy the property of type List to the profile, either a single value already exist on the profile or the property type is declared as a single value property. Enable debug log level for more information"); + LOGGER.debug("cannot copy property {}, because it's a List", mappedProperty); + tracer.trace("Error: Cannot copy List to single value property", null); + changed = false; + } else { + changed = PropertyHelper.setProperty(event.getProfile(), propertyName, entry.getValue(), singleValueStrategy); + } + } + + tracer.trace("Property copy result", new HashMap() {{ + put("changed", changed); + }}); + atLeastOnechanged = atLeastOnechanged || changed; + } finally { + tracer.endOperation(null, "Completed property copy operation"); } } - atLeastOnechanged = atLeastOnechanged || changed; + + final boolean finalAtLeastOneChanged = atLeastOnechanged; + tracer.trace("Overall copy operation result", new HashMap() {{ + put("profile_updated", finalAtLeastOneChanged); + }}); + return finalAtLeastOneChanged ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; + } catch (Exception e) { + tracer.trace("Error during property copy operation", e); + throw e; + } finally { + tracer.endOperation(null, "Completed all property copy operations"); } - return atLeastOnechanged ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; } private Map getEventPropsToCopy(Action action, Event event) { diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EvaluateProfileAgeAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EvaluateProfileAgeAction.java index b8f60df139..34ab5c552d 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EvaluateProfileAgeAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EvaluateProfileAgeAction.java @@ -21,24 +21,90 @@ import org.apache.unomi.api.actions.Action; import org.apache.unomi.api.actions.ActionExecutor; import org.apache.unomi.api.services.EventService; +import org.apache.unomi.api.services.ProfileService; +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TracerService; import org.joda.time.DateTime; import org.joda.time.Years; +import java.util.Map; + /** * An action that sets the age of a profile based on his birth date */ public class EvaluateProfileAgeAction implements ActionExecutor { + private ProfileService profileService; + private TracerService tracerService; + + public void setProfileService(ProfileService profileService) { + this.profileService = profileService; + } + + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + @Override public int execute(Action action, Event event) { - boolean updated = false; - if (event.getProfile().getProperty("birthDate") != null) { - Integer y = Years.yearsBetween(new DateTime(event.getProfile().getProperty("birthDate")), new DateTime()).getYears(); - if (event.getProfile().getProperty("age") == null || event.getProfile().getProperty("age") != y) { - updated = true; - event.getProfile().setProperty("age", y); + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.startOperation("evaluate-age", + "Evaluating profile age", action); + } + + try { + if (event.getProfile() == null) { + if (tracer != null) { + tracer.endOperation(false, "No profile in event"); + } + return EventService.NO_CHANGE; + } + + boolean updated = false; + if (event.getProfile().getProperty("birthDate") != null) { + Integer y = Years.yearsBetween(new DateTime(event.getProfile().getProperty("birthDate")), new DateTime()).getYears(); + Integer currentAge = (Integer) event.getProfile().getProperty("age"); + if (currentAge == null || !currentAge.equals(y)) { + updated = true; + event.getProfile().setProperty("age", y); + if (tracer != null) { + tracer.trace("Age updated", Map.of( + "birthDate", event.getProfile().getProperty("birthDate"), + "newAge", y + )); + } + } + } else { + if (tracer != null) { + tracer.trace("No birth date found", Map.of()); + } + } + + // If this action was triggered by a profileUpdated event, save the profile + // but don't return PROFILE_UPDATED to prevent loops + if (updated && "profileUpdated".equals(event.getEventType()) && profileService != null) { + profileService.save(event.getProfile()); + if (tracer != null) { + tracer.trace("Profile saved after age evaluation (preventing loop)", Map.of( + "newAge", event.getProfile().getProperty("age") + )); + tracer.endOperation(true, "Profile age updated and saved (loop prevented)"); + } + return EventService.NO_CHANGE; + } + + if (tracer != null) { + tracer.endOperation(updated, + updated ? "Profile age updated" : "No changes needed"); + } + return updated ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; + } catch (Exception e) { + if (tracer != null) { + tracer.endOperation(false, "Error evaluating age: " + e.getMessage()); } + throw e; } - return updated ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; } } diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EvaluateProfileSegmentsAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EvaluateProfileSegmentsAction.java index 2002eff517..b4f66ab194 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EvaluateProfileSegmentsAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EvaluateProfileSegmentsAction.java @@ -22,7 +22,10 @@ import org.apache.unomi.api.actions.ActionExecutor; import org.apache.unomi.api.segments.SegmentsAndScores; import org.apache.unomi.api.services.EventService; +import org.apache.unomi.api.services.ProfileService; import org.apache.unomi.api.services.SegmentService; +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TracerService; import java.util.Map; import java.util.Set; @@ -30,6 +33,8 @@ public class EvaluateProfileSegmentsAction implements ActionExecutor { private SegmentService segmentService; + private ProfileService profileService; + private TracerService tracerService; public SegmentService getSegmentService() { return segmentService; @@ -39,23 +44,80 @@ public void setSegmentService(SegmentService segmentService) { this.segmentService = segmentService; } + public void setProfileService(ProfileService profileService) { + this.profileService = profileService; + } + + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + @Override public int execute(Action action, Event event) { - if (event.getProfile().isAnonymousProfile()) { - return EventService.NO_CHANGE; - } - boolean updated = false; - SegmentsAndScores segmentsAndScoringForProfile = segmentService.getSegmentsAndScoresForProfile(event.getProfile()); - Set segments = segmentsAndScoringForProfile.getSegments(); - if (!segments.equals(event.getProfile().getSegments())) { - event.getProfile().setSegments(segments); - updated = true; + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.startOperation("evaluate-segments", + "Evaluating profile segments", action); } - Map scores = segmentsAndScoringForProfile.getScores(); - if (!scores.equals(event.getProfile().getScores())) { - event.getProfile().setScores(scores); - updated = true; + + try { + if (event.getProfile() == null) { + if (tracer != null) { + tracer.endOperation(false, "No profile in event"); + } + return EventService.NO_CHANGE; + } + + if (event.getProfile().isAnonymousProfile()) { + if (tracer != null) { + tracer.endOperation(false, "Skipping anonymous profile"); + } + return EventService.NO_CHANGE; + } + + boolean updated = false; + SegmentsAndScores segmentsAndScoringForProfile = segmentService.getSegmentsAndScoresForProfile(event.getProfile()); + Set segments = segmentsAndScoringForProfile.getSegments(); + if (!segments.equals(event.getProfile().getSegments())) { + event.getProfile().setSegments(segments); + updated = true; + } + Map scores = segmentsAndScoringForProfile.getScores(); + if (!scores.equals(event.getProfile().getScores())) { + event.getProfile().setScores(scores); + updated = true; + } + + // If this action was triggered by a profileUpdated event, save the profile + // but don't return PROFILE_UPDATED to prevent loops + if (updated && "profileUpdated".equals(event.getEventType()) && profileService != null) { + profileService.save(event.getProfile()); + if (tracer != null) { + tracer.trace("Profile saved after segment evaluation (preventing loop)", Map.of( + "segmentsCount", segments.size(), + "scoresCount", scores.size() + )); + tracer.endOperation(true, "Profile segments updated and saved (loop prevented)"); + } + return EventService.NO_CHANGE; + } + + if (tracer != null) { + tracer.trace("Segments evaluated", Map.of( + "segmentsCount", segments.size(), + "scoresCount", scores.size(), + "isUpdated", updated + )); + tracer.endOperation(updated, + updated ? "Profile segments updated" : "No changes needed"); + } + return updated ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; + } catch (Exception e) { + if (tracer != null) { + tracer.endOperation(false, "Error evaluating segments: " + e.getMessage()); + } + throw e; } - return updated ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; } } diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EvaluateVisitPropertiesAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EvaluateVisitPropertiesAction.java index 43f81cd699..697142a5af 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EvaluateVisitPropertiesAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EvaluateVisitPropertiesAction.java @@ -23,6 +23,8 @@ import org.apache.unomi.api.actions.ActionExecutor; import org.apache.unomi.api.services.EventService; import org.apache.unomi.persistence.spi.PropertyHelper; +import org.apache.unomi.tracing.api.TracerService; +import org.apache.unomi.tracing.api.RequestTracer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,6 +33,7 @@ import java.text.SimpleDateFormat; import java.util.Date; import java.util.TimeZone; +import java.util.HashMap; /** * This action is used to calculate the firstVisit, lastVisit and previousVisit date properties on the profile @@ -38,40 +41,89 @@ */ public class EvaluateVisitPropertiesAction implements ActionExecutor { private static final Logger LOGGER = LoggerFactory.getLogger(EvaluateVisitPropertiesAction.class.getName()); + private TracerService tracerService; + + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } public int execute(Action action, Event event) { - SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'"); - dateFormat.setTimeZone(TimeZone.getTimeZone("UTC")); + RequestTracer tracer = tracerService.getCurrentTracer(); + if (!tracer.isEnabled()) { + tracer.setEnabled(true); + } - Date currentEventTimeStamp = event.getTimeStamp(); - Date currentProfileFirstVisit = extractDateFromProperty(event.getProfile(), "firstVisit", dateFormat); - Date currentProfilePreviousVisit = extractDateFromProperty(event.getProfile(), "previousVisit", dateFormat); - Date currentProfileLastVisit = extractDateFromProperty(event.getProfile(), "lastVisit", dateFormat); + tracer.startOperation("evaluate-visit-properties", "Evaluating visit properties", new HashMap() {{ + put("action.type", action.getActionTypeId()); + put("event.type", event.getEventType()); + put("event.timestamp", event.getTimeStamp()); + }}); - int result = EventService.NO_CHANGE; + try { + SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'"); + dateFormat.setTimeZone(TimeZone.getTimeZone("UTC")); - if (currentProfileFirstVisit == null || currentProfileFirstVisit.after(currentEventTimeStamp)) { - // event < firstVisit < previousVisit < lastVisit. we need to update firstVisit - result = PropertyHelper.setProperty(event.getProfile(), "properties.firstVisit", dateFormat.format(currentEventTimeStamp), "alwaysSet") ? - EventService.PROFILE_UPDATED : result; - } + Date currentEventTimeStamp = event.getTimeStamp(); + Date currentProfileFirstVisit = extractDateFromProperty(event.getProfile(), "firstVisit", dateFormat); + Date currentProfilePreviousVisit = extractDateFromProperty(event.getProfile(), "previousVisit", dateFormat); + Date currentProfileLastVisit = extractDateFromProperty(event.getProfile(), "lastVisit", dateFormat); + + tracer.trace("Current visit properties", new HashMap() {{ + put("first.visit", currentProfileFirstVisit); + put("previous.visit", currentProfilePreviousVisit); + put("last.visit", currentProfileLastVisit); + }}); - if (currentProfileLastVisit == null || currentProfileLastVisit.before(currentEventTimeStamp)) { - // firstVisit < previousVisit < lastVisit < event. we need to update lastVisit and previousVisit - if (PropertyHelper.setProperty(event.getProfile(), "properties.lastVisit", dateFormat.format(currentEventTimeStamp), "alwaysSet")) { - result = EventService.PROFILE_UPDATED; + final int[] result = {EventService.NO_CHANGE}; - if (currentProfileLastVisit != null) { - PropertyHelper.setProperty(event.getProfile(), "properties.previousVisit", dateFormat.format(currentProfileLastVisit), "alwaysSet"); + if (currentProfileFirstVisit == null || currentProfileFirstVisit.after(currentEventTimeStamp)) { + // event < firstVisit < previousVisit < lastVisit. we need to update firstVisit + boolean updated = PropertyHelper.setProperty(event.getProfile(), "properties.firstVisit", dateFormat.format(currentEventTimeStamp), "alwaysSet"); + tracer.trace("First visit update", new HashMap() {{ + put("updated", updated); + put("new.value", currentEventTimeStamp); + }}); + result[0] = updated ? EventService.PROFILE_UPDATED : result[0]; + } + + if (currentProfileLastVisit == null || currentProfileLastVisit.before(currentEventTimeStamp)) { + // firstVisit < previousVisit < lastVisit < event. we need to update lastVisit and previousVisit + boolean updated = PropertyHelper.setProperty(event.getProfile(), "properties.lastVisit", dateFormat.format(currentEventTimeStamp), "alwaysSet"); + tracer.trace("Last visit update", new HashMap() {{ + put("updated", updated); + put("new.value", currentEventTimeStamp); + }}); + if (updated) { + result[0] = EventService.PROFILE_UPDATED; + + if (currentProfileLastVisit != null) { + boolean prevUpdated = PropertyHelper.setProperty(event.getProfile(), "properties.previousVisit", dateFormat.format(currentProfileLastVisit), "alwaysSet"); + tracer.trace("Previous visit update", new HashMap() {{ + put("updated", prevUpdated); + put("new.value", currentProfileLastVisit); + }}); + } } + } else if (currentProfilePreviousVisit != null && currentProfilePreviousVisit.before(currentEventTimeStamp)) { + // firstVisit < previousVisit < event < lastVisit. we need to update previousVisit + boolean updated = PropertyHelper.setProperty(event.getProfile(), "properties.previousVisit", dateFormat.format(currentEventTimeStamp), "alwaysSet"); + tracer.trace("Previous visit update", new HashMap() {{ + put("updated", updated); + put("new.value", currentEventTimeStamp); + }}); + result[0] = updated ? EventService.PROFILE_UPDATED : result[0]; } - } else if (currentProfilePreviousVisit != null && currentProfilePreviousVisit.before(currentEventTimeStamp)) { - // firstVisit < previousVisit < event < lastVisit. we need to update previousVisit - result = PropertyHelper.setProperty(event.getProfile(), "properties.previousVisit", dateFormat.format(currentEventTimeStamp), "alwaysSet") ? - EventService.PROFILE_UPDATED : result; - } - return result; + tracer.trace("Operation result", new HashMap() {{ + put("profile.updated", result[0] == EventService.PROFILE_UPDATED); + }}); + return result[0]; + } catch (Exception e) { + tracer.trace("Error during visit properties evaluation", e); + throw e; + } finally { + tracer.endOperation(null, "Completed visit properties evaluation"); + } } private Date extractDateFromProperty(Profile profile, String propertyName, DateFormat dateFormat) { diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EventToProfilePropertyAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EventToProfilePropertyAction.java index 32f8a9b044..2920e7ece8 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EventToProfilePropertyAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EventToProfilePropertyAction.java @@ -21,20 +21,63 @@ import org.apache.unomi.api.actions.Action; import org.apache.unomi.api.actions.ActionExecutor; import org.apache.unomi.api.services.EventService; +import org.apache.unomi.tracing.api.TracerService; +import org.apache.unomi.tracing.api.RequestTracer; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.HashMap; /** * A action to copy an event property to a profile property */ public class EventToProfilePropertyAction implements ActionExecutor { + private static final Logger LOGGER = LoggerFactory.getLogger(EventToProfilePropertyAction.class); + private TracerService tracerService; + + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } public int execute(Action action, Event event) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (!tracer.isEnabled()) { + tracer.setEnabled(true); + } + String eventPropertyName = (String) action.getParameterValues().get("eventPropertyName"); String profilePropertyName = (String) action.getParameterValues().get("profilePropertyName"); - if (event.getProfile().getProperty(profilePropertyName) == null || !event.getProfile().getProperty(profilePropertyName).equals(event.getProperty(eventPropertyName))) { - event.getProfile().setProperty(profilePropertyName, event.getProperty(eventPropertyName)); - return EventService.PROFILE_UPDATED; + tracer.startOperation("event-to-profile-property", "Copying event property to profile property", new HashMap() {{ + put("action.type", action.getActionTypeId()); + put("event.type", event.getEventType()); + put("event.property", eventPropertyName); + put("profile.property", profilePropertyName); + }}); + + try { + Object currentProfileValue = event.getProfile().getProperty(profilePropertyName); + Object eventValue = event.getProperty(eventPropertyName); + boolean needsUpdate = currentProfileValue == null || !currentProfileValue.equals(eventValue); + + tracer.trace("Property values", new HashMap() {{ + put("current.profile.value", currentProfileValue); + put("event.value", eventValue); + put("needs.update", needsUpdate); + }}); + + if (needsUpdate) { + event.getProfile().setProperty(profilePropertyName, eventValue); + tracer.trace("Property updated", null); + return EventService.PROFILE_UPDATED; + } + tracer.trace("No update needed", null); + return EventService.NO_CHANGE; + } catch (Exception e) { + tracer.trace("Error during property copy", e); + throw e; + } finally { + tracer.endOperation(null, "Completed event to profile property copy"); } - return EventService.NO_CHANGE; } } diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java index 275d36c288..273f3efdba 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/IncrementPropertyAction.java @@ -25,6 +25,8 @@ import org.apache.unomi.persistence.spi.PropertyHelper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.unomi.tracing.api.TracerService; +import org.apache.unomi.tracing.api.RequestTracer; import java.lang.reflect.InvocationTargetException; import java.util.HashMap; @@ -32,33 +34,77 @@ public class IncrementPropertyAction implements ActionExecutor { private static final Logger LOGGER = LoggerFactory.getLogger(IncrementPropertyAction.class.getName()); + private TracerService tracerService; + + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } @Override public int execute(final Action action, final Event event) { - boolean storeInSession = Boolean.TRUE.equals(action.getParameterValues().get("storeInSession")); - if (storeInSession && event.getSession() == null) { - return EventService.NO_CHANGE; + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.startOperation("increment-property", + "Incrementing property", action); } - String propertyName = (String) action.getParameterValues().get("propertyName"); - Profile profile = event.getProfile(); - Session session = event.getSession(); - try { - Map properties = storeInSession ? session.getProperties() : profile.getProperties(); - Object propertyValue = getPropertyValue(action, event, propertyName, properties); - if (PropertyHelper.setProperty(properties, propertyName, propertyValue, "alwaysSet")) { - return storeInSession ? EventService.SESSION_UPDATED : EventService.PROFILE_UPDATED; + boolean storeInSession = Boolean.TRUE.equals(action.getParameterValues().get("storeInSession")); + if (storeInSession && event.getSession() == null) { + if (tracer != null) { + tracer.endOperation(false, "No session available for storing property"); + } + return EventService.NO_CHANGE; + } + + String propertyName = (String) action.getParameterValues().get("propertyName"); + Profile profile = event.getProfile(); + Session session = event.getSession(); + + if (tracer != null) { + Map traceData = new HashMap<>(); + traceData.put("propertyName", propertyName); + traceData.put("storeInSession", storeInSession); + traceData.put("hasTarget", event.getTarget() != null); + tracer.trace("Processing property increment", traceData); } - } catch (IllegalAccessException | NoSuchMethodException | InvocationTargetException e) { - LOGGER.warn("Error resolving nested property of object. See debug log level for more information"); - LOGGER.debug("Error resolving nested property of item: {}", storeInSession ? session : profile, e); - } catch (IllegalStateException ee) { - LOGGER.warn("Error increment existing property, because existing property doesn't have expected type. See debug log level for more information"); - LOGGER.debug("{}", ee.getMessage(), ee); - } - return EventService.NO_CHANGE; + try { + Map properties = storeInSession ? session.getProperties() : profile.getProperties(); + Object propertyValue = getPropertyValue(action, event, propertyName, properties); + boolean updated = PropertyHelper.setProperty(properties, propertyName, propertyValue, "alwaysSet"); + + if (tracer != null) { + Map traceData = new HashMap<>(); + traceData.put("newValue", propertyValue); + traceData.put("isUpdated", updated); + tracer.trace("Property increment result", traceData); + tracer.endOperation(updated, + updated ? "Property incremented successfully" : "No changes needed"); + } + return updated ? (storeInSession ? EventService.SESSION_UPDATED : EventService.PROFILE_UPDATED) : EventService.NO_CHANGE; + } catch (IllegalAccessException | NoSuchMethodException | InvocationTargetException e) { + if (tracer != null) { + tracer.endOperation(false, "Error resolving nested property: " + e.getMessage()); + } + LOGGER.warn("Error resolving nested property of object. See debug log level for more information"); + LOGGER.debug("Error resolving nested property of item: {}", storeInSession ? session : profile, e); + } catch (IllegalStateException ee) { + if (tracer != null) { + tracer.endOperation(false, "Error incrementing property: " + ee.getMessage()); + } + LOGGER.warn("Error increment existing property, because existing property doesn't have expected type. See debug log level for more information"); + LOGGER.debug("{}", ee.getMessage(), ee); + } + + return EventService.NO_CHANGE; + } catch (Exception e) { + if (tracer != null) { + tracer.endOperation(false, "Error in property increment: " + e.getMessage()); + } + throw e; + } } private Object getPropertyValue(Action action, Event event, String propertyName, Map properties) diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/MergeProfilesOnPropertyAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/MergeProfilesOnPropertyAction.java index 0079008fe3..83db5fee2a 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/MergeProfilesOnPropertyAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/MergeProfilesOnPropertyAction.java @@ -32,6 +32,8 @@ import org.slf4j.LoggerFactory; import org.apache.unomi.api.tasks.ScheduledTask; import org.apache.unomi.api.tasks.TaskExecutor; +import org.apache.unomi.tracing.api.TracerService; +import org.apache.unomi.tracing.api.RequestTracer; import org.apache.unomi.api.services.ExecutionContextManager; import java.util.*; @@ -47,12 +49,19 @@ public class MergeProfilesOnPropertyAction implements ActionExecutor { private DefinitionsService definitionsService; private PrivacyService privacyService; private SchedulerService schedulerService; + private TracerService tracerService; private ExecutionContextManager executionContextManager; private SecurityService securityService; // TODO we can remove this limit after dealing with: UNOMI-776 (50 is completely arbitrary and it's used to bypass the auto-scroll done by the persistence Service) private int maxProfilesInOneMerge = 50; public int execute(Action action, Event event) { + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.startOperation("merge-profiles", + "Starting profile merge operation", action); + } try { Profile eventProfile = event.getProfile(); @@ -65,14 +74,33 @@ public int execute(Action action, Event event) { if (eventProfile instanceof Persona || eventProfile.isAnonymousProfile() || StringUtils.isEmpty(mergePropName) || StringUtils.isEmpty(mergePropValue)) { + if (tracer != null) { + tracer.endOperation(false, "Invalid profile or missing merge parameters"); + } return EventService.NO_CHANGE; } final List profilesToBeMerge = getProfilesToBeMerge(mergePropName, mergePropValue); + if (tracer != null) { + tracer.trace("Found profiles to merge", Map.of( + "count", profilesToBeMerge.size(), + "profileIds", profilesToBeMerge.stream().map(Profile::getItemId).collect(Collectors.toList()) + )); + } + // Check if the user switched to another profile if (StringUtils.isNotEmpty(currentProfileMergeValue) && !currentProfileMergeValue.equals(mergePropValue)) { + if (tracer != null) { + tracer.trace("Profile switch detected", Map.of( + "fromValue", currentProfileMergeValue, + "toValue", mergePropValue + )); + } reassignCurrentBrowsingData(event, profilesToBeMerge, forceEventProfileAsMaster, mergePropName, mergePropValue); + if (tracer != null) { + tracer.endOperation(true, "Profile switch completed"); + } return EventService.PROFILE_UPDATED + EventService.SESSION_UPDATED; } @@ -85,6 +113,9 @@ public int execute(Action action, Event event) { // If not profiles to merge we are done here. if (profilesToBeMerge.isEmpty()) { + if (tracer != null) { + tracer.endOperation(profileUpdated, profileUpdated ? "Profile updated but no merges needed" : "No changes needed"); + } return profileUpdated ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; } @@ -97,8 +128,18 @@ public int execute(Action action, Event event) { final Profile masterProfile = profileService.mergeProfiles(forceEventProfileAsMaster ? eventProfile : profilesToBeMerge.get(0), profilesToBeMerge); final String masterProfileId = masterProfile.getItemId(); + if (tracer != null) { + tracer.trace("Profile merge completed", Map.of( + "masterProfileId", masterProfileId, + "originalProfileId", eventProfileId + )); + } + // Profile is still using the same profileId after being merged, no need to rewrite exists data, merge is done if (!forceEventProfileAsMaster && masterProfileId.equals(eventProfileId)) { + if (tracer != null) { + tracer.endOperation(profileUpdated, "Profile merge completed with same ID"); + } return profileUpdated ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; } @@ -118,6 +159,7 @@ public int execute(Action action, Event event) { event.setProfileId(anonymousBrowsing ? null : masterProfileId); event.setProfile(masterProfile); + final RequestTracer finalTracer = tracer; event.getActionPostExecutors().add(() -> { try { // This is the list of profile Ids to be updated in browsing data (events/sessions) @@ -146,15 +188,30 @@ public int execute(Action action, Event event) { } } + if (finalTracer != null) { + finalTracer.trace("Post-merge cleanup completed", Map.of( + "mergedProfileIds", mergedProfileIds, + "masterProfileId", masterProfileId + )); + } } catch (Exception e) { LOGGER.error("unable to execute callback action, profile and session will not be saved", e); + if (finalTracer != null) { + finalTracer.endOperation(false, "Error during post-execution: " + e.getMessage()); + } return false; } return true; }); + if (tracer != null) { + tracer.endOperation(true, "Profile merge completed successfully"); + } return EventService.PROFILE_UPDATED + EventService.SESSION_UPDATED; } catch (Exception e) { + if (tracer != null) { + tracer.endOperation(false, "Error during profile merge: " + e.getMessage()); + } throw e; } } @@ -292,6 +349,10 @@ public void setMaxProfilesInOneMerge(String maxProfilesInOneMerge) { this.maxProfilesInOneMerge = Integer.parseInt(maxProfilesInOneMerge); } + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + public void bindExecutionContextManager(ExecutionContextManager executionContextManager) { this.executionContextManager = executionContextManager; } diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/ModifyConsentAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/ModifyConsentAction.java index be18678b76..fed4273733 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/ModifyConsentAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/ModifyConsentAction.java @@ -25,6 +25,8 @@ import org.apache.unomi.api.services.EventService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.unomi.tracing.api.TracerService; +import org.apache.unomi.tracing.api.RequestTracer; import java.text.ParseException; import java.util.Map; @@ -36,29 +38,69 @@ public class ModifyConsentAction implements ActionExecutor { private static final Logger LOGGER = LoggerFactory.getLogger(ModifyConsentAction.class.getName()); + private TracerService tracerService; + public static final String CONSENT_PROPERTY_NAME = "consent"; @Override public int execute(Action action, Event event) { - Profile profile = event.getProfile(); - boolean isProfileUpdated = false; + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.startOperation("modify-consent", + "Modifying consent", action); + } - ISO8601DateFormat dateFormat = new ISO8601DateFormat(); - Map consentMap = (Map) event.getProperties().get(CONSENT_PROPERTY_NAME); - if (consentMap != null) { - if (consentMap.containsKey("typeIdentifier") && consentMap.containsKey("status")) { - Consent consent = null; - try { - consent = new Consent(consentMap, dateFormat); - isProfileUpdated = profile.setConsent(consent); - } catch (ParseException e) { - LOGGER.error("Error parsing consent dates (statusDate or revokeDate). See debug log level to have more information"); - LOGGER.debug("Error parsing consent dates (statusDate or revokeDate).", e); + try { + Profile profile = event.getProfile(); + boolean isProfileUpdated = false; + + ISO8601DateFormat dateFormat = new ISO8601DateFormat(); + Map consentMap = (Map) event.getProperties().get(CONSENT_PROPERTY_NAME); + if (consentMap != null) { + if (consentMap.containsKey("typeIdentifier") && consentMap.containsKey("status")) { + Consent consent = null; + try { + consent = new Consent(consentMap, dateFormat); + isProfileUpdated = profile.setConsent(consent); + if (tracer != null) { + tracer.trace("Consent modified", Map.of( + "typeIdentifier", consent.getTypeIdentifier(), + "status", consent.getStatus(), + "isUpdated", isProfileUpdated + )); + } + } catch (ParseException e) { + if (tracer != null) { + tracer.endOperation(false, "Error parsing consent dates: " + e.getMessage()); + } + LOGGER.error("Error parsing consent dates (statusDate or revokeDate). See debug log level to have more information"); + LOGGER.debug("Error parsing consent dates (statusDate or revokeDate).", e); + return EventService.NO_CHANGE; + } + } else { + if (tracer != null) { + tracer.endOperation(false, "Missing required consent properties"); + } + LOGGER.warn("Event properties for modifyConsent is missing typeIdentifier and grant properties. We will ignore this event."); + return EventService.NO_CHANGE; } - } else { - LOGGER.warn("Event properties for modifyConsent is missing typeIdentifier and grant properties. We will ignore this event."); } + + if (tracer != null) { + tracer.endOperation(isProfileUpdated, + isProfileUpdated ? "Consent updated successfully" : "No changes needed"); + } + return isProfileUpdated ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; + } catch (Exception e) { + if (tracer != null) { + tracer.endOperation(false, "Error modifying consent: " + e.getMessage()); + } + throw e; } - return isProfileUpdated ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; + } + + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; } } diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/SendEventAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/SendEventAction.java index 909fb59a52..94effffe11 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/SendEventAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/SendEventAction.java @@ -23,34 +23,72 @@ import org.apache.unomi.api.actions.Action; import org.apache.unomi.api.actions.ActionExecutor; import org.apache.unomi.api.services.EventService; +import org.apache.unomi.tracing.api.TracerService; +import org.apache.unomi.tracing.api.RequestTracer; +import java.util.HashMap; import java.util.Map; public class SendEventAction implements ActionExecutor { private EventService eventService; + private TracerService tracerService; public void setEventService(EventService eventService) { this.eventService = eventService; } + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + @Override public int execute(Action action, Event event) { - String eventType = (String) action.getParameterValues().get("eventType"); - Boolean toBePersisted = (Boolean) action.getParameterValues().get("toBePersisted"); - - @SuppressWarnings("unchecked") - Map eventProperties = (Map) action.getParameterValues().get("eventProperties"); - Item target = (Item) action.getParameterValues().get("eventTarget"); - - Event subEvent = new Event(eventType, event.getSession(), event.getProfile(), event.getScope(), event, target, event.getTimeStamp()); - subEvent.setProfileId(event.getProfileId()); - subEvent.getAttributes().putAll(event.getAttributes()); - subEvent.getProperties().putAll(eventProperties); - if (toBePersisted != null && !toBePersisted) { - subEvent.setPersistent(false); + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.startOperation("send-event", + "Sending event", action); } - return eventService.send(subEvent); + try { + String eventType = (String) action.getParameterValues().get("eventType"); + Boolean toBePersisted = (Boolean) action.getParameterValues().get("toBePersisted"); + + if (tracer != null) { + Map traceData = new HashMap<>(); + traceData.put("eventType", eventType); + traceData.put("toBePersisted", toBePersisted); + traceData.put("hasTarget", action.getParameterValues().get("eventTarget") != null); + tracer.trace("Preparing event", traceData); + } + + @SuppressWarnings("unchecked") + Map eventProperties = (Map) action.getParameterValues().get("eventProperties"); + Item target = (Item) action.getParameterValues().get("eventTarget"); + + Event subEvent = new Event(eventType, event.getSession(), event.getProfile(), event.getScope(), event, target, event.getTimeStamp()); + subEvent.setProfileId(event.getProfileId()); + subEvent.getAttributes().putAll(event.getAttributes()); + subEvent.getProperties().putAll(eventProperties); + if (toBePersisted != null && !toBePersisted) { + subEvent.setPersistent(false); + } + + int result = eventService.send(subEvent); + if (tracer != null) { + Map traceData = new HashMap<>(); + traceData.put("eventId", subEvent.getItemId()); + traceData.put("result", result); + tracer.trace("Event sent", traceData); + tracer.endOperation(true, "Event sent successfully"); + } + return result; + } catch (Exception e) { + if (tracer != null) { + tracer.endOperation(false, "Error sending event: " + e.getMessage()); + } + throw e; + } } } diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/SetEventOccurenceCountAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/SetEventOccurenceCountAction.java index cb5e78f59d..6716ce74d0 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/SetEventOccurenceCountAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/SetEventOccurenceCountAction.java @@ -24,20 +24,19 @@ import org.apache.unomi.api.services.DefinitionsService; import org.apache.unomi.api.services.EventService; import org.apache.unomi.persistence.spi.PersistenceService; -import org.apache.unomi.persistence.spi.PropertyHelper; +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TracerService; +import javax.xml.bind.DatatypeConverter; import java.time.Duration; import java.time.LocalDateTime; import java.time.ZoneId; import java.util.*; -import java.util.stream.Collectors; - -import javax.xml.bind.DatatypeConverter; public class SetEventOccurenceCountAction implements ActionExecutor { private DefinitionsService definitionsService; - private PersistenceService persistenceService; + private TracerService tracerService; public void setDefinitionsService(DefinitionsService definitionsService) { this.definitionsService = definitionsService; @@ -47,85 +46,118 @@ public void setPersistenceService(PersistenceService persistenceService) { this.persistenceService = persistenceService; } + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + @Override public int execute(Action action, Event event) { - final Condition pastEventCondition = (Condition) action.getParameterValues().get("pastEventCondition"); - - Condition andCondition = new Condition(definitionsService.getConditionType("booleanCondition")); - andCondition.setParameter("operator", "and"); - ArrayList conditions = new ArrayList(); - - Condition eventCondition = (Condition) pastEventCondition.getParameter("eventCondition"); - definitionsService.resolveConditionType(eventCondition); - conditions.add(eventCondition); + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.startOperation("set-event-count", + "Setting event occurrence count", action); + } - Condition c = new Condition(definitionsService.getConditionType("eventPropertyCondition")); - c.setParameter("propertyName", "profileId"); - c.setParameter("comparisonOperator", "equals"); - c.setParameter("propertyValue", event.getProfileId()); - conditions.add(c); + try { + final Condition pastEventCondition = (Condition) action.getParameterValues().get("pastEventCondition"); + String generatedPropertyKey = (String) pastEventCondition.getParameter("generatedPropertyKey"); - // may be current event is already persisted and indexed, in that case we filter it from the count to increment it manually at the end - Condition eventIdFilter = new Condition(definitionsService.getConditionType("eventPropertyCondition")); - eventIdFilter.setParameter("propertyName", "itemId"); - eventIdFilter.setParameter("comparisonOperator", "notEquals"); - eventIdFilter.setParameter("propertyValue", event.getItemId()); - conditions.add(eventIdFilter); + if (tracer != null) { + tracer.trace("Processing event count", Map.of( + "propertyKey", generatedPropertyKey, + "eventId", event.getItemId() + )); + } - Integer numberOfDays = (Integer) pastEventCondition.getParameter("numberOfDays"); - String fromDate = (String) pastEventCondition.getParameter("fromDate"); - String toDate = (String) pastEventCondition.getParameter("toDate"); + Condition andCondition = new Condition(definitionsService.getConditionType("booleanCondition")); + andCondition.setParameter("operator", "and"); + ArrayList conditions = new ArrayList(); - if (numberOfDays != null) { - Condition numberOfDaysCondition = new Condition(definitionsService.getConditionType("eventPropertyCondition")); - numberOfDaysCondition.setParameter("propertyName", "timeStamp"); - numberOfDaysCondition.setParameter("comparisonOperator", "greaterThan"); - numberOfDaysCondition.setParameter("propertyValueDateExpr", "now-" + numberOfDays + "d"); - conditions.add(numberOfDaysCondition); - } - if (fromDate != null) { - Condition startDateCondition = new Condition(); - startDateCondition.setConditionType(definitionsService.getConditionType("eventPropertyCondition")); - startDateCondition.setParameter("propertyName", "timeStamp"); - startDateCondition.setParameter("comparisonOperator", "greaterThanOrEqualTo"); - startDateCondition.setParameter("propertyValueDate", fromDate); - conditions.add(startDateCondition); - } - if (toDate != null) { - Condition endDateCondition = new Condition(); - endDateCondition.setConditionType(definitionsService.getConditionType("eventPropertyCondition")); - endDateCondition.setParameter("propertyName", "timeStamp"); - endDateCondition.setParameter("comparisonOperator", "lessThanOrEqualTo"); - endDateCondition.setParameter("propertyValueDate", toDate); - conditions.add(endDateCondition); - } + Condition eventCondition = (Condition) pastEventCondition.getParameter("eventCondition"); + if (eventCondition != null) { + definitionsService.getConditionValidationService().validate(eventCondition); + } + conditions.add(eventCondition); + + Condition c = new Condition(definitionsService.getConditionType("eventPropertyCondition")); + c.setParameter("propertyName", "profileId"); + c.setParameter("comparisonOperator", "equals"); + c.setParameter("propertyValue", event.getProfileId()); + conditions.add(c); + + // may be current event is already persisted and indexed, in that case we filter it from the count to increment it manually at the end + Condition eventIdFilter = new Condition(definitionsService.getConditionType("eventPropertyCondition")); + eventIdFilter.setParameter("propertyName", "itemId"); + eventIdFilter.setParameter("comparisonOperator", "notEquals"); + eventIdFilter.setParameter("propertyValue", event.getItemId()); + conditions.add(eventIdFilter); + + Integer numberOfDays = (Integer) pastEventCondition.getParameter("numberOfDays"); + String fromDate = (String) pastEventCondition.getParameter("fromDate"); + String toDate = (String) pastEventCondition.getParameter("toDate"); + + if (numberOfDays != null) { + Condition numberOfDaysCondition = new Condition(definitionsService.getConditionType("eventPropertyCondition")); + numberOfDaysCondition.setParameter("propertyName", "timeStamp"); + numberOfDaysCondition.setParameter("comparisonOperator", "greaterThan"); + numberOfDaysCondition.setParameter("propertyValueDateExpr", "now-" + numberOfDays + "d"); + conditions.add(numberOfDaysCondition); + } + if (fromDate != null) { + Condition startDateCondition = new Condition(); + startDateCondition.setConditionType(definitionsService.getConditionType("eventPropertyCondition")); + startDateCondition.setParameter("propertyName", "timeStamp"); + startDateCondition.setParameter("comparisonOperator", "greaterThanOrEqualTo"); + startDateCondition.setParameter("propertyValueDate", fromDate); + conditions.add(startDateCondition); + } + if (toDate != null) { + Condition endDateCondition = new Condition(); + endDateCondition.setConditionType(definitionsService.getConditionType("eventPropertyCondition")); + endDateCondition.setParameter("propertyName", "timeStamp"); + endDateCondition.setParameter("comparisonOperator", "lessThanOrEqualTo"); + endDateCondition.setParameter("propertyValueDate", toDate); + conditions.add(endDateCondition); + } - andCondition.setParameter("subConditions", conditions); + andCondition.setParameter("subConditions", conditions); - long count = persistenceService.queryCount(andCondition, Event.ITEM_TYPE); + long count = persistenceService.queryCount(andCondition, Event.ITEM_TYPE); - LocalDateTime fromDateTime = null; - if (fromDate != null) { - Calendar fromDateCalendar = DatatypeConverter.parseDateTime(fromDate); - fromDateTime = LocalDateTime.ofInstant(fromDateCalendar.toInstant(), ZoneId.of("UTC")); - } - LocalDateTime toDateTime = null; - if (toDate != null) { - Calendar toDateCalendar = DatatypeConverter.parseDateTime(toDate); - toDateTime = LocalDateTime.ofInstant(toDateCalendar.toInstant(), ZoneId.of("UTC")); - } + LocalDateTime fromDateTime = null; + if (fromDate != null) { + Calendar fromDateCalendar = DatatypeConverter.parseDateTime(fromDate); + fromDateTime = LocalDateTime.ofInstant(fromDateCalendar.toInstant(), ZoneId.of("UTC")); + } + LocalDateTime toDateTime = null; + if (toDate != null) { + Calendar toDateCalendar = DatatypeConverter.parseDateTime(toDate); + toDateTime = LocalDateTime.ofInstant(toDateCalendar.toInstant(), ZoneId.of("UTC")); + } - LocalDateTime eventTime = LocalDateTime.ofInstant(event.getTimeStamp().toInstant(),ZoneId.of("UTC")); + LocalDateTime eventTime = LocalDateTime.ofInstant(event.getTimeStamp().toInstant(),ZoneId.of("UTC")); - if (inTimeRange(eventTime, numberOfDays, fromDateTime, toDateTime)) { - count++; - } + if (inTimeRange(eventTime, numberOfDays, fromDateTime, toDateTime)) { + count++; + } - if (updatePastEvents(event, (String) pastEventCondition.getParameter("generatedPropertyKey"), count)) { - return EventService.PROFILE_UPDATED; + boolean updated = updatePastEvents(event, generatedPropertyKey, count); + if (tracer != null) { + tracer.trace("Event count updated", Map.of( + "count", count, + "isUpdated", updated + )); + tracer.endOperation(updated, + updated ? "Event count updated successfully" : "No changes needed"); + } + return updated ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; + } catch (Exception e) { + if (tracer != null) { + tracer.endOperation(false, "Error setting event count: " + e.getMessage()); + } + throw e; } - - return EventService.NO_CHANGE; } private boolean updatePastEvents(Event event, String generatedPropertyKey, long count) { diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/SetPropertyAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/SetPropertyAction.java index f67eb158b5..df3749bad1 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/SetPropertyAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/SetPropertyAction.java @@ -24,6 +24,8 @@ import org.apache.unomi.persistence.spi.PropertyHelper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.unomi.tracing.api.TracerService; +import org.apache.unomi.tracing.api.RequestTracer; import java.text.SimpleDateFormat; import java.util.Date; @@ -37,6 +39,7 @@ public class SetPropertyAction implements ActionExecutor { private static final Logger LOGGER = LoggerFactory.getLogger(SetPropertyAction.class.getName()); private EventService eventService; + private TracerService tracerService; // TODO Temporary solution that should be handle by: https://issues.apache.org/jira/browse/UNOMI-630 (Implement a global solution to avoid multiple same log pollution.) private static final AtomicLong nowDeprecatedLogTimestamp = new AtomicLong(); @@ -47,48 +50,81 @@ public void setUseEventToUpdateProfile(boolean useEventToUpdateProfile) { } public int execute(Action action, Event event) { - boolean storeInSession = Boolean.TRUE.equals(action.getParameterValues().get("storeInSession")); - if (storeInSession && event.getSession() == null) { - return EventService.NO_CHANGE; + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.startOperation("set-property", + "Setting property value", action); } - String propertyName = (String) action.getParameterValues().get("setPropertyName"); - Object propertyValue = getPropertyValue(action, event); - - if (storeInSession) { - // in the case of session storage we directly update the session - if (PropertyHelper.setProperty(event.getSession(), propertyName, propertyValue, (String) action.getParameterValues().get("setPropertyStrategy"))) { - return EventService.SESSION_UPDATED; + try { + boolean storeInSession = Boolean.TRUE.equals(action.getParameterValues().get("storeInSession")); + if (storeInSession && event.getSession() == null) { + if (tracer != null) { + tracer.endOperation(false, "No session available for session storage"); + } + return EventService.NO_CHANGE; } - } else { - if (useEventToUpdateProfile) { - // in the case of profile storage we use the update profile properties event instead. - Map propertyToUpdate = new HashMap<>(); - propertyToUpdate.put(propertyName, propertyValue); - Event updateProperties = new Event("updateProperties", event.getSession(), event.getProfile(), event.getScope(), null, null, new Date()); - updateProperties.setPersistent(false); + String propertyName = (String) action.getParameterValues().get("setPropertyName"); + Object propertyValue = getPropertyValue(action, event); - updateProperties.setProperty(UpdatePropertiesAction.PROPS_TO_UPDATE, propertyToUpdate); - int changes = eventService.send(updateProperties); + if (tracer != null) { + Map traceData = new HashMap<>(); + traceData.put("propertyName", propertyName); + traceData.put("propertyValue", propertyValue); + traceData.put("storeInSession", storeInSession); + tracer.trace("Setting property", traceData); + } - if ((changes & EventService.PROFILE_UPDATED) == EventService.PROFILE_UPDATED) { - return EventService.PROFILE_UPDATED; + int result = EventService.NO_CHANGE; + if (storeInSession) { + // in the case of session storage we directly update the session + if (PropertyHelper.setProperty(event.getSession(), propertyName, propertyValue, (String) action.getParameterValues().get("setPropertyStrategy"))) { + result = EventService.SESSION_UPDATED; } } else { - if (PropertyHelper.setProperty(event.getProfile(), propertyName, propertyValue, (String) action.getParameterValues().get("setPropertyStrategy"))) { - return EventService.PROFILE_UPDATED; + if (useEventToUpdateProfile) { + // in the case of profile storage we use the update profile properties event instead. + Map propertyToUpdate = new HashMap<>(); + propertyToUpdate.put(propertyName, propertyValue); + + Event updateProperties = new Event("updateProperties", event.getSession(), event.getProfile(), event.getScope(), null, null, new Date()); + updateProperties.setPersistent(false); + + updateProperties.setProperty(UpdatePropertiesAction.PROPS_TO_UPDATE, propertyToUpdate); + result = eventService.send(updateProperties); + if ((result & EventService.PROFILE_UPDATED) == EventService.PROFILE_UPDATED) { + result = EventService.PROFILE_UPDATED; + } + } else { + if (PropertyHelper.setProperty(event.getProfile(), propertyName, propertyValue, (String) action.getParameterValues().get("setPropertyStrategy"))) { + result = EventService.PROFILE_UPDATED; + } } } - } - return EventService.NO_CHANGE; + if (tracer != null) { + tracer.endOperation(result != EventService.NO_CHANGE, + result != EventService.NO_CHANGE ? "Property set successfully" : "No changes needed"); + } + return result; + } catch (Exception e) { + if (tracer != null) { + tracer.endOperation(false, "Error setting property: " + e.getMessage()); + } + throw e; + } } public void setEventService(EventService eventService) { this.eventService = eventService; } + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + private Object getPropertyValue(Action action, Event event) { Object propertyValue = action.getParameterValues().get("setPropertyValue"); if (propertyValue == null) { diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/UpdatePropertiesAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/UpdatePropertiesAction.java index eb890ad151..af9a2a0912 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/UpdatePropertiesAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/UpdatePropertiesAction.java @@ -29,6 +29,8 @@ import org.apache.unomi.persistence.spi.PropertyHelper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.unomi.tracing.api.TracerService; +import org.apache.unomi.tracing.api.RequestTracer; import java.util.*; @@ -48,67 +50,94 @@ public class UpdatePropertiesAction implements ActionExecutor { private ProfileService profileService; private EventService eventService; + private TracerService tracerService; public int execute(Action action, Event event) { + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.startOperation("update-properties", + "Updating properties", action); + } - Profile target = event.getProfile(); - - String targetId = (String) event.getProperty(TARGET_ID_KEY); - String targetType = (String) event.getProperty(TARGET_TYPE_KEY); - - if (StringUtils.isNotBlank(targetId) && event.getProfile() != null && !targetId.equals(event.getProfile().getItemId())) { - target = TARGET_TYPE_PROFILE.equals(targetType) ? profileService.load(targetId) : profileService.loadPersona(targetId); - if (target == null) { - LOGGER.warn("No profile found with Id : {}. Update skipped.", targetId); - return EventService.NO_CHANGE; + try { + Profile target = event.getProfile(); + String targetId = (String) event.getProperty(TARGET_ID_KEY); + String targetType = (String) event.getProperty(TARGET_TYPE_KEY); + + if (tracer != null) { + Map traceData = new HashMap<>(); + traceData.put("targetId", targetId); + traceData.put("targetType", targetType); + traceData.put("hasTarget", target != null); + tracer.trace("Processing properties update", traceData); } - } - boolean isProfileOrPersonaUpdated = false; + if (StringUtils.isNotBlank(targetId) && event.getProfile() != null && !targetId.equals(event.getProfile().getItemId())) { + target = TARGET_TYPE_PROFILE.equals(targetType) ? profileService.load(targetId) : profileService.loadPersona(targetId); + if (target == null) { + if (tracer != null) { + tracer.endOperation(false, "No profile found with Id: " + targetId); + } + LOGGER.warn("No profile found with Id : {}. Update skipped.", targetId); + return EventService.NO_CHANGE; + } + } - Map propsToAdd = (HashMap) event.getProperties().get(PROPS_TO_ADD); + boolean isProfileOrPersonaUpdated = false; - if (propsToAdd != null) { - isProfileOrPersonaUpdated |= processProperties(target, propsToAdd, "setIfMissing"); - } + Map propsToAdd = (HashMap) event.getProperties().get(PROPS_TO_ADD); - Map propsToUpdate = (HashMap) event.getProperties().get(PROPS_TO_UPDATE); - if (propsToUpdate != null) { - isProfileOrPersonaUpdated |= processProperties(target, propsToUpdate, "alwaysSet"); - } + if (propsToAdd != null) { + isProfileOrPersonaUpdated |= processProperties(target, propsToAdd, "setIfMissing"); + } - Map propsToAddToSet = (HashMap) event.getProperties().get(PROPS_TO_ADD_TO_SET); - if (propsToAddToSet != null) { - isProfileOrPersonaUpdated |= processProperties(target, propsToAddToSet, "addValues"); - } + Map propsToUpdate = (HashMap) event.getProperties().get(PROPS_TO_UPDATE); + if (propsToUpdate != null) { + isProfileOrPersonaUpdated |= processProperties(target, propsToUpdate, "alwaysSet"); + } - List propsToDelete = (List) event.getProperties().get(PROPS_TO_DELETE); - if (propsToDelete != null) { - for (String prop : propsToDelete) { - isProfileOrPersonaUpdated |= PropertyHelper.setProperty(target, prop, null, "remove"); + Map propsToAddToSet = (HashMap) event.getProperties().get(PROPS_TO_ADD_TO_SET); + if (propsToAddToSet != null) { + isProfileOrPersonaUpdated |= processProperties(target, propsToAddToSet, "addValues"); } - } - if (StringUtils.isNotBlank(targetId) && isProfileOrPersonaUpdated && - event.getProfile() != null && !targetId.equals(event.getProfile().getItemId())) { - if (TARGET_TYPE_PROFILE.equals(targetType)) { - profileService.save(target); - Event profileUpdated = new Event("profileUpdated", null, target, null, null, target, new Date()); - profileUpdated.setPersistent(false); - int changes = eventService.send(profileUpdated); - if ((changes & EventService.PROFILE_UPDATED) == EventService.PROFILE_UPDATED) { - profileService.save(target); + List propsToDelete = (List) event.getProperties().get(PROPS_TO_DELETE); + if (propsToDelete != null) { + for (String prop : propsToDelete) { + isProfileOrPersonaUpdated |= PropertyHelper.setProperty(target, prop, null, "remove"); } - } else { - profileService.savePersona((Persona) target); } - return EventService.NO_CHANGE; + if (StringUtils.isNotBlank(targetId) && isProfileOrPersonaUpdated && + event.getProfile() != null && !targetId.equals(event.getProfile().getItemId())) { + if (TARGET_TYPE_PROFILE.equals(targetType)) { + profileService.save(target); + Event profileUpdated = new Event("profileUpdated", null, target, null, null, target, new Date()); + profileUpdated.setPersistent(false); + int changes = eventService.send(profileUpdated); + if ((changes & EventService.PROFILE_UPDATED) == EventService.PROFILE_UPDATED) { + profileService.save(target); + } + } else { + profileService.savePersona((Persona) target); + } - } + return EventService.NO_CHANGE; - return isProfileOrPersonaUpdated ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; + } + if (tracer != null) { + tracer.endOperation(isProfileOrPersonaUpdated, + isProfileOrPersonaUpdated ? "Properties updated successfully" : "No changes needed"); + } + return isProfileOrPersonaUpdated ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; + } catch (Exception e) { + if (tracer != null) { + tracer.endOperation(false, "Error updating properties: " + e.getMessage()); + } + throw e; + } } private boolean processProperties(Profile target, Map propsMap, String strategy) { @@ -141,4 +170,8 @@ public void setEventService(EventService eventService) { this.eventService = eventService; } + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + } diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/BooleanConditionEvaluator.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/BooleanConditionEvaluator.java index 8eb57b1ead..df9fb677e5 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/BooleanConditionEvaluator.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/BooleanConditionEvaluator.java @@ -21,39 +21,75 @@ import org.apache.unomi.api.conditions.Condition; import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluator; import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluatorDispatcher; +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TracerService; import java.util.List; import java.util.Map; -/** - * Evaluator for AND and OR conditions. - */ +/** Evaluator for AND and OR conditions. */ public class BooleanConditionEvaluator implements ConditionEvaluator { + private TracerService tracerService; + + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } @Override public boolean eval(Condition condition, Item item, Map context, ConditionEvaluatorDispatcher dispatcher) { - boolean isAnd = "and".equalsIgnoreCase((String) condition.getParameter("operator")); - Object subConditionsParam = condition.getParameter("subConditions"); - if (subConditionsParam != null && !(subConditionsParam instanceof List)) { - throw new IllegalArgumentException("Parameter 'subConditions' of condition type '" - + condition.getConditionTypeId() + "' must be a List, got: " + subConditionsParam.getClass().getName()); + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.startOperation("boolean", + "Evaluating boolean condition with operator: " + condition.getParameter("operator"), condition); } - @SuppressWarnings("unchecked") - List conditions = (List) subConditionsParam; - if (conditions == null || conditions.isEmpty()) { - return isAnd; - } + try { + boolean isAnd = "and".equalsIgnoreCase((String) condition.getParameter("operator")); + Object subConditionsParam = condition.getParameter("subConditions"); + if (subConditionsParam != null && !(subConditionsParam instanceof List)) { + throw new IllegalArgumentException("Parameter 'subConditions' of condition type '" + + condition.getConditionTypeId() + "' must be a List, got: " + subConditionsParam.getClass().getName()); + } + @SuppressWarnings("unchecked") + List conditions = (List) subConditionsParam; + + if (conditions == null || conditions.isEmpty()) { + if (tracer != null) { + tracer.endOperation(isAnd, "No subconditions found, returning " + isAnd); + } + return isAnd; + } + + if (tracer != null) { + tracer.trace("Using " + (isAnd ? "AND" : "OR") + " operator for " + conditions.size() + " subconditions", condition); + } - for (Condition sub : conditions) { - boolean eval = dispatcher.eval(sub, item, context); - if (!eval && isAnd) { - return false; - } else if (eval && !isAnd) { - return true; + for (Condition sub : conditions) { + boolean eval = dispatcher.eval(sub, item, context); + if (!eval && isAnd) { + if (tracer != null) { + tracer.endOperation(false, "AND condition failed on subcondition"); + } + return false; + } else if (eval && !isAnd) { + if (tracer != null) { + tracer.endOperation(true, "OR condition succeeded on subcondition"); + } + return true; + } + } + + if (tracer != null) { + tracer.endOperation(isAnd, "All subconditions processed, returning " + isAnd); + } + return isAnd; + } catch (Exception e) { + if (tracer != null) { + tracer.endOperation(false, "Error during boolean condition evaluation: " + e.getMessage()); } + throw e; } - return isAnd; } } diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/IdsConditionEvaluator.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/IdsConditionEvaluator.java index 68d008bf46..37d82bb58b 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/IdsConditionEvaluator.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/IdsConditionEvaluator.java @@ -20,22 +20,51 @@ import org.apache.unomi.api.conditions.Condition; import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluator; import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluatorDispatcher; +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TracerService; import java.util.Collection; import java.util.Map; public class IdsConditionEvaluator implements ConditionEvaluator { + private TracerService tracerService; + + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + @Override public boolean eval(Condition condition, Item item, Map context, ConditionEvaluatorDispatcher dispatcher) { - Collection ids = (Collection) condition.getParameter("ids"); - Boolean match = (Boolean) condition.getParameter("match"); - - if (ids == null || ids.isEmpty()) { - return false; + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.startOperation("ids", "Evaluating IDs condition", condition); } - boolean contained = ids.contains(item.getItemId()); - boolean matchValue = match == null || match; - return matchValue == contained; + try { + Collection ids = (Collection) condition.getParameter("ids"); + Boolean match = (Boolean) condition.getParameter("match"); + + if (ids == null || ids.isEmpty()) { + if (tracer != null) { + tracer.endOperation(false, "No IDs found"); + } + return false; + } + + boolean contained = ids.contains(item.getItemId()); + boolean matchValue = match == null || match; + boolean result = matchValue == contained; + + if (tracer != null) { + tracer.endOperation(result, "IDs condition evaluation completed"); + } + return result; + } catch (Exception e) { + if (tracer != null) { + tracer.endOperation(false, "Error during IDs condition evaluation: " + e.getMessage()); + } + throw e; + } } } diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/NotConditionEvaluator.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/NotConditionEvaluator.java index fff7dc799f..a9494252b1 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/NotConditionEvaluator.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/NotConditionEvaluator.java @@ -21,6 +21,8 @@ import org.apache.unomi.api.conditions.Condition; import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluator; import org.apache.unomi.persistence.spi.conditions.evaluator.ConditionEvaluatorDispatcher; +import org.apache.unomi.tracing.api.TracerService; +import org.apache.unomi.tracing.api.RequestTracer; import java.util.Map; @@ -28,13 +30,41 @@ * Evaluator for NOT condition. */ public class NotConditionEvaluator implements ConditionEvaluator { + private TracerService tracerService; + + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } @Override public boolean eval(Condition condition, Item item, Map context, ConditionEvaluatorDispatcher dispatcher) { - Condition subCondition = (Condition) condition.getParameter("subCondition"); - if (subCondition == null) { - return false; + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.startOperation("not", + "Evaluating NOT condition", condition); + } + + try { + Condition subCondition = (Condition) condition.getParameter("subCondition"); + if (subCondition == null) { + if (tracer != null) { + tracer.endOperation(false, "No subcondition found"); + } + return false; + } + + boolean result = !dispatcher.eval(subCondition, item, context); + + if (tracer != null) { + tracer.endOperation(result, "NOT condition evaluation completed"); + } + return result; + } catch (Exception e) { + if (tracer != null) { + tracer.endOperation(false, "Error during NOT condition evaluation: " + e.getMessage()); + } + throw e; } - return !dispatcher.eval(subCondition, item, context); } } diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java index 18d8225a3d..8229fe6c14 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java @@ -32,6 +32,8 @@ import org.apache.unomi.plugins.baseplugin.conditions.accessors.HardcodedPropertyAccessor; import org.apache.unomi.scripting.ExpressionFilterFactory; import org.apache.unomi.scripting.SecureFilteringClassLoader; +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TracerService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -56,6 +58,8 @@ public class PropertyConditionEvaluator implements ConditionEvaluator { private static final HardcodedPropertyAccessorRegistry hardcodedPropertyAccessorRegistry = new HardcodedPropertyAccessorRegistry(); private ExpressionFilterFactory expressionFilterFactory; + private TracerService tracerService; + public void setUsePropertyConditionOptimizations(boolean usePropertyConditionOptimizations) { this.usePropertyConditionOptimizations = usePropertyConditionOptimizations; } @@ -64,6 +68,12 @@ public void setExpressionFilterFactory(ExpressionFilterFactory expressionFilterF this.expressionFilterFactory = expressionFilterFactory; } + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + + public void init() { + } private int compare(Object actualValue, String expectedValue, Object expectedValueDate, Object expectedValueInteger, Object expectedValueDateExpr, Object expectedValueDouble) { if (expectedValue == null && expectedValueDate == null && expectedValueInteger == null && getDate(expectedValueDateExpr) == null && expectedValueDouble == null) { @@ -204,6 +214,14 @@ private Object normalizeScalar(Object value) { @Override public boolean eval(Condition condition, Item item, Map context, ConditionEvaluatorDispatcher dispatcher) { + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.startOperation("property", + "Evaluating property condition", condition); + } + + try { String op = (String) condition.getParameter("comparisonOperator"); String name = (String) condition.getParameter("propertyName"); @@ -239,9 +257,32 @@ public boolean eval(Condition condition, Item item, Map context, actualValue = ConditionContextHelper.foldToASCII((String) actualValue); } + final Object finalActualValue = actualValue; + if (tracer != null) { + tracer.trace("Property value comparison: " + name + " " + op + " " + expectedValue, + new HashMap() {{ + put("actualValue", finalActualValue); + put("expectedValue", expectedValue); + put("expectedValueInteger", expectedValueInteger); + put("expectedValueDouble", expectedValueDouble); + put("expectedValueDate", expectedValueDate); + put("expectedValueDateExpr", expectedValueDateExpr); + }}); + } + boolean result = isMatch(op, actualValue, expectedValue, expectedValueInteger, expectedValueDouble, expectedValueDate, expectedValueDateExpr, condition); + + if (tracer != null) { + tracer.endOperation(result, "Property condition evaluation completed"); + } return result; + } catch (Exception e) { + if (tracer != null) { + tracer.endOperation(false, "Error during property condition evaluation: " + e.getMessage()); + } + throw e; + } } protected boolean isMatch(String op, Object actualValue, String expectedValue, Object expectedValueInteger, Object expectedValueDouble, diff --git a/plugins/baseplugin/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/plugins/baseplugin/src/main/resources/OSGI-INF/blueprint/blueprint.xml index 189131ff28..2b10af39a0 100644 --- a/plugins/baseplugin/src/main/resources/OSGI-INF/blueprint/blueprint.xml +++ b/plugins/baseplugin/src/main/resources/OSGI-INF/blueprint/blueprint.xml @@ -37,6 +37,7 @@ + @@ -44,12 +45,13 @@ - + + @@ -62,25 +64,32 @@ - + + + - + + + - + + + - + + @@ -102,6 +111,7 @@ + @@ -120,6 +130,7 @@ + @@ -129,6 +140,7 @@ + @@ -136,7 +148,9 @@ - + + + @@ -145,6 +159,7 @@ + @@ -153,7 +168,9 @@ - + + + @@ -163,6 +180,7 @@ + @@ -171,6 +189,7 @@ + @@ -180,6 +199,8 @@ + + @@ -187,7 +208,10 @@ - + + + + @@ -197,6 +221,7 @@ + @@ -206,6 +231,7 @@ + @@ -216,6 +242,7 @@ + @@ -229,6 +256,8 @@ - + + + diff --git a/pom.xml b/pom.xml index d8261f819b..0f98cbbfd7 100644 --- a/pom.xml +++ b/pom.xml @@ -410,6 +410,7 @@ persistence-opensearch services-common services + tracing plugins plugins/baseplugin @@ -430,6 +431,7 @@ extensions/privacy-extension extensions/web-tracker tools + web-servlets wab rest manual diff --git a/rest/pom.xml b/rest/pom.xml index 9e67c1f292..5380ef5b06 100644 --- a/rest/pom.xml +++ b/rest/pom.xml @@ -47,6 +47,13 @@ unomi-api provided + + org.apache.unomi + unomi-tracing-api + ${project.version} + provided + + org.apache.unomi unomi-json-schema-services diff --git a/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java b/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java index 0d604271cd..f5b70de9c6 100644 --- a/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java +++ b/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java @@ -29,9 +29,11 @@ import org.apache.unomi.api.services.ProfileService; import org.apache.unomi.api.services.RulesService; import org.apache.unomi.persistence.spi.CustomObjectMapper; +import org.apache.unomi.persistence.spi.conditions.ConditionContextHelper; import org.apache.unomi.rest.exception.InvalidRequestException; import org.apache.unomi.rest.service.RestServiceUtils; import org.apache.unomi.schema.api.SchemaService; +import org.apache.unomi.tracing.api.TracerService; import org.apache.unomi.utils.EventsRequestContext; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Reference; @@ -78,6 +80,8 @@ public class ContextJsonEndpoint { private SchemaService schemaService; @Reference private ProfileService profileService; + @Reference + private TracerService tracerService; @OPTIONS @Path("/context.js") @@ -100,8 +104,9 @@ public Response contextJSAsPost(ContextRequest contextRequest, @QueryParam("timestamp") Long timestampAsLong, @QueryParam("invalidateProfile") boolean invalidateProfile, @QueryParam("invalidateSession") boolean invalidateSession, + @QueryParam("explain") boolean explain, @Context SecurityContext securityContext) throws JsonProcessingException { - return contextJSAsGet(contextRequest, personaId, sessionId, timestampAsLong, invalidateProfile, invalidateSession, securityContext); + return contextJSAsGet(contextRequest, personaId, sessionId, timestampAsLong, invalidateProfile, invalidateSession, explain, securityContext); } @GET @@ -113,9 +118,10 @@ public Response contextJSAsGet(@QueryParam("payload") ContextRequest contextRequ @QueryParam("timestamp") Long timestampAsLong, @QueryParam("invalidateProfile") boolean invalidateProfile, @QueryParam("invalidateSession") boolean invalidateSession, + @QueryParam("explain") boolean explain, @Context SecurityContext securityContext) throws JsonProcessingException { ContextResponse contextResponse = contextJSONAsPost(contextRequest, personaId, sessionId, timestampAsLong, invalidateProfile, - invalidateSession, securityContext); + invalidateSession, explain, securityContext); String contextAsJSONString = CustomObjectMapper.getObjectMapper().writeValueAsString(contextResponse); StringBuilder responseAsString = new StringBuilder(); responseAsString.append("window.digitalData = window.digitalData || {};\n").append("var cxs = ").append(contextAsJSONString) @@ -132,8 +138,9 @@ public ContextResponse contextJSONAsGet(@QueryParam("payload") ContextRequest co @QueryParam("timestamp") Long timestampAsLong, @QueryParam("invalidateProfile") boolean invalidateProfile, @QueryParam("invalidateSession") boolean invalidateSession, + @QueryParam("explain") boolean explain, @Context SecurityContext securityContext) { - return contextJSONAsPost(contextRequest, personaId, sessionId, timestampAsLong, invalidateProfile, invalidateSession, securityContext); + return contextJSONAsPost(contextRequest, personaId, sessionId, timestampAsLong, invalidateProfile, invalidateSession, explain, securityContext); } @POST @@ -145,14 +152,29 @@ public ContextResponse contextJSONAsPost(ContextRequest contextRequest, @QueryParam("timestamp") Long timestampAsLong, @QueryParam("invalidateProfile") boolean invalidateProfile, @QueryParam("invalidateSession") boolean invalidateSession, + @QueryParam("explain") boolean explain, @Context SecurityContext securityContext) { + // Check if tracing is requested and user has required role + if (explain && !(securityContext.isUserInRole(UnomiRoles.ADMINISTRATOR) || + securityContext.isUserInRole(UnomiRoles.TENANT_ADMINISTRATOR))) { + throw new ForbiddenException("Insufficient privileges to access tracing information"); + } + + if (explain) { + tracerService.enableTracing(); + tracerService.getCurrentTracer().startOperation("context-request", "Processing context request", contextRequest); + } + try { // Schema validation ObjectNode paramsAsJson = JsonNodeFactory.instance.objectNode(); paramsAsJson.put("personaId", personaId); paramsAsJson.put("sessionId", sessionId); if (!schemaService.isValid(paramsAsJson.toString(), "https://unomi.apache.org/schemas/json/rest/requestIds/1-0-0")) { + if (explain) { + tracerService.getCurrentTracer().endOperation(false, "Schema validation failed"); + } throw new InvalidRequestException("Invalid parameter", "Invalid received data"); } @@ -191,9 +213,17 @@ public ContextResponse contextJSONAsPost(ContextRequest contextRequest, contextResponse.setSessionId(sessionId); } + // Add tracing information if requested + if (explain) { + contextResponse.setRequestTracing(tracerService.getTraceNode()); + tracerService.getCurrentTracer().endOperation(null, "Context request processed successfully"); + } + return contextResponse; } finally { - // @todo placeholder for tracing integration + if (explain) { + tracerService.disableTracing(); + } } } @@ -356,7 +386,7 @@ private Condition sanitizeCondition(Condition condition) { private Object sanitizeValue(Object value) { if (value instanceof String) { String stringValue = (String) value; - if (stringValue.startsWith("script::") || stringValue.startsWith("parameter::")) { + if (ConditionContextHelper.isParameterReference(value)) { LOGGER.warn("Scripting detected in context request, filtering out. See debug level for more information"); LOGGER.debug("Scripting detected in context request with value {}, filtering out...", value); return null; diff --git a/rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java b/rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java index bd28f4e2b2..a5954ba0bf 100644 --- a/rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java +++ b/rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java @@ -25,6 +25,7 @@ import org.apache.unomi.rest.exception.InvalidRequestException; import org.apache.unomi.rest.models.EventCollectorResponse; import org.apache.unomi.rest.service.RestServiceUtils; +import org.apache.unomi.tracing.api.TracerService; import org.apache.unomi.utils.EventsRequestContext; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Reference; @@ -50,6 +51,9 @@ public class EventsCollectorEndpoint { @Reference private RestServiceUtils restServiceUtils; + @Reference + private TracerService tracerService; + @Context HttpServletRequest request; @Context @@ -65,23 +69,36 @@ public Response options() { @Path("/eventcollector") public EventCollectorResponse collectAsGet(@QueryParam("payload") EventsCollectorRequest eventsCollectorRequest, @QueryParam("timestamp") Long timestampAsString, + @QueryParam("explain") boolean explain, @Context SecurityContext securityContext) { - return doEvent(eventsCollectorRequest, timestampAsString, securityContext); + return doEvent(eventsCollectorRequest, timestampAsString, explain, securityContext); } @POST @Path("/eventcollector") public EventCollectorResponse collectAsPost(EventsCollectorRequest eventsCollectorRequest, @QueryParam("timestamp") Long timestampAsLong, + @QueryParam("explain") boolean explain, @Context SecurityContext securityContext) { - return doEvent(eventsCollectorRequest, timestampAsLong, securityContext); + return doEvent(eventsCollectorRequest, timestampAsLong, explain, securityContext); } - private EventCollectorResponse doEvent(EventsCollectorRequest eventsCollectorRequest, Long timestampAsLong, SecurityContext securityContext) { + private EventCollectorResponse doEvent(EventsCollectorRequest eventsCollectorRequest, Long timestampAsLong, boolean explain, SecurityContext securityContext) { if (eventsCollectorRequest == null) { throw new InvalidRequestException("events collector cannot be empty", "Invalid received data"); } + // Check if tracing is requested and user has required role + if (explain && !(securityContext.isUserInRole(UnomiRoles.ADMINISTRATOR) || + securityContext.isUserInRole(UnomiRoles.TENANT_ADMINISTRATOR))) { + throw new ForbiddenException("Insufficient privileges to access tracing information"); + } + + if (explain) { + tracerService.enableTracing(); + tracerService.getCurrentTracer().startOperation("event-collection", "Processing event collection request", eventsCollectorRequest); + } + try { Date timestamp = new Date(); if (timestampAsLong != null) { @@ -122,9 +139,17 @@ private EventCollectorResponse doEvent(EventsCollectorRequest eventsCollectorReq EventCollectorResponse response = new EventCollectorResponse(eventsRequestContext.getChanges()); + // Add tracing information if requested + if (explain) { + response.setRequestTracing(tracerService.getTraceNode()); + tracerService.getCurrentTracer().endOperation(null, "Event collection request processed successfully"); + } + return response; } finally { - // @todo placeholder for tracing integration + if (explain) { + tracerService.disableTracing(); + } } } } diff --git a/rest/src/main/java/org/apache/unomi/rest/models/EventCollectorResponse.java b/rest/src/main/java/org/apache/unomi/rest/models/EventCollectorResponse.java index bca06297bc..7b8ba7119b 100644 --- a/rest/src/main/java/org/apache/unomi/rest/models/EventCollectorResponse.java +++ b/rest/src/main/java/org/apache/unomi/rest/models/EventCollectorResponse.java @@ -17,12 +17,13 @@ package org.apache.unomi.rest.models; +import org.apache.unomi.tracing.api.TraceNode; import java.io.Serializable; /** * Response model for the event collector endpoint. * This class provides information about the result of event processing, including - * which entities were updated. + * which entities were updated and tracing information. */ public class EventCollectorResponse implements Serializable { /** @@ -41,9 +42,20 @@ public class EventCollectorResponse implements Serializable { */ private int updated; + /** + * Contains tracing information about the request processing. + * This can be used for debugging and monitoring purposes. + */ + private TraceNode requestTracing; + public EventCollectorResponse() { } + /** + * Creates a new EventCollectorResponse with the specified update flags. + * + * @param updated The bitwise combination of EventService flags indicating what was updated + */ public EventCollectorResponse(int updated) { this.updated = updated; } @@ -65,4 +77,22 @@ public void setUpdated(int updated) { public int getUpdated() { return updated; } + + /** + * Gets the tracing information for the request. + * + * @return The TraceNode containing request tracing information + */ + public TraceNode getRequestTracing() { + return requestTracing; + } + + /** + * Sets the tracing information for the request. + * + * @param requestTracing The TraceNode containing request tracing information + */ + public void setRequestTracing(TraceNode requestTracing) { + this.requestTracing = requestTracing; + } } diff --git a/services/pom.xml b/services/pom.xml index 280d8df57a..eb800cbdae 100644 --- a/services/pom.xml +++ b/services/pom.xml @@ -46,6 +46,19 @@ unomi-api provided + + org.apache.unomi + unomi-tracing-api + ${project.version} + provided + + + org.apache.unomi + unomi-tracing-impl + ${project.version} + test + + org.apache.unomi unomi-metrics diff --git a/services/src/main/java/org/apache/unomi/services/actions/impl/ActionExecutorDispatcherImpl.java b/services/src/main/java/org/apache/unomi/services/actions/impl/ActionExecutorDispatcherImpl.java index ec4fa55352..91683c61d0 100644 --- a/services/src/main/java/org/apache/unomi/services/actions/impl/ActionExecutorDispatcherImpl.java +++ b/services/src/main/java/org/apache/unomi/services/actions/impl/ActionExecutorDispatcherImpl.java @@ -23,11 +23,14 @@ import org.apache.unomi.api.actions.ActionExecutor; import org.apache.unomi.api.services.DefinitionsService; import org.apache.unomi.api.services.EventService; +import org.apache.unomi.api.services.TypeResolutionService; import org.apache.unomi.api.utils.ParserHelper; import org.apache.unomi.metrics.MetricAdapter; import org.apache.unomi.metrics.MetricsService; import org.apache.unomi.scripting.ScriptExecutor; import org.apache.unomi.services.actions.ActionExecutorDispatcher; +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TracerService; import org.osgi.framework.BundleContext; import org.osgi.framework.ServiceReference; import org.slf4j.Logger; @@ -46,6 +49,7 @@ public class ActionExecutorDispatcherImpl implements ActionExecutorDispatcher { private final Map actionDispatchers = new ConcurrentHashMap<>(); private BundleContext bundleContext; private ScriptExecutor scriptExecutor; + private TracerService tracerService; private DefinitionsService definitionsService; public void setMetricsService(MetricsService metricsService) { @@ -60,10 +64,22 @@ public void setScriptExecutor(ScriptExecutor scriptExecutor) { this.scriptExecutor = scriptExecutor; } + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + public void setDefinitionsService(DefinitionsService definitionsService) { this.definitionsService = definitionsService; } + /** + * Helper method to get TypeResolutionService from DefinitionsService. + * Returns null if DefinitionsService is not available or doesn't have TypeResolutionService. + */ + private TypeResolutionService getTypeResolutionService() { + return definitionsService != null ? definitionsService.getTypeResolutionService() : null; + } + public ActionExecutorDispatcherImpl() { valueExtractors.putAll(ParserHelper.DEFAULT_VALUE_EXTRACTORS); valueExtractors.put("script", new ParserHelper.ValueExtractor() { @@ -91,43 +107,98 @@ public int execute(Action action, Event event) { if (action == null) { throw new UnsupportedOperationException("Null action passed for event : " + event); } - // Defensively resolve the action type if missing (e.g. deserialized actions only have actionTypeId). - // This matches the behaviour from unomi-3-dev. - if (action.getActionType() == null && definitionsService != null) { - ParserHelper.resolveActionType(definitionsService, action); - } + String actionKey = null; - if (action.getActionType() != null) { - actionKey = action.getActionType().getActionExecutor(); - } - if (actionKey == null) { - LOGGER.warn("Action type or executor is null for actionTypeId={}, action won't execute", action.getActionTypeId()); - return EventService.NO_CHANGE; + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.startOperation("action-execution", + "Executing action: " + action.getActionTypeId(), action); } - int colonPos = actionKey.indexOf(":"); - if (colonPos > 0) { - String actionPrefix = actionKey.substring(0, colonPos); - String actionName = actionKey.substring(colonPos + 1); - ActionDispatcher actionDispatcher = actionDispatchers.get(actionPrefix); - if (actionDispatcher == null) { - LOGGER.warn("Couldn't find any action dispatcher for prefix '{}', action {} won't execute !", actionPrefix, actionKey); + try { + // Resolve action type if needed (actions deserialized from JSON may only have actionTypeId) + if (action.getActionType() == null) { + TypeResolutionService typeResolutionService = getTypeResolutionService(); + if (typeResolutionService != null) { + typeResolutionService.resolveActionType(action); + } else { + LOGGER.debug("TypeResolutionService not available, cannot resolve action type for actionTypeId={}", action.getActionTypeId()); + } + if (action.getActionType() == null) { + LOGGER.warn("Action type is null for actionTypeId={}, action won't execute", action.getActionTypeId()); + if (tracer != null) { + tracer.endOperation(EventService.NO_CHANGE, "Action type not resolved"); + } + return EventService.NO_CHANGE; + } } - return actionDispatcher.execute(action, event, actionName); - } else if (executors.containsKey(actionKey)) { - ActionExecutor actionExecutor = executors.get(actionKey); - try { - return new MetricAdapter(metricsService, this.getClass().getName() + ".action." + actionKey) { - @Override - public Integer execute(Object... args) throws Exception { - return actionExecutor.execute(getContextualAction(action, event), event); + + actionKey = action.getActionType().getActionExecutor(); + if (actionKey == null) { + LOGGER.warn("No action executor configured for actionTypeId={}, action won't execute", action.getActionTypeId()); + if (tracer != null) { + tracer.endOperation(EventService.NO_CHANGE, "No action executor configured for action type"); + } + return EventService.NO_CHANGE; + } + + int colonPos = actionKey.indexOf(":"); + if (colonPos > 0) { + String actionPrefix = actionKey.substring(0, colonPos); + String actionName = actionKey.substring(colonPos + 1); + ActionDispatcher actionDispatcher = actionDispatchers.get(actionPrefix); + if (actionDispatcher == null) { + LOGGER.warn("Couldn't find any action dispatcher for prefix '{}', action {} won't execute !", actionPrefix, actionKey); + if (tracer != null) { + tracer.endOperation(EventService.NO_CHANGE, + "No action dispatcher found for prefix: " + actionPrefix); } - }.runWithTimer(); - } catch (Exception e) { - LOGGER.error("Error executing action with key={}", actionKey, e); + return EventService.NO_CHANGE; + } + int result = actionDispatcher.execute(action, event, actionName); + if (tracer != null) { + tracer.endOperation(result, "Action execution completed"); + } + return result; + } else if (executors.containsKey(actionKey)) { + ActionExecutor actionExecutor = executors.get(actionKey); + try { + int result = new MetricAdapter(metricsService, this.getClass().getName() + ".action." + actionKey) { + @Override + public Integer execute(Object... args) throws Exception { + return actionExecutor.execute(getContextualAction(action, event), event); + } + }.runWithTimer(); + + if (tracer != null) { + tracer.endOperation(result, "Action execution completed"); + } + return result; + } catch (Exception e) { + LOGGER.error("Error executing action with key={}", actionKey, e); + if (tracer != null) { + tracer.endOperation(EventService.NO_CHANGE, + "Error during action execution: " + e.getMessage()); + } + return EventService.NO_CHANGE; + } + } else { + LOGGER.error("Couldn't find executor with key={}", actionKey); + if (tracer != null) { + tracer.endOperation(EventService.NO_CHANGE, + "No executor found for action type: " + actionKey); + } + return EventService.NO_CHANGE; } + } catch (Exception e) { + LOGGER.error("Error during action execution", e); + if (tracer != null) { + tracer.endOperation(EventService.NO_CHANGE, + "Error during action execution: " + e.getMessage()); + } + return EventService.NO_CHANGE; } - return EventService.NO_CHANGE; } public void bindExecutor(ServiceReference actionExecutorServiceReference) { diff --git a/services/src/main/java/org/apache/unomi/services/impl/definitions/DefinitionsServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/definitions/DefinitionsServiceImpl.java index c8584ef278..db2cbd393f 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/definitions/DefinitionsServiceImpl.java +++ b/services/src/main/java/org/apache/unomi/services/impl/definitions/DefinitionsServiceImpl.java @@ -33,6 +33,8 @@ import org.apache.unomi.services.common.cache.AbstractMultiTypeCachingService; import org.apache.unomi.services.impl.TypeResolutionServiceImpl; import org.apache.unomi.services.impl.validation.ConditionValidationServiceImpl; +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TracerService; import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; import org.osgi.framework.SynchronousBundleListener; @@ -68,6 +70,7 @@ public class DefinitionsServiceImpl extends AbstractMultiTypeCachingService impl private static final long TASK_TIMEOUT_MS = 60000; // 1 minute timeout for tasks private ConditionValidationServiceImpl conditionValidationService; + private TracerService tracerService; private EventAdmin eventAdmin; private TypeResolutionServiceImpl typeResolutionService; @@ -87,6 +90,10 @@ public void setCacheService(MultiTypeCacheService cacheService) { super.setCacheService(cacheService); } + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + public void setEventAdmin(EventAdmin eventAdmin) { this.eventAdmin = eventAdmin; } @@ -573,9 +580,25 @@ public boolean resolveConditionType(Condition rootCondition) { // Delegate to TypeResolutionService for resolution boolean resolved = typeResolutionService.resolveConditionType(rootCondition, "condition type " + rootCondition.getConditionTypeId()); if (resolved) { + // Start validation operation in tracer + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + tracer.startOperation("condition-validation", "Validating condition: " + rootCondition.getConditionTypeId(), rootCondition); + } + } + // Validate the condition after resolving its type (validation service will auto-resolve if needed) List validationErrors = conditionValidationService.validate(rootCondition); + // Add validation info to tracer + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + tracer.addValidationInfo(validationErrors, "condition-validation"); + tracer.endOperation(!validationErrors.isEmpty(), String.format("Validation completed with %d errors", validationErrors.size())); + } + } // Separate errors and warnings List errors = validationErrors.stream() diff --git a/services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java index 94da42cda8..3442a59acc 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java +++ b/services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java @@ -25,11 +25,14 @@ import org.apache.unomi.api.services.DefinitionsService; import org.apache.unomi.api.services.EventListenerService; import org.apache.unomi.api.services.EventService; +import org.apache.unomi.api.services.TypeResolutionService; import org.apache.unomi.api.tenants.Tenant; import org.apache.unomi.api.tenants.TenantService; import org.apache.unomi.persistence.spi.PersistenceService; import org.apache.unomi.persistence.spi.aggregate.TermsAggregate; import org.apache.unomi.services.common.security.IPValidationUtils; +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TracerService; import org.osgi.framework.BundleContext; import org.osgi.framework.ServiceReference; import org.slf4j.Logger; @@ -106,6 +109,8 @@ public String toString() { private Set restrictedEventTypeIds = new LinkedHashSet(); + private TracerService tracerService; + public void setPredefinedEventTypeIds(Set predefinedEventTypeIds) { this.predefinedEventTypeIds = predefinedEventTypeIds; } @@ -122,6 +127,14 @@ public void setDefinitionsService(DefinitionsService definitionsService) { this.definitionsService = definitionsService; } + /** + * Helper method to get TypeResolutionService from DefinitionsService. + * Returns null if DefinitionsService is not available or doesn't have TypeResolutionService. + */ + private TypeResolutionService getTypeResolutionService() { + return definitionsService != null ? definitionsService.getTypeResolutionService() : null; + } + public void setTenantService(TenantService tenantService) { this.tenantService = tenantService; } @@ -130,6 +143,10 @@ public void setBundleContext(BundleContext bundleContext) { this.bundleContext = bundleContext; } + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + @Override public boolean isEventAllowedForTenant(Event event, String tenantId, String sourceIP) { if (event == null || tenantId == null) { @@ -191,6 +208,12 @@ private static String recursionChainKey(List eventStack) { } public int send(Event event) { + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + tracer.trace("Sending event: " + event.getEventType(), event.getItemId()); + } + // Get current event stack from ThreadLocal List eventStack = EVENT_STACK.get(); @@ -200,6 +223,9 @@ public int send(Event event) { String chainKey = recursionChainKey(eventStack); if (LOGGED_RECURSION_CHAINS.add(chainKey)) { EventInfo currentEventInfo = new EventInfo(event); + if (tracer != null) { + tracer.trace("Max recursion depth reached for event: " + event.getEventType(), event.getItemId()); + } StringBuilder errorMsg = new StringBuilder("Max recursion depth reached (depth: ").append(eventStack.size() + 1) .append(", max: ").append(MAX_RECURSION_DEPTH + 1) .append("). Current event: ").append(currentEventInfo); @@ -231,11 +257,22 @@ public int send(Event event) { } private int sendInternal(Event event) { + RequestTracer tracer = null; + if (tracerService != null && tracerService.isTracingEnabled()) { + tracer = tracerService.getCurrentTracer(); + } + boolean saveSucceeded = true; if (event.isPersistent()) { try { + if (tracer != null) { + tracer.trace("Saving persistent event: " + event.getEventType(), event.getItemId()); + } saveSucceeded = persistenceService.save(event, null, true); } catch (Throwable t) { + if (tracer != null) { + tracer.trace("Failed to save event: " + event.getEventType() + ", error: " + t.getMessage(), event.getItemId()); + } LOGGER.error("Failed to save event: ", t); return NO_CHANGE; } @@ -248,20 +285,35 @@ private int sendInternal(Event event) { final Session session = event.getSession(); if (event.isPersistent() && session != null) { session.setLastEventDate(event.getTimeStamp()); + if (tracer != null) { + tracer.trace("Updated session last event date", session.getItemId()); + } } if (event.getProfile() != null) { + if (tracer != null) { + tracer.trace("Processing event for profile: " + event.getProfile().getItemId(), event.getItemId()); + } for (EventListenerService eventListenerService : eventListeners) { if (eventListenerService.canHandle(event)) { + if (tracer != null) { + tracer.trace("Event listener service handling event: " + eventListenerService.getClass().getSimpleName(), event.getItemId()); + } changes |= eventListenerService.onEvent(event); } } // At the end of the processing event execute the post executor actions for (ActionPostExecutor actionPostExecutor : event.getActionPostExecutors()) { + if (tracer != null) { + tracer.trace("Executing post action executor", event.getItemId()); + } changes |= actionPostExecutor.execute() ? changes : NO_CHANGE; } if ((changes & PROFILE_UPDATED) == PROFILE_UPDATED) { + if (tracer != null) { + tracer.trace("Profile updated, sending profileUpdated event", event.getItemId()); + } Event profileUpdated = new Event("profileUpdated", session, event.getProfile(), event.getScope(), event.getSource(), event.getProfile(), event.getTimeStamp()); profileUpdated.setPersistent(false); profileUpdated.getAttributes().putAll(event.getAttributes()); @@ -358,6 +410,10 @@ public Set getEventTypeIds() { @Override public PartialList searchEvents(Condition condition, int offset, int size) { + TypeResolutionService typeResolutionService = getTypeResolutionService(); + if (typeResolutionService != null) { + typeResolutionService.resolveConditionType(condition, "event search"); + } // Note: Effective condition resolution happens in the query builder dispatcher or condition evaluator dispatcher // For in-memory persistence, the condition evaluator dispatcher will resolve the effective condition return persistenceService.query(condition, "timeStamp", Event.class, offset, size); @@ -403,6 +459,7 @@ public PartialList search(Query query) { return persistenceService.continueScrollQuery(Event.class, query.getScrollIdentifier(), query.getScrollTimeValidity()); } if (query.getCondition() != null) { + definitionsService.getConditionValidationService().validate(query.getCondition()); if (StringUtils.isNotBlank(query.getText())) { return persistenceService.queryFullText(query.getText(), query.getCondition(), query.getSortby(), Event.class, query.getOffset(), query.getLimit()); } else { diff --git a/services/src/main/java/org/apache/unomi/services/impl/goals/GoalsServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/goals/GoalsServiceImpl.java index 7711fb6b2f..6260aee0b7 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/goals/GoalsServiceImpl.java +++ b/services/src/main/java/org/apache/unomi/services/impl/goals/GoalsServiceImpl.java @@ -31,18 +31,20 @@ import org.apache.unomi.api.query.AggregateQuery; import org.apache.unomi.api.query.Query; import org.apache.unomi.api.rules.Rule; +import org.apache.unomi.api.services.ConditionValidationService.ValidationError; +import org.apache.unomi.api.services.ConditionValidationService.ValidationErrorType; import org.apache.unomi.api.services.DefinitionsService; import org.apache.unomi.api.services.GoalsService; import org.apache.unomi.api.services.RulesService; +import org.apache.unomi.api.services.TypeResolutionService; import org.apache.unomi.api.services.cache.CacheableTypeConfig; -import org.apache.unomi.api.utils.ParserHelper; import org.apache.unomi.persistence.spi.aggregate.*; import org.apache.unomi.services.common.cache.AbstractMultiTypeCachingService; +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TracerService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.IOException; -import java.net.URL; import java.util.*; import java.util.stream.Collectors; @@ -54,6 +56,8 @@ public class GoalsServiceImpl extends AbstractMultiTypeCachingService implements private RulesService rulesService; + private TracerService tracerService; + private long goalRefreshInterval = 5000; // 5 seconds private long campaignRefreshInterval = 5000; // 5 seconds @@ -65,6 +69,18 @@ public void setRulesService(RulesService rulesService) { this.rulesService = rulesService; } + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + + /** + * Helper method to get TypeResolutionService from DefinitionsService. + * Returns null if DefinitionsService is not available or doesn't have TypeResolutionService. + */ + private TypeResolutionService getTypeResolutionService() { + return definitionsService != null ? definitionsService.getTypeResolutionService() : null; + } + public void setGoalRefreshInterval(long goalRefreshInterval) { this.goalRefreshInterval = goalRefreshInterval; } @@ -152,13 +168,7 @@ public Set getGoalMetadatas() { public Set getGoalMetadatas(Query query) { if (query.getCondition() != null) { - ParserHelper.resolveConditionType(definitionsService, query.getCondition(), "goal metadata query"); - if (definitionsService.getConditionValidationService() != null) { - List validationErrors = definitionsService.getConditionValidationService().validate(query.getCondition()); - if (!validationErrors.isEmpty()) { - LOGGER.warn("Goal query condition has {} validation issue(s)", validationErrors.size()); - } - } + definitionsService.getConditionValidationService().validate(query.getCondition()); } Set descriptions = new LinkedHashSet<>(); @@ -188,8 +198,119 @@ public void setGoal(Goal goal) { LOGGER.warn("Trying to save null goal, aborting..."); return; } - ParserHelper.resolveConditionType(definitionsService, goal.getStartEvent(), "goal "+goal.getItemId()+" start event"); - ParserHelper.resolveConditionType(definitionsService, goal.getTargetEvent(), "goal "+goal.getItemId()+" start event"); + boolean isValid = true; + if (goal.getStartEvent() != null) { + // Resolve condition type (skips parameter resolution - happens on-demand in query builders/evaluators) + TypeResolutionService typeResolutionService = getTypeResolutionService(); + boolean startValid = typeResolutionService != null && typeResolutionService.resolveCondition("goals", goal, goal.getStartEvent(), "goal "+goal.getItemId()+" start event"); + isValid = isValid && startValid; + // Start validation operation in tracer for start event + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + tracer.startOperation("goal-start-event-validation", "Validating goal start event: " + goal.getItemId(), goal.getStartEvent()); + } + } + + // Validate condition (skips parameters with references/scripts) + // Validation service will auto-resolve types if needed + List validationErrors = definitionsService.getConditionValidationService().validate(goal.getStartEvent()); + + // Add validation info to tracer + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + tracer.addValidationInfo(validationErrors, "goal-start-event-validation"); + tracer.endOperation(!validationErrors.isEmpty(), String.format("Goal start event validation completed with %d errors", validationErrors.size())); + } + } + + // Separate errors and warnings + List errors = validationErrors.stream() + .filter(error -> error.getType() != ValidationErrorType.MISSING_RECOMMENDED_PARAMETER) + .collect(Collectors.toList()); + + List warnings = validationErrors.stream() + .filter(error -> error.getType() == ValidationErrorType.MISSING_RECOMMENDED_PARAMETER) + .collect(Collectors.toList()); + + // Log warnings but don't block the operation + if (!warnings.isEmpty()) { + StringBuilder warningMessage = new StringBuilder("Goal start event has warnings:"); + for (ValidationError warning : warnings) { + warningMessage.append("\n- ").append(warning.getMessage()); + } + LOGGER.warn(warningMessage.toString()); + } + + // Only throw exception for actual errors + if (!errors.isEmpty()) { + StringBuilder errorMessage = new StringBuilder("Invalid goal start event:"); + for (ValidationError error : errors) { + errorMessage.append("\n- ").append(error.getMessage()); + } + if (typeResolutionService != null) { + typeResolutionService.markInvalid("goals", goal.getItemId(), "Start event validation errors: " + errorMessage.toString()); + } + throw new IllegalArgumentException(errorMessage.toString()); + } + } + if (goal.getTargetEvent() != null) { + // Resolve condition type (skips parameter resolution - happens on-demand in query builders/evaluators) + TypeResolutionService typeResolutionService = getTypeResolutionService(); + boolean targetValid = typeResolutionService != null && typeResolutionService.resolveCondition("goals", goal, goal.getTargetEvent(), "goal "+goal.getItemId()+" target event"); + isValid = isValid && targetValid; + // Start validation operation in tracer for target event + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + tracer.startOperation("goal-target-event-validation", "Validating goal target event: " + goal.getItemId(), goal.getTargetEvent()); + } + } + + // Validate condition (skips parameters with references/scripts) + // Validation service will auto-resolve types if needed + List targetValidationErrors = definitionsService.getConditionValidationService().validate(goal.getTargetEvent()); + + // Add validation info to tracer + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + tracer.addValidationInfo(targetValidationErrors, "goal-target-event-validation"); + tracer.endOperation(!targetValidationErrors.isEmpty(), String.format("Goal target event validation completed with %d errors", targetValidationErrors.size())); + } + } + + // Separate errors and warnings + List targetErrors = targetValidationErrors.stream() + .filter(error -> error.getType() != ValidationErrorType.MISSING_RECOMMENDED_PARAMETER) + .collect(Collectors.toList()); + + List targetWarnings = targetValidationErrors.stream() + .filter(error -> error.getType() == ValidationErrorType.MISSING_RECOMMENDED_PARAMETER) + .collect(Collectors.toList()); + + // Log warnings but don't block the operation + if (!targetWarnings.isEmpty()) { + StringBuilder warningMessage = new StringBuilder("Goal target event has warnings:"); + for (ValidationError warning : targetWarnings) { + warningMessage.append("\n- ").append(warning.getMessage()); + } + LOGGER.warn(warningMessage.toString()); + } + + // Only throw exception for actual errors + if (!targetErrors.isEmpty()) { + StringBuilder errorMessage = new StringBuilder("Invalid goal target event:"); + for (ValidationError error : targetErrors) { + errorMessage.append("\n- ").append(error.getMessage()); + } + if (typeResolutionService != null) { + typeResolutionService.markInvalid("goals", goal.getItemId(), "Target event validation errors: " + errorMessage.toString()); + } + throw new IllegalArgumentException(errorMessage.toString()); + } + } if (goal.getMetadata().isEnabled()) { if (goal.getStartEvent() != null) { @@ -271,13 +392,7 @@ public Set getCampaignMetadatas() { public Set getCampaignMetadatas(Query query) { if (query.getCondition() != null) { - ParserHelper.resolveConditionType(definitionsService, query.getCondition(), "campaign metadata query"); - if (definitionsService.getConditionValidationService() != null) { - List validationErrors = definitionsService.getConditionValidationService().validate(query.getCondition()); - if (!validationErrors.isEmpty()) { - LOGGER.warn("Campaign query condition has {} validation issue(s)", validationErrors.size()); - } - } + definitionsService.getConditionValidationService().validate(query.getCondition()); } Set descriptions = new HashSet(); for (Campaign definition : persistenceService.query(query.getCondition(), query.getSortby(), Campaign.class, query.getOffset(), query.getLimit()).getList()) { @@ -288,13 +403,7 @@ public Set getCampaignMetadatas(Query query) { public PartialList getCampaignDetails(Query query) { if (query.getCondition() != null) { - ParserHelper.resolveConditionType(definitionsService, query.getCondition(), "campaign details query"); - if (definitionsService.getConditionValidationService() != null) { - List validationErrors = definitionsService.getConditionValidationService().validate(query.getCondition()); - if (!validationErrors.isEmpty()) { - LOGGER.warn("Campaign details query condition has {} validation issue(s)", validationErrors.size()); - } - } + definitionsService.getConditionValidationService().validate(query.getCondition()); } PartialList campaigns = persistenceService.query(query.getCondition(), query.getSortby(), Campaign.class, query.getOffset(), query.getLimit()); List details = new LinkedList<>(); @@ -406,12 +515,9 @@ public GoalReport getGoalReport(String goalId, AggregateQuery query) { } if (query != null && query.getCondition() != null) { - ParserHelper.resolveConditionType(definitionsService, query.getCondition(), "goal report query"); - if (definitionsService.getConditionValidationService() != null) { - List validationErrors = definitionsService.getConditionValidationService().validate(query.getCondition()); - if (!validationErrors.isEmpty()) { - LOGGER.warn("Events query condition has {} validation issue(s)", validationErrors.size()); - } + TypeResolutionService typeResolutionService = getTypeResolutionService(); + if (typeResolutionService != null) { + typeResolutionService.resolveConditionType(query.getCondition(), "goal " + goalId + " report"); } list.add(query.getCondition()); } @@ -495,13 +601,7 @@ public PartialList getEvents(Query query) { persistenceService.refreshIndex(CampaignEvent.class); } if (query.getCondition() != null) { - ParserHelper.resolveConditionType(definitionsService, query.getCondition(), "campaign events query"); - if (definitionsService.getConditionValidationService() != null) { - List validationErrors = definitionsService.getConditionValidationService().validate(query.getCondition()); - if (!validationErrors.isEmpty()) { - LOGGER.warn("Campaign events query condition has {} validation issue(s)", validationErrors.size()); - } - } + definitionsService.getConditionValidationService().validate(query.getCondition()); } return persistenceService.query(query.getCondition(), query.getSortby(), CampaignEvent.class, query.getOffset(), query.getLimit()); } @@ -545,14 +645,20 @@ protected Set> getTypeConfigs() { /** - * Hook for campaign type resolution (validation stack not backported on this branch). + * Resolve a campaign's types and track its validity status using the TypeResolutionService. * - * @param campaign the campaign being saved + * @param campaign the campaign to resolve */ private void resolveCampaign(Campaign campaign) { if (campaign == null) { return; } - } + TypeResolutionService typeResolutionService = getTypeResolutionService(); + if (typeResolutionService == null) { + return; + } + typeResolutionService.resolveCondition("campaigns", campaign, campaign.getEntryCondition(), "campaign " + campaign.getItemId()); + } } + diff --git a/services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java index bfe2ad07c5..b321d8248f 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java +++ b/services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java @@ -27,17 +27,18 @@ import org.apache.unomi.api.query.Query; import org.apache.unomi.api.rules.Rule; import org.apache.unomi.api.rules.RuleStatistics; +import org.apache.unomi.api.services.ConditionValidationService.ValidationError; +import org.apache.unomi.api.services.ConditionValidationService.ValidationErrorType; import org.apache.unomi.api.services.*; import org.apache.unomi.api.services.cache.CacheableTypeConfig; import org.apache.unomi.api.tasks.ScheduledTask; import org.apache.unomi.api.tenants.Tenant; -import org.apache.unomi.api.services.ConditionValidationService.ValidationError; -import org.apache.unomi.api.services.ConditionValidationService.ValidationErrorType; -import org.apache.unomi.api.services.TypeResolutionService; import org.apache.unomi.api.utils.ParserHelper; import org.apache.unomi.persistence.spi.config.ConfigurationUpdateHelper; import org.apache.unomi.services.actions.ActionExecutorDispatcher; import org.apache.unomi.services.common.cache.AbstractMultiTypeCachingService; +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TracerService; import org.osgi.framework.*; import org.osgi.service.cm.ManagedService; import org.osgi.service.event.EventHandler; @@ -61,6 +62,7 @@ public class RulesServiceImpl extends AbstractMultiTypeCachingService implements private DefinitionsService definitionsService; private EventService eventService; private ActionExecutorDispatcher actionExecutorDispatcher; + private TracerService tracerService; private Integer rulesRefreshInterval = 1000; private Integer rulesStatisticsRefreshInterval = 10000; @@ -113,9 +115,13 @@ public void setOptimizedRulesActivated(Boolean optimizedRulesActivated) { this.optimizedRulesActivated = optimizedRulesActivated; } + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + /** * Helper method to get TypeResolutionService from DefinitionsService. - * Returns null if DefinitionsService is not available or does not expose TypeResolutionService. + * Returns null if DefinitionsService is not available or doesn't have TypeResolutionService. */ private TypeResolutionService getTypeResolutionService() { return definitionsService != null ? definitionsService.getTypeResolutionService() : null; @@ -677,10 +683,29 @@ protected void setRule(Rule rule, boolean allowInvalidRules) { } } - if (rule.getCondition() != null) { + // Start validation operation in tracer + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + tracer.startOperation("rule-condition-validation", "Validating rule condition: " + rule.getItemId(), rule.getCondition()); + } + } + + // Validate condition (skips parameters with references/scripts) + // Validation service will auto-resolve types if needed List validationErrors = definitionsService.getConditionValidationService().validate(rule.getCondition()); + // Add validation info to tracer + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + tracer.addValidationInfo(validationErrors, "rule-condition-validation"); + tracer.endOperation(!validationErrors.isEmpty(), String.format("Rule validation completed with %d errors", validationErrors.size())); + } + } + + // Separate errors and warnings List errors = validationErrors.stream() .filter(error -> error.getType() != ValidationErrorType.MISSING_RECOMMENDED_PARAMETER) .collect(Collectors.toList()); @@ -689,6 +714,7 @@ protected void setRule(Rule rule, boolean allowInvalidRules) { .filter(error -> error.getType() == ValidationErrorType.MISSING_RECOMMENDED_PARAMETER) .collect(Collectors.toList()); + // Log warnings but don't block the operation if (!warnings.isEmpty()) { StringBuilder warningMessage = new StringBuilder("Rule condition has warnings:"); for (ValidationError warning : warnings) { @@ -697,6 +723,7 @@ protected void setRule(Rule rule, boolean allowInvalidRules) { LOGGER.warn(warningMessage.toString()); } + // Only throw exception for actual errors if (!errors.isEmpty()) { StringBuilder errorMessage = new StringBuilder("Invalid rule condition:"); for (ValidationError error : errors) { @@ -846,6 +873,7 @@ private boolean shouldExcludeRuleFromEventTypeIndex(Rule rule) { return true; } + // Check if rule has missing plugins or is invalid (set by resolveRule) boolean hasMissingPlugins = rule.getMetadata().isMissingPlugins(); TypeResolutionService typeResolutionService = getTypeResolutionService(); boolean isInvalid = typeResolutionService != null && typeResolutionService.isInvalid("rules", rule.getItemId()); @@ -913,7 +941,7 @@ private Set resolveEventTypesWithWarnings(Rule rule) { // Before defaulting to wildcard when eventTypeIds is empty, check if rule has unresolved types // This relies on ensureRuleResolvedForIndexing() having been called, which marks the rule appropriately // We check for unresolved types by looking at the rule's resolution status (missingPlugins or invalid) - // This avoids duplicating the resolution logic - we rely on ensureRuleResolvedForIndexing / ParserHelper + // This avoids duplicating the resolution logic - we rely on TypeResolutionService infrastructure if (eventTypeIds.isEmpty()) { // Check if rule has unresolved types by checking resolution status // Note: shouldExcludeRuleFromEventTypeIndex() also checks disabled, so we need to check specifically @@ -999,13 +1027,17 @@ private boolean ensureRuleResolved(Rule rule) { return false; } + // Check if rule needs resolution (invalid or missing plugins) boolean wasInvalid = typeResolutionService.isInvalid("rules", rule.getItemId()); boolean hadMissingPlugins = rule.getMetadata() != null && rule.getMetadata().isMissingPlugins(); + // If rule is already valid, no need to resolve if (!wasInvalid && !hadMissingPlugins) { return true; } + // Attempt to resolve the rule (conditions + actions) + // This will update missingPlugins flag and invalid status if resolution succeeds return typeResolutionService.resolveRule("rules", rule); } @@ -1027,6 +1059,10 @@ private boolean ensureRuleResolvedForIndexing(Rule rule) { return false; } + // Always attempt to resolve the rule to ensure resolution status is up-to-date + // This is idempotent - resolveRule() efficiently checks if types are already resolved + // and only performs actual resolution if needed. This ensures newly loaded rules + // are properly checked for unresolved types before indexing. return typeResolutionService.resolveRule("rules", rule); } @@ -1043,6 +1079,7 @@ private boolean reEvaluateRuleResolution(Rule rule) { return false; } + // Check if rule was previously invalid or had missing plugins TypeResolutionService typeResolutionService = getTypeResolutionService(); if (typeResolutionService == null) { return false; @@ -1144,6 +1181,7 @@ public Set getTrackedConditions(Item source) { trackedConditions.add(trackedCondition); } } else { + // Resolve condition type if needed before accessing it if (trackedCondition.getConditionType() == null) { TypeResolutionService typeResolutionService = getTypeResolutionService(); if (typeResolutionService != null) { @@ -1230,9 +1268,15 @@ public void handleEvent(org.osgi.service.event.Event event) { List rules = persistenceService.query("tenantId", tId, "priority", Rule.class); for (Rule rule : rules) { + // Only re-evaluate rules that were previously invalid or had missing plugins + TypeResolutionService typeResolutionService = getTypeResolutionService(); + if (typeResolutionService == null) { + continue; + } + boolean wasInvalid = typeResolutionService.isInvalid("rules", rule.getItemId()); boolean hadMissingPlugins = rule.getMetadata() != null && rule.getMetadata().isMissingPlugins(); - if (hadMissingPlugins) { + if (wasInvalid || hadMissingPlugins) { // Re-evaluate this rule boolean resolved = reEvaluateRuleResolution(rule); diff --git a/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java index fd8f400024..be26b3434e 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java +++ b/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java @@ -28,6 +28,8 @@ import org.apache.unomi.api.query.Query; import org.apache.unomi.api.rules.Rule; import org.apache.unomi.api.segments.*; +import org.apache.unomi.api.services.ConditionValidationService.ValidationError; +import org.apache.unomi.api.services.ConditionValidationService.ValidationErrorType; import org.apache.unomi.api.services.*; import org.apache.unomi.api.services.cache.CacheableTypeConfig; import org.apache.unomi.api.tasks.ScheduledTask; @@ -35,18 +37,13 @@ import org.apache.unomi.api.tenants.TenantService; import org.apache.unomi.api.utils.ConditionBuilder; import org.apache.unomi.persistence.spi.CustomObjectMapper; -import org.apache.unomi.persistence.spi.PropertyHelper; import org.apache.unomi.persistence.spi.aggregate.TermsAggregate; import org.apache.unomi.services.common.cache.AbstractMultiTypeCachingService; import org.apache.unomi.services.impl.scheduler.SchedulerServiceImpl; -import org.apache.unomi.api.services.ConditionValidationService.ValidationError; -import org.apache.unomi.api.services.ConditionValidationService.ValidationErrorType; -import org.apache.unomi.api.utils.ParserHelper; -import org.apache.unomi.api.exceptions.BadSegmentConditionException; -import org.osgi.framework.Bundle; +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TracerService; import org.osgi.framework.BundleContext; import org.osgi.framework.BundleEvent; -import org.osgi.framework.SynchronousBundleListener; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -73,6 +70,7 @@ public class SegmentServiceImpl extends AbstractMultiTypeCachingService implemen private EventService eventService; private RulesService rulesService; private DefinitionsService definitionsService; + private TracerService tracerService; private long taskExecutionPeriod = 1; private int segmentUpdateBatchSize = 1000; @@ -102,6 +100,14 @@ public void setDefinitionsService(DefinitionsService definitionsService) { this.definitionsService = definitionsService; } + public void setTracerService(TracerService tracerService) { + this.tracerService = tracerService; + } + + /** + * Helper method to get TypeResolutionService from DefinitionsService. + * Returns null if DefinitionsService is not available or doesn't have TypeResolutionService. + */ private TypeResolutionService getTypeResolutionService() { return definitionsService != null ? definitionsService.getTypeResolutionService() : null; } @@ -368,9 +374,6 @@ public PartialList getSegmentMetadatas(Query query) { public Segment getSegmentDefinition(String segmentId) { String currentTenant = contextManager.getCurrentContext().getTenantId(); Segment segment = cacheService.getWithInheritance(segmentId, currentTenant, Segment.class); - if (segment != null && segment.getMetadata().isEnabled()) { - ParserHelper.resolveConditionType(definitionsService, segment.getCondition(), "segment " + segmentId); - } return segment; } @@ -388,52 +391,71 @@ public void setSegmentDefinition(Segment segment) { throw new BadSegmentConditionException(); } } else { + // Start validation operation in tracer + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + tracer.startOperation("segment-condition-validation", "Validating segment condition: " + segment.getItemId(), segment.getCondition()); + } + } + if (segment.getMetadata().isEnabled()) { TypeResolutionService typeResolutionService = getTypeResolutionService(); if (typeResolutionService != null) { typeResolutionService.resolveCondition("segments", segment, segment.getCondition(), "segment " + segment.getItemId()); } + } - if (!persistenceService.isValidCondition(segment.getCondition(), new Profile(VALIDATION_PROFILE_ID))) { - throw new BadSegmentConditionException(); - } + // Validate condition (skips parameters with references/scripts) + // Validation service will auto-resolve types if needed + List validationErrors = definitionsService.getConditionValidationService().validate(segment.getCondition()); - if (definitionsService.getConditionValidationService() != null) { - List validationErrors = definitionsService.getConditionValidationService().validate(segment.getCondition()); + // Add validation info to tracer + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + tracer.addValidationInfo(validationErrors, "segment-condition-validation"); + tracer.endOperation(!validationErrors.isEmpty(), String.format("Segment validation completed with %d errors", validationErrors.size())); + } + } - List errors = validationErrors.stream() - .filter(error -> error.getType() != ValidationErrorType.MISSING_RECOMMENDED_PARAMETER) - .collect(Collectors.toList()); + // Separate errors and warnings + List errors = validationErrors.stream() + .filter(error -> error.getType() != ValidationErrorType.MISSING_RECOMMENDED_PARAMETER) + .collect(Collectors.toList()); - List warnings = validationErrors.stream() - .filter(error -> error.getType() == ValidationErrorType.MISSING_RECOMMENDED_PARAMETER) - .collect(Collectors.toList()); + List warnings = validationErrors.stream() + .filter(error -> error.getType() == ValidationErrorType.MISSING_RECOMMENDED_PARAMETER) + .collect(Collectors.toList()); - if (!warnings.isEmpty()) { - StringBuilder warningMessage = new StringBuilder("Segment condition has warnings:"); - for (ValidationError warning : warnings) { - warningMessage.append("\n- ").append(warning.getDetailedMessage()); - } - LOGGER.warn(warningMessage.toString()); - } + // Log warnings but don't block the operation + if (!warnings.isEmpty()) { + StringBuilder warningMessage = new StringBuilder("Segment condition has warnings:"); + for (ValidationError warning : warnings) { + warningMessage.append("\n- ").append(warning.getDetailedMessage()); + } + LOGGER.warn(warningMessage.toString()); + } - if (!errors.isEmpty()) { - StringBuilder errorMessage = new StringBuilder("Invalid segment condition:"); - for (ValidationError error : errors) { - errorMessage.append("\n- ").append(error.getDetailedMessage()); - } - throw new BadSegmentConditionException(errorMessage.toString()); - } + // Only throw exception for actual errors + if (!errors.isEmpty()) { + StringBuilder errorMessage = new StringBuilder("Invalid segment condition:"); + for (ValidationError error : errors) { + errorMessage.append("\n- ").append(error.getDetailedMessage()); } + throw new BadSegmentConditionException(errorMessage.toString()); } + } + // Update auto-generated rules if metadata is enabled and no missing plugins if (segment.getMetadata().isEnabled() && !segment.getMetadata().isMissingPlugins()) { updateAutoGeneratedRules(segment.getMetadata(), segment.getCondition()); } segment.setTenantId(contextManager.getCurrentContext().getTenantId()); + // Save segment and update cache persistenceService.save(segment, null, true); cacheService.put(Segment.ITEM_TYPE, segment.getItemId(), segment.getTenantId(), segment); updateExistingProfilesForSegment(segment); @@ -639,16 +661,22 @@ public long getMatchingIndividualsCount(String segmentID) { } public Boolean isProfileInSegment(Profile profile, String segmentId) { + RequestTracer tracer = tracerService.getCurrentTracer(); + tracer.trace("Checking if profile is in segment: " + segmentId, profile.getItemId()); Set matchingSegments = getSegmentsAndScoresForProfile(profile).getSegments(); boolean isInSegment = matchingSegments.contains(segmentId); + tracer.trace("Profile " + profile.getItemId() + " is " + (isInSegment ? "in" : "not in") + " segment: " + segmentId, profile.getItemId()); return isInSegment; } public SegmentsAndScores getSegmentsAndScoresForProfile(Profile profile) { + RequestTracer tracer = tracerService.getCurrentTracer(); + tracer.trace("Getting segments and scores for profile: " + profile.getItemId(), profile.getItemId()); Set segments = new HashSet(); Map scores = new HashMap(); String currentTenant = contextManager.getCurrentContext().getTenantId(); + tracer.trace("Current tenant: " + currentTenant, profile.getItemId()); // Get system tenant segments and scoring first Map systemSegments = cacheService.getTenantCache("system", Segment.class); @@ -660,11 +688,9 @@ public SegmentsAndScores getSegmentsAndScoresForProfile(Profile profile) { LOGGER.warn("Found empty condition for segment {}, will skip", segment); continue; } - if (segment.getMetadata().isEnabled()) { - ParserHelper.resolveConditionType(definitionsService, segment.getCondition(), "segment " + segment.getItemId()); - if (persistenceService.testMatch(segment.getCondition(), profile)) { - segments.add(segment.getMetadata().getId()); - } + if (segment.getMetadata().isEnabled() && persistenceService.testMatch(segment.getCondition(), profile)) { + segments.add(segment.getMetadata().getId()); + tracer.trace("Profile matches system segment: " + segment.getMetadata().getId(), profile.getItemId()); } } } @@ -679,11 +705,9 @@ public SegmentsAndScores getSegmentsAndScoresForProfile(Profile profile) { LOGGER.warn("Found empty condition for segment {}, will skip", segment); continue; } - if (segment.getMetadata().isEnabled()) { - ParserHelper.resolveConditionType(definitionsService, segment.getCondition(), "segment " + segment.getItemId()); - if (persistenceService.testMatch(segment.getCondition(), profile)) { - segments.add(segment.getMetadata().getId()); - } + if (segment.getMetadata().isEnabled() && persistenceService.testMatch(segment.getCondition(), profile)) { + segments.add(segment.getMetadata().getId()); + tracer.trace("Profile matches tenant segment: " + segment.getMetadata().getId(), profile.getItemId()); } } } @@ -700,12 +724,12 @@ public SegmentsAndScores getSegmentsAndScoresForProfile(Profile profile) { } private void processScoring(Map scoringMap, Profile profile, Map scores) { + RequestTracer tracer = tracerService.getCurrentTracer(); Map scoreModifiers = (Map) profile.getSystemProperties().get("scoreModifiers"); for (Scoring scoring : scoringMap.values()) { if (scoring.getMetadata().isEnabled()) { int score = 0; for (ScoringElement scoringElement : scoring.getElements()) { - ParserHelper.resolveConditionType(definitionsService, scoringElement.getCondition(), "scoring " + scoring.getItemId()); if (persistenceService.testMatch(scoringElement.getCondition(), profile)) { score += scoringElement.getValue(); } @@ -770,20 +794,15 @@ public PartialList getScoringMetadatas(Query query) { @Override public Scoring getScoringDefinition(String scoringId) { String currentTenant = contextManager.getCurrentContext().getTenantId(); - Scoring definition = cacheService.getWithInheritance(scoringId, currentTenant, Scoring.class); - if (definition != null && definition.getMetadata().isEnabled() && definition.getElements() != null) { - for (ScoringElement element : definition.getElements()) { - ParserHelper.resolveConditionType(definitionsService, element.getCondition(), "scoring " + scoringId); - } - } - return definition; + return cacheService.getWithInheritance(scoringId, currentTenant, Scoring.class); } @Override public void setScoringDefinition(Scoring scoring) { + resolveScoring(scoring); + if (scoring.getMetadata().isEnabled()) { for (ScoringElement element : scoring.getElements()) { - ParserHelper.resolveConditionType(definitionsService, element.getCondition(), "scoring " + scoring.getItemId() + " element "); if (!scoring.getMetadata().isMissingPlugins()) { updateAutoGeneratedRules(scoring.getMetadata(), element.getCondition()); } @@ -1009,6 +1028,16 @@ private void clearAutoGeneratedRules(List rules, String idWithScope) { } private void getAutoGeneratedRules(Metadata metadata, Condition condition, Condition parentCondition, List rules) { + // Resolve condition type if needed before accessing it + if (condition.getConditionType() == null) { + TypeResolutionService typeResolutionService = getTypeResolutionService(); + if (typeResolutionService != null) { + typeResolutionService.resolveConditionType(condition, "auto-generated rules"); + } + } + if (condition.getConditionType() == null) { + return; // Cannot proceed without condition type + } Set tags = condition.getConditionType().getMetadata().getSystemTags(); if (tags.contains("eventCondition") && !tags.contains("profileCondition")) { String key = getGeneratedPropertyKey(condition, parentCondition); @@ -1072,7 +1101,7 @@ private void recalculatePastEventOccurrencesOnProfiles(Condition eventCondition, l.add(eventCondition); - Integer numberOfDays = PropertyHelper.getInteger(parentCondition.getParameter("numberOfDays")); + Integer numberOfDays = (Integer) parentCondition.getParameter("numberOfDays"); String fromDate = (String) parentCondition.getParameter("fromDate"); String toDate = (String) parentCondition.getParameter("toDate"); @@ -1598,7 +1627,7 @@ protected PartialList getMetadatas(int offset protected PartialList getMetadatas(Query query, Class clazz) { if (query.getCondition() != null) { - definitionsService.resolveConditionType(query.getCondition()); + definitionsService.getConditionValidationService().validate(query.getCondition()); } String currentTenantId = contextManager.getCurrentContext().getTenantId(); if (currentTenantId == null) { @@ -1680,4 +1709,52 @@ protected PartialList getMetadatas(Query quer } + /** + * Resolve a scoring's types and track its validity status using the TypeResolutionService. + * Automatically sets/clears the missingPlugins flag based on resolution success. + * + * @param scoring the scoring to resolve + */ + private void resolveScoring(Scoring scoring) { + if (scoring == null) { + return; + } + TypeResolutionService typeResolutionService = getTypeResolutionService(); + if (typeResolutionService == null) { + return; + } + + String scoringId = scoring.getMetadata().getId(); + boolean allResolved = true; + List reasons = new ArrayList<>(); + + if (scoring.getElements() != null) { + for (ScoringElement element : scoring.getElements()) { + if (element.getCondition() != null) { + // Use partial resolution - resolves types but skips parameter resolution for references/scripts + boolean elementValid = typeResolutionService != null && typeResolutionService.resolveConditionType(element.getCondition(), + "scoring element for scoring " + scoringId); + allResolved = allResolved && elementValid; + if (!elementValid) { + String unresolvedTypeId = element.getCondition().getConditionTypeId(); + reasons.add("Unresolved condition type" + (unresolvedTypeId != null ? ": " + unresolvedTypeId : "") + " in scoring element"); + } + } + } + } + + // Set/clear missingPlugins based on resolution success + if (scoring.getMetadata() != null) { + scoring.getMetadata().setMissingPlugins(!allResolved); + } + + // Track invalid objects + if (typeResolutionService != null) { + if (!allResolved) { + typeResolutionService.markInvalid("scoring", scoringId, String.join("; ", reasons)); + } else { + typeResolutionService.markValid("scoring", scoringId); + } + } + } } diff --git a/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml index 5a2881f985..1bba07ee3e 100644 --- a/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml +++ b/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml @@ -30,21 +30,12 @@ + - - - - - - + + + + + @@ -246,6 +246,7 @@ + @@ -267,6 +268,7 @@ + view @@ -296,6 +298,7 @@ + @@ -322,6 +325,7 @@ + @@ -338,6 +342,7 @@ + @@ -364,6 +369,7 @@ + @@ -677,7 +683,6 @@ - diff --git a/tracing/pom.xml b/tracing/pom.xml new file mode 100644 index 0000000000..4e3424860d --- /dev/null +++ b/tracing/pom.xml @@ -0,0 +1,36 @@ + + + + 4.0.0 + + + org.apache.unomi + unomi-root + 3.1.0-SNAPSHOT + + + unomi-tracing + Apache Unomi :: Tracing + Apache Unomi Tracing module + pom + + + tracing-api + tracing-impl + + \ No newline at end of file diff --git a/tracing/tracing-api/pom.xml b/tracing/tracing-api/pom.xml new file mode 100644 index 0000000000..379a3b6c7b --- /dev/null +++ b/tracing/tracing-api/pom.xml @@ -0,0 +1,68 @@ + + + + 4.0.0 + + + org.apache.unomi + unomi-tracing + 3.1.0-SNAPSHOT + + + unomi-tracing-api + Apache Unomi :: Tracing :: API + Apache Unomi Tracing API module + bundle + + + + + org.apache.unomi + unomi-bom + ${project.version} + pom + import + + + + + + + org.osgi + org.osgi.service.component.annotations + provided + + + + + + + org.apache.felix + maven-bundle-plugin + true + + + + org.apache.unomi.tracing.api + + + + + + + diff --git a/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/RequestTracer.java b/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/RequestTracer.java new file mode 100644 index 0000000000..4ec886077c --- /dev/null +++ b/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/RequestTracer.java @@ -0,0 +1,84 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.tracing.api; + +import java.util.Collection; + +/** + * Interface for tracing request processing and internal operations. + * This provides a generic tracing mechanism that can be used to track various operations + * including but not limited to: + * - Condition evaluations + * - Authentication + * - Event validation + * - Event processing + * - Rules evaluation + * - Action execution + */ +public interface RequestTracer { + /** + * Start tracing a new operation + * @param operationType the type of operation being traced (e.g. "condition-evaluation", "authentication", "event-validation") + * @param description description of what is being traced + * @param context additional context information for the operation + */ + void startOperation(String operationType, String description, Object context); + + /** + * End the current operation with a result + * @param result the result of the operation + * @param description explanation of the result. The implementation should include timing information. + */ + void endOperation(Object result, String description); + + /** + * Add a trace message to the current operation + * @param message the trace message + * @param context additional context information for the trace + */ + void trace(String message, Object context); + + /** + * Add validation information to the current operation + * @param validationMessages collection of validation messages/errors + * @param schemaId the ID of the schema that was used for validation + */ + void addValidationInfo(Collection validationMessages, String schemaId); + + /** + * Get the root trace node + * @return the root trace node or null if tracing is not enabled + */ + TraceNode getTraceNode(); + + /** + * Check if tracing is enabled + * @return true if tracing is enabled + */ + boolean isEnabled(); + + /** + * Enable or disable tracing + * @param enabled true to enable tracing, false to disable + */ + void setEnabled(boolean enabled); + + /** + * Reset the tracer, clearing all stored traces + */ + void reset(); +} \ No newline at end of file diff --git a/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/TraceNode.java b/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/TraceNode.java new file mode 100644 index 0000000000..be75ec09ba --- /dev/null +++ b/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/TraceNode.java @@ -0,0 +1,109 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.tracing.api; + +import java.io.Serializable; +import java.util.ArrayList; +import java.util.List; + +/** + * Represents a node in the request tracing tree structure. + * Each node contains information about an operation, its timing, and any child operations. + */ +public class TraceNode implements Serializable { + private String operationType; + private String description; + private Object context; + private Object result; + private long startTime; + private long endTime; + private List traces; + private List children; + + public TraceNode() { + this.traces = new ArrayList<>(); + this.children = new ArrayList<>(); + } + + public String getOperationType() { + return operationType; + } + + public void setOperationType(String operationType) { + this.operationType = operationType; + } + + public String getDescription() { + return description; + } + + public void setDescription(String description) { + this.description = description; + } + + public Object getContext() { + return context; + } + + public void setContext(Object context) { + this.context = context; + } + + public Object getResult() { + return result; + } + + public void setResult(Object result) { + this.result = result; + } + + public long getStartTime() { + return startTime; + } + + public void setStartTime(long startTime) { + this.startTime = startTime; + } + + public long getEndTime() { + return endTime; + } + + public void setEndTime(long endTime) { + this.endTime = endTime; + } + + public long getDuration() { + return endTime - startTime; + } + + public List getTraces() { + return traces; + } + + public void setTraces(List traces) { + this.traces = traces; + } + + public List getChildren() { + return children; + } + + public void setChildren(List children) { + this.children = children; + } +} \ No newline at end of file diff --git a/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/TracerService.java b/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/TracerService.java new file mode 100644 index 0000000000..a603be17e2 --- /dev/null +++ b/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/TracerService.java @@ -0,0 +1,52 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.tracing.api; + +/** + * Service for managing request tracing throughout the application. + * This service provides access to tracers that can monitor various operations + * including condition evaluations, authentication, event processing, etc. + */ +public interface TracerService { + /** + * Get the current request tracer + * @return the current request tracer + */ + RequestTracer getCurrentTracer(); + + /** + * Enable tracing for the current request + */ + void enableTracing(); + + /** + * Disable tracing for the current request + */ + void disableTracing(); + + /** + * Check if tracing is enabled for the current request + * @return true if tracing is enabled + */ + boolean isTracingEnabled(); + + /** + * Get the root trace node for the current request + * @return the root trace node or null if tracing is not enabled + */ + TraceNode getTraceNode(); +} \ No newline at end of file diff --git a/tracing/tracing-impl/pom.xml b/tracing/tracing-impl/pom.xml new file mode 100644 index 0000000000..27a0f2028c --- /dev/null +++ b/tracing/tracing-impl/pom.xml @@ -0,0 +1,94 @@ + + + + 4.0.0 + + + org.apache.unomi + unomi-tracing + 3.1.0-SNAPSHOT + + + unomi-tracing-impl + Apache Unomi :: Tracing :: Implementation + Apache Unomi Tracing Implementation module + bundle + + + + + org.apache.unomi + unomi-bom + ${project.version} + pom + import + + + + + + + org.apache.unomi + unomi-tracing-api + ${project.version} + provided + + + org.apache.unomi + unomi-api + provided + + + org.osgi + org.osgi.service.component.annotations + provided + + + org.slf4j + slf4j-api + provided + + + com.fasterxml.jackson.core + jackson-databind + provided + + + org.junit.jupiter + junit-jupiter + test + + + + + + + org.apache.felix + maven-bundle-plugin + true + + + + org.apache.unomi.tracing.impl + + + + + + + diff --git a/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultRequestTracer.java b/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultRequestTracer.java new file mode 100644 index 0000000000..d15d87b06b --- /dev/null +++ b/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultRequestTracer.java @@ -0,0 +1,146 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.tracing.impl; + +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TraceNode; + +import java.util.Collection; +import java.util.Stack; + +/** + * Default implementation of the RequestTracer interface that stores trace information in a tree structure + */ +public class DefaultRequestTracer implements RequestTracer { + + private final ThreadLocal enabled = ThreadLocal.withInitial(() -> false); + private final ThreadLocal currentNode = new ThreadLocal<>(); + private final ThreadLocal rootNode = new ThreadLocal<>(); + private final ThreadLocal> nodeStack = ThreadLocal.withInitial(Stack::new); + private static final int MAX_CONTEXT_STRING_LENGTH = 4096; + + private static String safeContextToString(Object context) { + if (context == null) { + return "null"; + } + try { + String rendered = String.valueOf(context); + if (rendered != null && rendered.length() > MAX_CONTEXT_STRING_LENGTH) { + return rendered.substring(0, MAX_CONTEXT_STRING_LENGTH) + "...(truncated)"; + } + return rendered; + } catch (StackOverflowError e) { + return ""; + } catch (Throwable t) { + return ""; + } + } + + @Override + public void startOperation(String operationType, String description, Object context) { + if (!isEnabled()) { + return; + } + + TraceNode node = new TraceNode(); + node.setOperationType(operationType); + node.setDescription(description); + node.setContext(context); + node.setStartTime(System.currentTimeMillis()); + + if (rootNode.get() == null) { + rootNode.set(node); + currentNode.set(node); + } else { + TraceNode parent = currentNode.get(); + parent.getChildren().add(node); + nodeStack.get().push(currentNode.get()); + currentNode.set(node); + } + } + + @Override + public void endOperation(Object result, String description) { + if (!isEnabled()) { + return; + } + + TraceNode node = currentNode.get(); + if (node != null) { + node.setResult(result); + node.setDescription(description); + node.setEndTime(System.currentTimeMillis()); + + if (!nodeStack.get().isEmpty()) { + currentNode.set(nodeStack.get().pop()); + } + } + } + + @Override + public void trace(String message, Object context) { + if (!isEnabled()) { + return; + } + + TraceNode node = currentNode.get(); + if (node != null) { + if (context != null) { + node.getTraces().add(message + " - Context: " + safeContextToString(context)); + } else { + node.getTraces().add(message); + } + } + } + + @Override + public void addValidationInfo(Collection validationMessages, String schemaId) { + if (!isEnabled()) { + return; + } + + TraceNode node = currentNode.get(); + if (node != null) { + node.getTraces().add("Validation against schema " + schemaId + ": " + safeContextToString(validationMessages)); + } + } + + @Override + public TraceNode getTraceNode() { + if (!isEnabled() || rootNode.get() == null) { + return null; + } + return rootNode.get(); + } + + @Override + public boolean isEnabled() { + return enabled.get(); + } + + @Override + public void setEnabled(boolean enabled) { + this.enabled.set(enabled); + } + + @Override + public void reset() { + rootNode.remove(); + currentNode.remove(); + nodeStack.get().clear(); + } +} diff --git a/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultTracerService.java b/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultTracerService.java new file mode 100644 index 0000000000..43e24aa45f --- /dev/null +++ b/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultTracerService.java @@ -0,0 +1,69 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.tracing.impl; + +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TracerService; +import org.apache.unomi.tracing.api.TraceNode; +import org.osgi.service.component.annotations.Component; + +/** + * Default implementation of the TracerService + */ +@Component(service = TracerService.class, immediate = true) +public class DefaultTracerService implements TracerService { + + private final ThreadLocal currentTracer = new ThreadLocal() { + @Override + protected RequestTracer initialValue() { + return new DefaultRequestTracer(); + } + }; + + @Override + public RequestTracer getCurrentTracer() { + return currentTracer.get(); + } + + @Override + public void enableTracing() { + RequestTracer tracer = getCurrentTracer(); + tracer.setEnabled(true); + tracer.reset(); + } + + @Override + public void disableTracing() { + RequestTracer tracer = getCurrentTracer(); + tracer.setEnabled(false); + tracer.reset(); + } + + @Override + public boolean isTracingEnabled() { + return getCurrentTracer().isEnabled(); + } + + @Override + public TraceNode getTraceNode() { + return getCurrentTracer().getTraceNode(); + } + + public void cleanup() { + currentTracer.remove(); + } +} \ No newline at end of file diff --git a/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/LoggingRequestTracer.java b/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/LoggingRequestTracer.java new file mode 100644 index 0000000000..34b7ec0e0b --- /dev/null +++ b/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/LoggingRequestTracer.java @@ -0,0 +1,160 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.tracing.impl; + +import org.apache.unomi.api.conditions.Condition; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.Objects; + +/** + * Implementation of RequestTracer that extends DefaultRequestTracer to leverage its tree structure + * while also logging operations to SLF4J with proper indentation. + */ +public class LoggingRequestTracer extends DefaultRequestTracer { + private static final Logger logger = LoggerFactory.getLogger(LoggingRequestTracer.class); + protected static final String INDENT_STRING = " "; // Two spaces per level + + protected final ThreadLocal indentLevel; + + public LoggingRequestTracer(boolean enabled) { + setEnabled(enabled); + this.indentLevel = ThreadLocal.withInitial(() -> 0); + } + + @Override + public void startOperation(String operationType, String description, Object context) { + super.startOperation(operationType, description, context); + if (!isEnabled()) { + return; + } + + logMessage(formatStartOperation(operationType, description), context); + indentLevel.set(indentLevel.get() + 1); + } + + @Override + public void endOperation(Object result, String description) { + if (isEnabled()) { + indentLevel.set(Math.max(0, indentLevel.get() - 1)); + logMessage(formatEndOperation(description, result), result); + } + super.endOperation(result, description); + } + + @Override + public void trace(String message, Object context) { + super.trace(message, context); + if (!isEnabled()) { + return; + } + + logMessage(message, context); + } + + @Override + public void reset() { + super.reset(); + indentLevel.set(0); + } + + /** + * Format a message for operation start. + * + * @param operationType the type of operation + * @param description the operation description + * @return formatted message + */ + protected String formatStartOperation(String operationType, String description) { + return String.format("Started operation: %s - %s", + Objects.toString(operationType, "null"), + Objects.toString(description, "null")); + } + + /** + * Format a message for operation end. + * + * @param description the operation description + * @param result the operation result + * @return formatted message + */ + protected String formatEndOperation(String description, Object result) { + return String.format("Ended operation: %s - Result: %s", + Objects.toString(description, "null"), + Objects.toString(result, "null")); + } + + /** + * Format a trace message with optional context. + * + * @param message the base message + * @param context optional context object + * @return formatted message + */ + public String formatMessage(String message, Object context) { + if (context instanceof Condition) { + Condition condition = (Condition) context; + if (message.startsWith("Starting ")) { + return String.format("Starting %s of condition %s - %s", + message.substring(9), condition.getConditionTypeId(), message); + } else { + return String.format("Condition %s - %s", condition.getConditionTypeId(), message); + } + } else if (context != null) { + return String.format("%s - Context: %s", + Objects.toString(message, "null"), + Objects.toString(context, "null")); + } + return Objects.toString(message, "null"); + } + + /** + * Add indentation to a message based on the current nesting level. + * + * @param message the message to indent + * @return the indented message + */ + public String indent(String message) { + if (message == null) { + return ""; + } + + int level = indentLevel.get(); + if (level <= 0) { + return message; + } + + StringBuilder indentation = new StringBuilder(level * INDENT_STRING.length()); + for (int i = 0; i < level; i++) { + indentation.append(INDENT_STRING); + } + return indentation.append(message).toString(); + } + + /** + * Log a message with proper formatting and indentation. + * + * @param message the message to log + * @param context optional context object + */ + protected void logMessage(String message, Object context) { + String formattedMessage = formatMessage(message, context); + String indentedMessage = indent(formattedMessage); + logger.debug(indentedMessage); + } +} \ No newline at end of file diff --git a/tracing/tracing-impl/src/test/java/org/apache/unomi/tracing/impl/DefaultTracerServiceTest.java b/tracing/tracing-impl/src/test/java/org/apache/unomi/tracing/impl/DefaultTracerServiceTest.java new file mode 100644 index 0000000000..d808fc09d7 --- /dev/null +++ b/tracing/tracing-impl/src/test/java/org/apache/unomi/tracing/impl/DefaultTracerServiceTest.java @@ -0,0 +1,281 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.tracing.impl; + +import org.apache.unomi.tracing.api.RequestTracer; +import org.apache.unomi.tracing.api.TraceNode; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.Arrays; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Unit tests for {@link DefaultTracerService} + */ +public class DefaultTracerServiceTest { + + private DefaultTracerService tracerService; + private static final int THREAD_COUNT = 10; + private static final int TIMEOUT_SECONDS = 10; + + @BeforeEach + public void setUp() { + tracerService = new DefaultTracerService(); + } + + @AfterEach + public void tearDown() { + tracerService.cleanup(); + } + + @Test + public void testGetCurrentTracer() { + RequestTracer tracer = tracerService.getCurrentTracer(); + assertNotNull(tracer, "Current tracer should not be null"); + assertTrue(tracer instanceof DefaultRequestTracer, "Current tracer should be an instance of DefaultRequestTracer"); + + // Test that we get the same tracer instance for the same thread + RequestTracer secondTracer = tracerService.getCurrentTracer(); + assertSame(tracer, secondTracer, "Should get same tracer instance within same thread"); + } + + @Test + public void testEnableDisableTracing() { + assertFalse(tracerService.isTracingEnabled(), "Tracing should be disabled by default"); + + tracerService.enableTracing(); + assertTrue(tracerService.isTracingEnabled(), "Tracing should be enabled after enableTracing()"); + + tracerService.disableTracing(); + assertFalse(tracerService.isTracingEnabled(), "Tracing should be disabled after disableTracing()"); + + // Verify that enable/disable resets the trace + tracerService.enableTracing(); + RequestTracer tracer = tracerService.getCurrentTracer(); + tracer.startOperation("test", "description", null); + assertNotNull(tracerService.getTraceNode(), "Should have trace node after operation"); + + tracerService.disableTracing(); + assertNull(tracerService.getTraceNode(), "Trace node should be reset after disable"); + } + + @Test + public void testTracingOperations() { + tracerService.enableTracing(); + RequestTracer tracer = tracerService.getCurrentTracer(); + + // Start a root operation + tracer.startOperation("test", "Root operation", null); + long startTime = System.currentTimeMillis(); + + // Add some traces + tracer.trace("Test message", null); + tracer.trace("Test with context", "context-data"); + + // Start a child operation + tracer.startOperation("child", "Child operation", "child-context"); + tracer.trace("Child trace", null); + tracer.endOperation("child-result", "Child completed"); + + // End root operation + tracer.endOperation("root-result", "Root completed"); + long endTime = System.currentTimeMillis(); + + // Get and verify the trace tree + TraceNode rootNode = tracerService.getTraceNode(); + assertNotNull(rootNode, "Root node should not be null"); + assertEquals("test", rootNode.getOperationType(), "Root operation type should match"); + assertEquals("Root completed", rootNode.getDescription(), "Root description should match"); + assertEquals("root-result", rootNode.getResult(), "Root result should match"); + assertEquals(2, rootNode.getTraces().size(), "Root should have 2 traces"); + assertEquals(1, rootNode.getChildren().size(), "Root should have 1 child"); + assertTrue(rootNode.getStartTime() >= startTime, "Root start time should be valid"); + assertTrue(rootNode.getEndTime() <= endTime, "Root end time should be valid"); + + TraceNode childNode = rootNode.getChildren().get(0); + assertEquals("child", childNode.getOperationType(), "Child operation type should match"); + assertEquals("Child completed", childNode.getDescription(), "Child description should match"); + assertEquals("child-context", childNode.getContext(), "Child context should match"); + assertEquals("child-result", childNode.getResult(), "Child result should match"); + assertEquals(1, childNode.getTraces().size(), "Child should have 1 trace"); + assertTrue(childNode.getStartTime() >= rootNode.getStartTime(), "Child start time should be after root"); + assertTrue(childNode.getEndTime() <= rootNode.getEndTime(), "Child end time should be before root end"); + } + + @Test + public void testValidationInfo() { + tracerService.enableTracing(); + RequestTracer tracer = tracerService.getCurrentTracer(); + + tracer.startOperation("validation", "Validation test", null); + tracer.addValidationInfo(Arrays.asList("error1", "error2"), "test-schema"); + tracer.endOperation(false, "Validation failed"); + + TraceNode node = tracerService.getTraceNode(); + assertNotNull(node, "Node should not be null"); + assertEquals(1, node.getTraces().size(), "Should have 1 validation trace"); + String validationTrace = node.getTraces().get(0); + assertTrue(validationTrace.contains("test-schema"), "Validation trace should contain schema id"); + assertTrue(validationTrace.contains("error1"), "Validation trace should contain first error"); + assertTrue(validationTrace.contains("error2"), "Validation trace should contain second error"); + } + + @Test + public void testTracingDisabled() { + RequestTracer tracer = tracerService.getCurrentTracer(); + + // Operations should not create any nodes when tracing is disabled + tracer.startOperation("test", "Test operation", null); + tracer.trace("Test message", null); + tracer.addValidationInfo(Arrays.asList("error"), "schema"); + tracer.endOperation("result", "Completed"); + + assertNull(tracerService.getTraceNode(), "No trace node should be created when tracing is disabled"); + } + + @Test + public void testReset() { + tracerService.enableTracing(); + RequestTracer tracer = tracerService.getCurrentTracer(); + + tracer.startOperation("test", "Test operation", null); + tracer.trace("Test message", null); + assertNotNull(tracerService.getTraceNode(), "Should have trace node before reset"); + + tracer.reset(); + assertNull(tracerService.getTraceNode(), "Should not have trace node after reset"); + + // Verify that tracing is still enabled after reset + assertTrue(tracerService.isTracingEnabled(), "Tracing should still be enabled after reset"); + } + + @Test + public void testCleanup() { + tracerService.enableTracing(); + RequestTracer tracer = tracerService.getCurrentTracer(); + + tracer.startOperation("test", "Test operation", null); + RequestTracer originalTracer = tracerService.getCurrentTracer(); + + tracerService.cleanup(); + RequestTracer newTracer = tracerService.getCurrentTracer(); + + assertNotSame(originalTracer, newTracer, "Should get new tracer instance after cleanup"); + assertFalse(newTracer.isEnabled(), "New tracer should be disabled"); + assertNull(tracerService.getTraceNode(), "New tracer should have no trace node"); + } + + @Test + public void testThreadSafety() throws InterruptedException { + ExecutorService executor = Executors.newFixedThreadPool(THREAD_COUNT); + CountDownLatch startLatch = new CountDownLatch(1); + CountDownLatch completionLatch = new CountDownLatch(THREAD_COUNT); + + try { + // Create tasks that will all start simultaneously + for (int i = 0; i < THREAD_COUNT; i++) { + final int threadId = i; + executor.submit(() -> { + try { + startLatch.await(); // Wait for all threads to be ready + + RequestTracer tracer = tracerService.getCurrentTracer(); + tracer.setEnabled(true); + + // Perform operations specific to this thread + tracer.startOperation("thread-" + threadId, "Thread operation", null); + tracer.trace("Thread message", "Thread-" + threadId); + tracer.endOperation("success", "Thread completed"); + + // Verify this thread's trace + TraceNode node = tracer.getTraceNode(); + assertNotNull(node, "Thread " + threadId + " should have a trace node"); + assertEquals("thread-" + threadId, node.getOperationType(), + "Thread " + threadId + " should have correct operation type"); + + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException("Thread interrupted during test", e); + } finally { + completionLatch.countDown(); + } + }); + } + + // Start all threads simultaneously + startLatch.countDown(); + + // Wait for all threads to complete + assertTrue(completionLatch.await(TIMEOUT_SECONDS, TimeUnit.SECONDS), + "All threads should complete within timeout"); + } finally { + executor.shutdown(); + if (!executor.awaitTermination(TIMEOUT_SECONDS, TimeUnit.SECONDS)) { + executor.shutdownNow(); + } + } + } + + @Test + public void testNullHandling() { + tracerService.enableTracing(); + RequestTracer tracer = tracerService.getCurrentTracer(); + + // Test null handling in various operations + tracer.startOperation(null, null, null); + tracer.trace(null, null); + tracer.addValidationInfo(null, null); + tracer.endOperation(null, null); + + TraceNode node = tracerService.getTraceNode(); + assertNotNull(node, "Node should exist even with null values"); + assertNull(node.getOperationType(), "Operation type should be null"); + assertNull(node.getDescription(), "Description should be null"); + assertNull(node.getContext(), "Context should be null"); + assertNull(node.getResult(), "Result should be null"); + } + + @Test + public void testTraceShouldNotFailWhenContextToStringOverflowsStack() { + tracerService.enableTracing(); + RequestTracer tracer = tracerService.getCurrentTracer(); + + tracer.startOperation("test", "Root operation", null); + Object badContext = new Object() { + @Override + public String toString() { + return toString(); + } + }; + + assertDoesNotThrow(() -> tracer.trace("Test with bad context", badContext), + "Tracer.trace should not throw even if context.toString overflows the stack"); + + TraceNode rootNode = tracerService.getTraceNode(); + assertNotNull(rootNode, "Root node should be created"); + assertEquals(1, rootNode.getTraces().size(), "Trace should be recorded even when context rendering fails"); + assertTrue(rootNode.getTraces().get(0).contains("StackOverflowError"), + "Trace should contain a StackOverflowError marker when context rendering overflows"); + } +} \ No newline at end of file diff --git a/wab/pom.xml b/wab/pom.xml index b391ea26b8..2b121a2aa4 100644 --- a/wab/pom.xml +++ b/wab/pom.xml @@ -46,107 +46,21 @@ unomi-api provided - - org.apache.unomi - unomi-persistence-spi - provided - - - - org.osgi - org.osgi.framework - provided - - - org.osgi - org.osgi.util.tracker - provided - - - org.osgi - org.osgi.service.component - provided - org.osgi org.osgi.service.component.annotations provided - - org.osgi - org.osgi.service.metatype - provided - org.osgi org.osgi.service.metatype.annotations provided - - javax.servlet - javax.servlet-api - provided - - - com.fasterxml.jackson.core - jackson-databind - provided - - - com.fasterxml.jackson.dataformat - jackson-dataformat-yaml - provided - - - com.fasterxml.jackson.core - jackson-annotations - provided - - - com.fasterxml.jackson.core - jackson-core - provided - - - org.ops4j.pax.web - pax-web-api - provided - - - commons-io - commons-io - provided - - - commons-collections - commons-collections - provided - - - org.apache.commons - commons-lang3 - provided - - - org.yaml - snakeyaml - provided - - - commons-beanutils - commons-beanutils - provided - org.slf4j slf4j-api provided - - - com.opencsv - opencsv - @@ -164,30 +78,6 @@ - - org.codehaus.mojo - build-helper-maven-plugin - - - attach-artifacts - package - - attach-artifact - - - - - - src/main/resources/org.apache.unomi.web.cfg - - cfg - unomicfg - - - - - - diff --git a/web-servlets/pom.xml b/web-servlets/pom.xml new file mode 100644 index 0000000000..d940c6276f --- /dev/null +++ b/web-servlets/pom.xml @@ -0,0 +1,189 @@ + + + + + 4.0.0 + + org.apache.unomi + unomi-root + 3.1.0-SNAPSHOT + + unomi-web-servlets + Apache Unomi :: Core Web Servlets + Apache Unomi Context Server Core Web Servlets + bundle + + + + + org.apache.unomi + unomi-bom + ${project.version} + pom + import + + + + + + + + org.apache.unomi + unomi-api + provided + + + org.apache.unomi + unomi-persistence-spi + provided + + + org.apache.unomi + unomi-rest + provided + + + + + org.osgi + org.osgi.framework + provided + + + org.osgi + org.osgi.util.tracker + provided + + + org.osgi + org.osgi.service.component + provided + + + org.osgi + org.osgi.service.component.annotations + provided + + + org.osgi + org.osgi.service.metatype + provided + + + org.osgi + org.osgi.service.metatype.annotations + provided + + + javax.servlet + javax.servlet-api + provided + + + + + com.fasterxml.jackson.core + jackson-databind + provided + + + com.fasterxml.jackson.dataformat + jackson-dataformat-yaml + provided + + + com.fasterxml.jackson.core + jackson-annotations + provided + + + com.fasterxml.jackson.core + jackson-core + provided + + + org.ops4j.pax.web + pax-web-api + provided + + + commons-io + commons-io + provided + + + commons-collections + commons-collections + provided + + + org.apache.commons + commons-lang3 + provided + + + org.yaml + snakeyaml + provided + + + commons-beanutils + commons-beanutils + provided + + + org.slf4j + slf4j-api + provided + + + com.opencsv + opencsv + + + + + + + org.codehaus.mojo + build-helper-maven-plugin + + + attach-artifacts + package + + attach-artifact + + + + + + src/main/resources/org.apache.unomi.web.cfg + + cfg + unomicfg + + + + + + + + + + diff --git a/wab/src/main/java/org/apache/unomi/web/ClientServlet.java b/web-servlets/src/main/java/org/apache/unomi/web/servlets/ClientServlet.java similarity index 84% rename from wab/src/main/java/org/apache/unomi/web/ClientServlet.java rename to web-servlets/src/main/java/org/apache/unomi/web/servlets/ClientServlet.java index 94850e6ad0..e3cb35acb1 100644 --- a/wab/src/main/java/org/apache/unomi/web/ClientServlet.java +++ b/web-servlets/src/main/java/org/apache/unomi/web/servlets/ClientServlet.java @@ -15,25 +15,33 @@ * limitations under the License. */ -package org.apache.unomi.web; +package org.apache.unomi.web.servlets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.servlet.Servlet; import javax.servlet.ServletConfig; import javax.servlet.ServletException; -import javax.servlet.annotation.WebServlet; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; +import org.osgi.service.component.annotations.Component; /** * @deprecated this servlet is now deprecated, because it have been migrated to REST endpoint. * A servlet filter to serve a context-specific Javascript containing the current request context object. */ @Deprecated -@WebServlet(name = "ClientServlet", urlPatterns = "/client/*") +@Component( + service = Servlet.class, + immediate = true, + property = { + "osgi.http.whiteboard.servlet.name=ClientServlet", + "osgi.http.whiteboard.servlet.pattern=/client/*" + } +) public class ClientServlet extends HttpServlet { private static final Logger LOGGER = LoggerFactory.getLogger(ClientServlet.class.getName()); diff --git a/wab/src/main/java/org/apache/unomi/web/ContextServlet.java b/web-servlets/src/main/java/org/apache/unomi/web/servlets/ContextServlet.java similarity index 82% rename from wab/src/main/java/org/apache/unomi/web/ContextServlet.java rename to web-servlets/src/main/java/org/apache/unomi/web/servlets/ContextServlet.java index 6c04f60b62..a86f5d06c7 100644 --- a/wab/src/main/java/org/apache/unomi/web/ContextServlet.java +++ b/web-servlets/src/main/java/org/apache/unomi/web/servlets/ContextServlet.java @@ -15,25 +15,34 @@ * limitations under the License. */ -package org.apache.unomi.web; +package org.apache.unomi.web.servlets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.servlet.Servlet; import javax.servlet.ServletConfig; import javax.servlet.ServletException; -import javax.servlet.annotation.WebServlet; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; +import org.osgi.service.component.annotations.Component; /** * @deprecated this servlet is now deprecated, because it have been migrated to REST endpoint. * A servlet filter to serve a context-specific Javascript containing the current request context object. */ @Deprecated -@WebServlet(name = "ContextServlet", urlPatterns = {"/context.json", "/context.js"}) +@Component( + service = Servlet.class, + immediate = true, + property = { + "osgi.http.whiteboard.servlet.name=ContextServlet", + "osgi.http.whiteboard.servlet.pattern=/context.json", + "osgi.http.whiteboard.servlet.pattern=/context.js" + } +) public class ContextServlet extends HttpServlet { private static final Logger LOGGER = LoggerFactory.getLogger(ContextServlet.class.getName()); diff --git a/wab/src/main/java/org/apache/unomi/web/EventsCollectorServlet.java b/web-servlets/src/main/java/org/apache/unomi/web/servlets/EventsCollectorServlet.java similarity index 83% rename from wab/src/main/java/org/apache/unomi/web/EventsCollectorServlet.java rename to web-servlets/src/main/java/org/apache/unomi/web/servlets/EventsCollectorServlet.java index bb72b2cb2d..2a953c57fb 100644 --- a/wab/src/main/java/org/apache/unomi/web/EventsCollectorServlet.java +++ b/web-servlets/src/main/java/org/apache/unomi/web/servlets/EventsCollectorServlet.java @@ -15,24 +15,32 @@ * limitations under the License. */ -package org.apache.unomi.web; +package org.apache.unomi.web.servlets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.servlet.Servlet; import javax.servlet.ServletConfig; import javax.servlet.ServletException; -import javax.servlet.annotation.WebServlet; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; +import org.osgi.service.component.annotations.Component; /** * @deprecated this servlet is now deprecated, because it have been migrated to REST endpoint. */ @Deprecated -@WebServlet(name = "EventsCollectorServlet", urlPatterns = "/eventcollector") +@Component( + service = Servlet.class, + immediate = true, + property = { + "osgi.http.whiteboard.servlet.name=EventsCollectorServlet", + "osgi.http.whiteboard.servlet.pattern=/eventcollector" + } +) public class EventsCollectorServlet extends HttpServlet { private static final Logger LOGGER = LoggerFactory.getLogger(EventsCollectorServlet.class.getName()); diff --git a/wab/src/main/java/org/apache/unomi/web/HttpServletRequestForwardWrapper.java b/web-servlets/src/main/java/org/apache/unomi/web/servlets/HttpServletRequestForwardWrapper.java similarity index 98% rename from wab/src/main/java/org/apache/unomi/web/HttpServletRequestForwardWrapper.java rename to web-servlets/src/main/java/org/apache/unomi/web/servlets/HttpServletRequestForwardWrapper.java index 21399ddc4f..b801754f42 100644 --- a/wab/src/main/java/org/apache/unomi/web/HttpServletRequestForwardWrapper.java +++ b/web-servlets/src/main/java/org/apache/unomi/web/servlets/HttpServletRequestForwardWrapper.java @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.unomi.web; +package org.apache.unomi.web.servlets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/wab/src/main/java/org/apache/unomi/web/HttpUtils.java b/web-servlets/src/main/java/org/apache/unomi/web/servlets/HttpUtils.java similarity index 98% rename from wab/src/main/java/org/apache/unomi/web/HttpUtils.java rename to web-servlets/src/main/java/org/apache/unomi/web/servlets/HttpUtils.java index 664d105d0d..ddc9dff316 100644 --- a/wab/src/main/java/org/apache/unomi/web/HttpUtils.java +++ b/web-servlets/src/main/java/org/apache/unomi/web/servlets/HttpUtils.java @@ -15,7 +15,7 @@ * limitations under the License. */ -package org.apache.unomi.web; +package org.apache.unomi.web.servlets; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; diff --git a/wab/src/main/java/org/apache/unomi/web/WebConfig.java b/web-servlets/src/main/java/org/apache/unomi/web/servlets/WebConfig.java similarity index 99% rename from wab/src/main/java/org/apache/unomi/web/WebConfig.java rename to web-servlets/src/main/java/org/apache/unomi/web/servlets/WebConfig.java index db45ed8e41..f5d1a145dc 100644 --- a/wab/src/main/java/org/apache/unomi/web/WebConfig.java +++ b/web-servlets/src/main/java/org/apache/unomi/web/servlets/WebConfig.java @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.unomi.web; +package org.apache.unomi.web.servlets; import org.apache.unomi.api.services.ConfigSharingService; import org.osgi.service.component.annotations.Activate; diff --git a/wab/src/main/resources/org.apache.unomi.web.cfg b/web-servlets/src/main/resources/org.apache.unomi.web.cfg similarity index 100% rename from wab/src/main/resources/org.apache.unomi.web.cfg rename to web-servlets/src/main/resources/org.apache.unomi.web.cfg From cbf3566c4f2b1c9400143339f30b9d8c65530780 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Wed, 17 Jun 2026 08:18:44 +0200 Subject: [PATCH 2/6] UNOMI-873: Fix package copy referencing unomi-wab instead of unomi-web-servlets for unomicfg --- package/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/pom.xml b/package/pom.xml index 0ed6875d5c..7fd7b4d41c 100644 --- a/package/pom.xml +++ b/package/pom.xml @@ -204,7 +204,7 @@ org.apache.unomi - unomi-wab + unomi-web-servlets unomicfg cfg ${project.build.directory}/assembly/etc From 67c9577cc74d90a27c005c3ee40c5f44da08e42d Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Wed, 17 Jun 2026 13:39:04 +0200 Subject: [PATCH 3/6] UNOMI-873: Fix tracing review issues (security, correctness, API) - Fix action executors (CopyProperties, EvaluateVisitProperties, EventToProfileProperty, GroovyActionDispatcher) unconditionally enabling tracing regardless of explain flag; now guard with isTracingEnabled() and null-check tracer before each call - Convert TraceNode context/result fields from Object to String, applying safeContextToString() in startOperation/endOperation to prevent Jackson serialization of arbitrary domain objects - Add cleanup() to TracerService interface; call it unconditionally in both endpoint finally blocks to prevent ThreadLocal leaks in Karaf thread pools - Fix endOperation/setRequestTracing ordering in both endpoints so root node endTime is set before the trace tree is captured - Fix inverted validation result booleans in RulesServiceImpl, SegmentServiceImpl and GoalsServiceImpl (isEmpty not !isEmpty) - Wrap validate() calls in try/catch in all three service impls to ensure endOperation is always called even if validation throws - Add null/enabled guards to SegmentServiceImpl hot-path tracer calls (isProfileInSegment, getSegmentsAndScoresForProfile, processScoring) to prevent NPE when tracerService is unbound - Add MAX_TRACE_DEPTH=100 guard in DefaultRequestTracer to bound trace tree size for complex rule sets - Return unmodifiable views from TraceNode.getChildren/getTraces; add addChild/addTrace mutators; clamp getDuration() to >= 0 - Remove redundant null check after String.valueOf in safeContextToString - Add tests for endOperation-without-startOperation and extra-endOperation-after-balanced-pair behaviours --- .../actions/GroovyActionDispatcher.java | 29 ++++--- .../actions/CopyPropertiesAction.java | 61 +++++++------- .../EvaluateVisitPropertiesAction.java | 81 +++++++++++-------- .../actions/EventToProfilePropertyAction.java | 40 ++++----- .../rest/endpoints/ContextJsonEndpoint.java | 3 +- .../endpoints/EventsCollectorEndpoint.java | 3 +- .../services/impl/goals/GoalsServiceImpl.java | 30 ++++++- .../services/impl/rules/RulesServiceImpl.java | 15 +++- .../impl/segments/SegmentServiceImpl.java | 36 ++++++--- .../apache/unomi/tracing/api/TraceNode.java | 29 ++++--- .../unomi/tracing/api/TracerService.java | 6 ++ .../tracing/impl/DefaultRequestTracer.java | 21 +++-- .../impl/DefaultTracerServiceTest.java | 25 ++++++ 13 files changed, 246 insertions(+), 133 deletions(-) diff --git a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java index 17932568ae..08eb876453 100644 --- a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java +++ b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java @@ -83,43 +83,42 @@ public String getPrefix() { } public Integer execute(Action action, Event event, String actionName) { - RequestTracer tracer = tracerService.getCurrentTracer(); - if (!tracer.isEnabled()) { - tracer.setEnabled(true); + final RequestTracer tracer = (tracerService != null && tracerService.isTracingEnabled()) + ? tracerService.getCurrentTracer() : null; + if (tracer != null) { + tracer.startOperation("groovy-action", "Executing Groovy action", new HashMap() {{ + put("action.name", actionName); + put("action.type", action.getActionTypeId()); + put("event.type", event.getEventType()); + }}); } - tracer.startOperation("groovy-action", "Executing Groovy action", new HashMap() {{ - put("action.name", actionName); - put("action.type", action.getActionTypeId()); - put("event.type", event.getEventType()); - }}); - try { Class scriptClass = groovyActionsService.getCompiledScript(actionName); if (scriptClass == null) { LOGGER.warn("Couldn't find a Groovy action with name {}, action will not execute!", actionName); - tracer.trace("Action not found", null); + if (tracer != null) tracer.trace("Action not found", null); return EventService.NO_CHANGE; } - + try { Script script = scriptClass.getDeclaredConstructor().newInstance(); setScriptVariables(script, action, event); - + return new MetricAdapter(metricsService, this.getClass().getName() + ".action.groovy." + actionName) { @Override public Integer execute(Object... args) throws Exception { return (Integer) script.invokeMethod("execute", null); } }.runWithTimer(); - + } catch (Exception e) { LOGGER.error("Error executing Groovy action with key={}", actionName, e); - tracer.trace("Error executing action", e); + if (tracer != null) tracer.trace("Error executing action", e); return EventService.NO_CHANGE; } } finally { - tracer.endOperation(null, "Completed Groovy action execution"); + if (tracer != null) tracer.endOperation(null, "Completed Groovy action execution"); } } diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/CopyPropertiesAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/CopyPropertiesAction.java index 2b7225d7bc..8f23eee3e2 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/CopyPropertiesAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/CopyPropertiesAction.java @@ -51,41 +51,44 @@ public void setTracerService(TracerService tracerService) { @SuppressWarnings({ "unchecked", "rawtypes" }) public int execute(Action action, Event event) { - RequestTracer tracer = tracerService.getCurrentTracer(); - if (!tracer.isEnabled()) { - tracer.setEnabled(true); + final RequestTracer tracer = (tracerService != null && tracerService.isTracingEnabled()) + ? tracerService.getCurrentTracer() : null; + if (tracer != null) { + tracer.startOperation("copy-properties", "Copying properties from event to profile", new HashMap() {{ + put("action.id", action.getActionTypeId()); + put("event.type", event.getEventType()); + }}); } - tracer.startOperation("copy-properties", "Copying properties from event to profile", new HashMap() {{ - put("action.id", action.getActionTypeId()); - put("event.type", event.getEventType()); - }}); - try { boolean atLeastOnechanged = false; List mandatoryPropTypeSystemTags = (List) action.getParameterValues().get("mandatoryPropTypeSystemTag"); String singleValueStrategy = (String) action.getParameterValues().get("singleValueStrategy"); Map propsToCopy = getEventPropsToCopy(action, event); - tracer.trace("Found properties to copy", new HashMap() {{ - put("properties.count", propsToCopy.size()); - }}); + if (tracer != null) { + tracer.trace("Found properties to copy", new HashMap() {{ + put("properties.count", propsToCopy.size()); + }}); + } for (Map.Entry entry : propsToCopy.entrySet()) { String mappedProperty = resolvePropertyName(entry.getKey()); - tracer.startOperation("copy-property", "Copying single property", new HashMap() {{ - put("property.name", mappedProperty); - }}); + if (tracer != null) { + tracer.startOperation("copy-property", "Copying single property", new HashMap() {{ + put("property.name", mappedProperty); + }}); + } try { // propType Check PropertyType propertyType = profileService.getPropertyType(mappedProperty); Object previousValue = event.getProfile().getProperty(mappedProperty); - + if (mandatoryPropTypeSystemTags != null && mandatoryPropTypeSystemTags.size() > 0) { if (propertyType == null || propertyType.getMetadata() == null || propertyType.getMetadata().getSystemTags() == null || !propertyType.getMetadata().getSystemTags().containsAll(mandatoryPropTypeSystemTags)) { - tracer.trace("Skipping property due to missing required system tags", null); + if (tracer != null) tracer.trace("Skipping property due to missing required system tags", null); continue; } } @@ -104,32 +107,36 @@ public int execute(Action action, Event event) { } else if (entry.getValue() instanceof List) { LOGGER.error("Impossible to copy the property of type List to the profile, either a single value already exist on the profile or the property type is declared as a single value property. Enable debug log level for more information"); LOGGER.debug("cannot copy property {}, because it's a List", mappedProperty); - tracer.trace("Error: Cannot copy List to single value property", null); + if (tracer != null) tracer.trace("Error: Cannot copy List to single value property", null); changed = false; } else { changed = PropertyHelper.setProperty(event.getProfile(), propertyName, entry.getValue(), singleValueStrategy); } } - - tracer.trace("Property copy result", new HashMap() {{ - put("changed", changed); - }}); + + if (tracer != null) { + tracer.trace("Property copy result", new HashMap() {{ + put("changed", changed); + }}); + } atLeastOnechanged = atLeastOnechanged || changed; } finally { - tracer.endOperation(null, "Completed property copy operation"); + if (tracer != null) tracer.endOperation(null, "Completed property copy operation"); } } final boolean finalAtLeastOneChanged = atLeastOnechanged; - tracer.trace("Overall copy operation result", new HashMap() {{ - put("profile_updated", finalAtLeastOneChanged); - }}); + if (tracer != null) { + tracer.trace("Overall copy operation result", new HashMap() {{ + put("profile_updated", finalAtLeastOneChanged); + }}); + } return finalAtLeastOneChanged ? EventService.PROFILE_UPDATED : EventService.NO_CHANGE; } catch (Exception e) { - tracer.trace("Error during property copy operation", e); + if (tracer != null) tracer.trace("Error during property copy operation", e); throw e; } finally { - tracer.endOperation(null, "Completed all property copy operations"); + if (tracer != null) tracer.endOperation(null, "Completed all property copy operations"); } } diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EvaluateVisitPropertiesAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EvaluateVisitPropertiesAction.java index 697142a5af..b5d3428e67 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EvaluateVisitPropertiesAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EvaluateVisitPropertiesAction.java @@ -48,17 +48,16 @@ public void setTracerService(TracerService tracerService) { } public int execute(Action action, Event event) { - RequestTracer tracer = tracerService.getCurrentTracer(); - if (!tracer.isEnabled()) { - tracer.setEnabled(true); + final RequestTracer tracer = (tracerService != null && tracerService.isTracingEnabled()) + ? tracerService.getCurrentTracer() : null; + if (tracer != null) { + tracer.startOperation("evaluate-visit-properties", "Evaluating visit properties", new HashMap() {{ + put("action.type", action.getActionTypeId()); + put("event.type", event.getEventType()); + put("event.timestamp", event.getTimeStamp()); + }}); } - tracer.startOperation("evaluate-visit-properties", "Evaluating visit properties", new HashMap() {{ - put("action.type", action.getActionTypeId()); - put("event.type", event.getEventType()); - put("event.timestamp", event.getTimeStamp()); - }}); - try { SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'"); dateFormat.setTimeZone(TimeZone.getTimeZone("UTC")); @@ -68,61 +67,73 @@ public int execute(Action action, Event event) { Date currentProfilePreviousVisit = extractDateFromProperty(event.getProfile(), "previousVisit", dateFormat); Date currentProfileLastVisit = extractDateFromProperty(event.getProfile(), "lastVisit", dateFormat); - tracer.trace("Current visit properties", new HashMap() {{ - put("first.visit", currentProfileFirstVisit); - put("previous.visit", currentProfilePreviousVisit); - put("last.visit", currentProfileLastVisit); - }}); + if (tracer != null) { + tracer.trace("Current visit properties", new HashMap() {{ + put("first.visit", currentProfileFirstVisit); + put("previous.visit", currentProfilePreviousVisit); + put("last.visit", currentProfileLastVisit); + }}); + } final int[] result = {EventService.NO_CHANGE}; if (currentProfileFirstVisit == null || currentProfileFirstVisit.after(currentEventTimeStamp)) { // event < firstVisit < previousVisit < lastVisit. we need to update firstVisit boolean updated = PropertyHelper.setProperty(event.getProfile(), "properties.firstVisit", dateFormat.format(currentEventTimeStamp), "alwaysSet"); - tracer.trace("First visit update", new HashMap() {{ - put("updated", updated); - put("new.value", currentEventTimeStamp); - }}); + if (tracer != null) { + tracer.trace("First visit update", new HashMap() {{ + put("updated", updated); + put("new.value", currentEventTimeStamp); + }}); + } result[0] = updated ? EventService.PROFILE_UPDATED : result[0]; } if (currentProfileLastVisit == null || currentProfileLastVisit.before(currentEventTimeStamp)) { // firstVisit < previousVisit < lastVisit < event. we need to update lastVisit and previousVisit boolean updated = PropertyHelper.setProperty(event.getProfile(), "properties.lastVisit", dateFormat.format(currentEventTimeStamp), "alwaysSet"); - tracer.trace("Last visit update", new HashMap() {{ - put("updated", updated); - put("new.value", currentEventTimeStamp); - }}); + if (tracer != null) { + tracer.trace("Last visit update", new HashMap() {{ + put("updated", updated); + put("new.value", currentEventTimeStamp); + }}); + } if (updated) { result[0] = EventService.PROFILE_UPDATED; if (currentProfileLastVisit != null) { boolean prevUpdated = PropertyHelper.setProperty(event.getProfile(), "properties.previousVisit", dateFormat.format(currentProfileLastVisit), "alwaysSet"); - tracer.trace("Previous visit update", new HashMap() {{ - put("updated", prevUpdated); - put("new.value", currentProfileLastVisit); - }}); + if (tracer != null) { + tracer.trace("Previous visit update", new HashMap() {{ + put("updated", prevUpdated); + put("new.value", currentProfileLastVisit); + }}); + } } } } else if (currentProfilePreviousVisit != null && currentProfilePreviousVisit.before(currentEventTimeStamp)) { // firstVisit < previousVisit < event < lastVisit. we need to update previousVisit boolean updated = PropertyHelper.setProperty(event.getProfile(), "properties.previousVisit", dateFormat.format(currentEventTimeStamp), "alwaysSet"); - tracer.trace("Previous visit update", new HashMap() {{ - put("updated", updated); - put("new.value", currentEventTimeStamp); - }}); + if (tracer != null) { + tracer.trace("Previous visit update", new HashMap() {{ + put("updated", updated); + put("new.value", currentEventTimeStamp); + }}); + } result[0] = updated ? EventService.PROFILE_UPDATED : result[0]; } - tracer.trace("Operation result", new HashMap() {{ - put("profile.updated", result[0] == EventService.PROFILE_UPDATED); - }}); + if (tracer != null) { + tracer.trace("Operation result", new HashMap() {{ + put("profile.updated", result[0] == EventService.PROFILE_UPDATED); + }}); + } return result[0]; } catch (Exception e) { - tracer.trace("Error during visit properties evaluation", e); + if (tracer != null) tracer.trace("Error during visit properties evaluation", e); throw e; } finally { - tracer.endOperation(null, "Completed visit properties evaluation"); + if (tracer != null) tracer.endOperation(null, "Completed visit properties evaluation"); } } diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EventToProfilePropertyAction.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EventToProfilePropertyAction.java index 2920e7ece8..9264eb926e 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EventToProfilePropertyAction.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/EventToProfilePropertyAction.java @@ -40,44 +40,46 @@ public void setTracerService(TracerService tracerService) { } public int execute(Action action, Event event) { - RequestTracer tracer = tracerService.getCurrentTracer(); - if (!tracer.isEnabled()) { - tracer.setEnabled(true); - } + final RequestTracer tracer = (tracerService != null && tracerService.isTracingEnabled()) + ? tracerService.getCurrentTracer() : null; String eventPropertyName = (String) action.getParameterValues().get("eventPropertyName"); String profilePropertyName = (String) action.getParameterValues().get("profilePropertyName"); - tracer.startOperation("event-to-profile-property", "Copying event property to profile property", new HashMap() {{ - put("action.type", action.getActionTypeId()); - put("event.type", event.getEventType()); - put("event.property", eventPropertyName); - put("profile.property", profilePropertyName); - }}); + if (tracer != null) { + tracer.startOperation("event-to-profile-property", "Copying event property to profile property", new HashMap() {{ + put("action.type", action.getActionTypeId()); + put("event.type", event.getEventType()); + put("event.property", eventPropertyName); + put("profile.property", profilePropertyName); + }}); + } try { Object currentProfileValue = event.getProfile().getProperty(profilePropertyName); Object eventValue = event.getProperty(eventPropertyName); boolean needsUpdate = currentProfileValue == null || !currentProfileValue.equals(eventValue); - tracer.trace("Property values", new HashMap() {{ - put("current.profile.value", currentProfileValue); - put("event.value", eventValue); - put("needs.update", needsUpdate); - }}); + if (tracer != null) { + tracer.trace("Property values", new HashMap() {{ + put("current.profile.value", currentProfileValue); + put("event.value", eventValue); + put("needs.update", needsUpdate); + }}); + } if (needsUpdate) { event.getProfile().setProperty(profilePropertyName, eventValue); - tracer.trace("Property updated", null); + if (tracer != null) tracer.trace("Property updated", null); return EventService.PROFILE_UPDATED; } - tracer.trace("No update needed", null); + if (tracer != null) tracer.trace("No update needed", null); return EventService.NO_CHANGE; } catch (Exception e) { - tracer.trace("Error during property copy", e); + if (tracer != null) tracer.trace("Error during property copy", e); throw e; } finally { - tracer.endOperation(null, "Completed event to profile property copy"); + if (tracer != null) tracer.endOperation(null, "Completed event to profile property copy"); } } } diff --git a/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java b/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java index f5b70de9c6..b8beef26c6 100644 --- a/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java +++ b/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java @@ -215,8 +215,8 @@ public ContextResponse contextJSONAsPost(ContextRequest contextRequest, // Add tracing information if requested if (explain) { - contextResponse.setRequestTracing(tracerService.getTraceNode()); tracerService.getCurrentTracer().endOperation(null, "Context request processed successfully"); + contextResponse.setRequestTracing(tracerService.getTraceNode()); } return contextResponse; @@ -224,6 +224,7 @@ public ContextResponse contextJSONAsPost(ContextRequest contextRequest, if (explain) { tracerService.disableTracing(); } + tracerService.cleanup(); } } diff --git a/rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java b/rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java index a5954ba0bf..9117e40c9d 100644 --- a/rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java +++ b/rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java @@ -141,8 +141,8 @@ private EventCollectorResponse doEvent(EventsCollectorRequest eventsCollectorReq // Add tracing information if requested if (explain) { - response.setRequestTracing(tracerService.getTraceNode()); tracerService.getCurrentTracer().endOperation(null, "Event collection request processed successfully"); + response.setRequestTracing(tracerService.getTraceNode()); } return response; @@ -150,6 +150,7 @@ private EventCollectorResponse doEvent(EventsCollectorRequest eventsCollectorReq if (explain) { tracerService.disableTracing(); } + tracerService.cleanup(); } } } diff --git a/services/src/main/java/org/apache/unomi/services/impl/goals/GoalsServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/goals/GoalsServiceImpl.java index 6260aee0b7..af47246868 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/goals/GoalsServiceImpl.java +++ b/services/src/main/java/org/apache/unomi/services/impl/goals/GoalsServiceImpl.java @@ -214,14 +214,25 @@ public void setGoal(Goal goal) { // Validate condition (skips parameters with references/scripts) // Validation service will auto-resolve types if needed - List validationErrors = definitionsService.getConditionValidationService().validate(goal.getStartEvent()); + List validationErrors; + try { + validationErrors = definitionsService.getConditionValidationService().validate(goal.getStartEvent()); + } catch (Exception e) { + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + tracer.endOperation(false, "Goal start event validation threw: " + e.getMessage()); + } + } + throw e; + } // Add validation info to tracer if (tracerService != null) { RequestTracer tracer = tracerService.getCurrentTracer(); if (tracer != null && tracer.isEnabled()) { tracer.addValidationInfo(validationErrors, "goal-start-event-validation"); - tracer.endOperation(!validationErrors.isEmpty(), String.format("Goal start event validation completed with %d errors", validationErrors.size())); + tracer.endOperation(validationErrors.isEmpty(), String.format("Goal start event validation completed with %d errors", validationErrors.size())); } } @@ -270,14 +281,25 @@ public void setGoal(Goal goal) { // Validate condition (skips parameters with references/scripts) // Validation service will auto-resolve types if needed - List targetValidationErrors = definitionsService.getConditionValidationService().validate(goal.getTargetEvent()); + List targetValidationErrors; + try { + targetValidationErrors = definitionsService.getConditionValidationService().validate(goal.getTargetEvent()); + } catch (Exception e) { + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + tracer.endOperation(false, "Goal target event validation threw: " + e.getMessage()); + } + } + throw e; + } // Add validation info to tracer if (tracerService != null) { RequestTracer tracer = tracerService.getCurrentTracer(); if (tracer != null && tracer.isEnabled()) { tracer.addValidationInfo(targetValidationErrors, "goal-target-event-validation"); - tracer.endOperation(!targetValidationErrors.isEmpty(), String.format("Goal target event validation completed with %d errors", targetValidationErrors.size())); + tracer.endOperation(targetValidationErrors.isEmpty(), String.format("Goal target event validation completed with %d errors", targetValidationErrors.size())); } } diff --git a/services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java index b321d8248f..8e9264c325 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java +++ b/services/src/main/java/org/apache/unomi/services/impl/rules/RulesServiceImpl.java @@ -694,14 +694,25 @@ protected void setRule(Rule rule, boolean allowInvalidRules) { // Validate condition (skips parameters with references/scripts) // Validation service will auto-resolve types if needed - List validationErrors = definitionsService.getConditionValidationService().validate(rule.getCondition()); + List validationErrors; + try { + validationErrors = definitionsService.getConditionValidationService().validate(rule.getCondition()); + } catch (Exception e) { + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + tracer.endOperation(false, "Rule validation threw: " + e.getMessage()); + } + } + throw e; + } // Add validation info to tracer if (tracerService != null) { RequestTracer tracer = tracerService.getCurrentTracer(); if (tracer != null && tracer.isEnabled()) { tracer.addValidationInfo(validationErrors, "rule-condition-validation"); - tracer.endOperation(!validationErrors.isEmpty(), String.format("Rule validation completed with %d errors", validationErrors.size())); + tracer.endOperation(validationErrors.isEmpty(), String.format("Rule validation completed with %d errors", validationErrors.size())); } } diff --git a/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java index be26b3434e..5500100ee7 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java +++ b/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java @@ -408,14 +408,25 @@ public void setSegmentDefinition(Segment segment) { // Validate condition (skips parameters with references/scripts) // Validation service will auto-resolve types if needed - List validationErrors = definitionsService.getConditionValidationService().validate(segment.getCondition()); + List validationErrors; + try { + validationErrors = definitionsService.getConditionValidationService().validate(segment.getCondition()); + } catch (Exception e) { + if (tracerService != null) { + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null && tracer.isEnabled()) { + tracer.endOperation(false, "Segment validation threw: " + e.getMessage()); + } + } + throw e; + } // Add validation info to tracer if (tracerService != null) { RequestTracer tracer = tracerService.getCurrentTracer(); if (tracer != null && tracer.isEnabled()) { tracer.addValidationInfo(validationErrors, "segment-condition-validation"); - tracer.endOperation(!validationErrors.isEmpty(), String.format("Segment validation completed with %d errors", validationErrors.size())); + tracer.endOperation(validationErrors.isEmpty(), String.format("Segment validation completed with %d errors", validationErrors.size())); } } @@ -661,22 +672,22 @@ public long getMatchingIndividualsCount(String segmentID) { } public Boolean isProfileInSegment(Profile profile, String segmentId) { - RequestTracer tracer = tracerService.getCurrentTracer(); - tracer.trace("Checking if profile is in segment: " + segmentId, profile.getItemId()); + if (tracerService != null && tracerService.isTracingEnabled()) { + tracerService.getCurrentTracer().trace("Checking if profile is in segment: " + segmentId, profile.getItemId()); + } Set matchingSegments = getSegmentsAndScoresForProfile(profile).getSegments(); boolean isInSegment = matchingSegments.contains(segmentId); - tracer.trace("Profile " + profile.getItemId() + " is " + (isInSegment ? "in" : "not in") + " segment: " + segmentId, profile.getItemId()); + if (tracerService != null && tracerService.isTracingEnabled()) { + tracerService.getCurrentTracer().trace("Profile " + profile.getItemId() + " is " + (isInSegment ? "in" : "not in") + " segment: " + segmentId, profile.getItemId()); + } return isInSegment; } public SegmentsAndScores getSegmentsAndScoresForProfile(Profile profile) { - RequestTracer tracer = tracerService.getCurrentTracer(); - tracer.trace("Getting segments and scores for profile: " + profile.getItemId(), profile.getItemId()); Set segments = new HashSet(); Map scores = new HashMap(); String currentTenant = contextManager.getCurrentContext().getTenantId(); - tracer.trace("Current tenant: " + currentTenant, profile.getItemId()); // Get system tenant segments and scoring first Map systemSegments = cacheService.getTenantCache("system", Segment.class); @@ -690,7 +701,9 @@ public SegmentsAndScores getSegmentsAndScoresForProfile(Profile profile) { } if (segment.getMetadata().isEnabled() && persistenceService.testMatch(segment.getCondition(), profile)) { segments.add(segment.getMetadata().getId()); - tracer.trace("Profile matches system segment: " + segment.getMetadata().getId(), profile.getItemId()); + if (tracerService != null && tracerService.isTracingEnabled()) { + tracerService.getCurrentTracer().trace("Profile matches system segment: " + segment.getMetadata().getId(), profile.getItemId()); + } } } } @@ -707,7 +720,9 @@ public SegmentsAndScores getSegmentsAndScoresForProfile(Profile profile) { } if (segment.getMetadata().isEnabled() && persistenceService.testMatch(segment.getCondition(), profile)) { segments.add(segment.getMetadata().getId()); - tracer.trace("Profile matches tenant segment: " + segment.getMetadata().getId(), profile.getItemId()); + if (tracerService != null && tracerService.isTracingEnabled()) { + tracerService.getCurrentTracer().trace("Profile matches tenant segment: " + segment.getMetadata().getId(), profile.getItemId()); + } } } } @@ -724,7 +739,6 @@ public SegmentsAndScores getSegmentsAndScoresForProfile(Profile profile) { } private void processScoring(Map scoringMap, Profile profile, Map scores) { - RequestTracer tracer = tracerService.getCurrentTracer(); Map scoreModifiers = (Map) profile.getSystemProperties().get("scoreModifiers"); for (Scoring scoring : scoringMap.values()) { if (scoring.getMetadata().isEnabled()) { diff --git a/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/TraceNode.java b/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/TraceNode.java index be75ec09ba..3224ca7dd1 100644 --- a/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/TraceNode.java +++ b/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/TraceNode.java @@ -18,6 +18,7 @@ import java.io.Serializable; import java.util.ArrayList; +import java.util.Collections; import java.util.List; /** @@ -27,8 +28,8 @@ public class TraceNode implements Serializable { private String operationType; private String description; - private Object context; - private Object result; + private String context; + private String result; private long startTime; private long endTime; private List traces; @@ -55,19 +56,19 @@ public void setDescription(String description) { this.description = description; } - public Object getContext() { + public String getContext() { return context; } - public void setContext(Object context) { + public void setContext(String context) { this.context = context; } - public Object getResult() { + public String getResult() { return result; } - public void setResult(Object result) { + public void setResult(String result) { this.result = result; } @@ -88,22 +89,30 @@ public void setEndTime(long endTime) { } public long getDuration() { - return endTime - startTime; + return Math.max(0, endTime - startTime); + } + + public void addTrace(String trace) { + this.traces.add(trace); } public List getTraces() { - return traces; + return Collections.unmodifiableList(traces); } public void setTraces(List traces) { this.traces = traces; } + public void addChild(TraceNode child) { + this.children.add(child); + } + public List getChildren() { - return children; + return Collections.unmodifiableList(children); } public void setChildren(List children) { this.children = children; } -} \ No newline at end of file +} \ No newline at end of file diff --git a/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/TracerService.java b/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/TracerService.java index a603be17e2..d450ede91d 100644 --- a/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/TracerService.java +++ b/tracing/tracing-api/src/main/java/org/apache/unomi/tracing/api/TracerService.java @@ -49,4 +49,10 @@ public interface TracerService { * @return the root trace node or null if tracing is not enabled */ TraceNode getTraceNode(); + + /** + * Release the ThreadLocal tracer for the current thread. Must be called at the end of + * every request (explain or not) in environments with thread pools to prevent class-loader leaks. + */ + void cleanup(); } \ No newline at end of file diff --git a/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultRequestTracer.java b/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultRequestTracer.java index d15d87b06b..7bab96e1e2 100644 --- a/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultRequestTracer.java +++ b/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultRequestTracer.java @@ -32,14 +32,15 @@ public class DefaultRequestTracer implements RequestTracer { private final ThreadLocal rootNode = new ThreadLocal<>(); private final ThreadLocal> nodeStack = ThreadLocal.withInitial(Stack::new); private static final int MAX_CONTEXT_STRING_LENGTH = 4096; + private static final int MAX_TRACE_DEPTH = 100; private static String safeContextToString(Object context) { if (context == null) { - return "null"; + return null; } try { String rendered = String.valueOf(context); - if (rendered != null && rendered.length() > MAX_CONTEXT_STRING_LENGTH) { + if (rendered.length() > MAX_CONTEXT_STRING_LENGTH) { return rendered.substring(0, MAX_CONTEXT_STRING_LENGTH) + "...(truncated)"; } return rendered; @@ -56,10 +57,14 @@ public void startOperation(String operationType, String description, Object cont return; } + if (rootNode.get() != null && nodeStack.get().size() >= MAX_TRACE_DEPTH) { + return; + } + TraceNode node = new TraceNode(); node.setOperationType(operationType); node.setDescription(description); - node.setContext(context); + node.setContext(safeContextToString(context)); node.setStartTime(System.currentTimeMillis()); if (rootNode.get() == null) { @@ -67,7 +72,7 @@ public void startOperation(String operationType, String description, Object cont currentNode.set(node); } else { TraceNode parent = currentNode.get(); - parent.getChildren().add(node); + parent.addChild(node); nodeStack.get().push(currentNode.get()); currentNode.set(node); } @@ -81,7 +86,7 @@ public void endOperation(Object result, String description) { TraceNode node = currentNode.get(); if (node != null) { - node.setResult(result); + node.setResult(safeContextToString(result)); node.setDescription(description); node.setEndTime(System.currentTimeMillis()); @@ -100,9 +105,9 @@ public void trace(String message, Object context) { TraceNode node = currentNode.get(); if (node != null) { if (context != null) { - node.getTraces().add(message + " - Context: " + safeContextToString(context)); + node.addTrace(message + " - Context: " + safeContextToString(context)); } else { - node.getTraces().add(message); + node.addTrace(message); } } } @@ -115,7 +120,7 @@ public void addValidationInfo(Collection validationMessages, String schemaId) TraceNode node = currentNode.get(); if (node != null) { - node.getTraces().add("Validation against schema " + schemaId + ": " + safeContextToString(validationMessages)); + node.addTrace("Validation against schema " + schemaId + ": " + safeContextToString(validationMessages)); } } diff --git a/tracing/tracing-impl/src/test/java/org/apache/unomi/tracing/impl/DefaultTracerServiceTest.java b/tracing/tracing-impl/src/test/java/org/apache/unomi/tracing/impl/DefaultTracerServiceTest.java index d808fc09d7..87577ae7ae 100644 --- a/tracing/tracing-impl/src/test/java/org/apache/unomi/tracing/impl/DefaultTracerServiceTest.java +++ b/tracing/tracing-impl/src/test/java/org/apache/unomi/tracing/impl/DefaultTracerServiceTest.java @@ -256,6 +256,31 @@ public void testNullHandling() { assertNull(node.getResult(), "Result should be null"); } + @Test + public void testEndOperationWithoutStartOperationShouldBeNoOp() { + tracerService.enableTracing(); + RequestTracer tracer = tracerService.getCurrentTracer(); + + tracer.endOperation("result", "desc"); + + assertNull(tracerService.getTraceNode(), "No trace node should exist when endOperation is called without startOperation"); + } + + @Test + public void testExtraEndOperationAfterBalancedPairUpdatesRootNode() { + tracerService.enableTracing(); + RequestTracer tracer = tracerService.getCurrentTracer(); + + tracer.startOperation("op", "description", null); + tracer.endOperation("r1", "first end"); + tracer.endOperation("r2", "second end"); + + TraceNode root = tracerService.getTraceNode(); + assertNotNull(root, "Root node should still exist after extra endOperation"); + assertEquals("second end", root.getDescription(), "Extra endOperation overwrites root description"); + assertEquals("r2", root.getResult(), "Extra endOperation overwrites root result"); + } + @Test public void testTraceShouldNotFailWhenContextToStringOverflowsStack() { tracerService.enableTracing(); From dd6e988299e3f7fc3ab866d1075c2a6bc9656390 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Wed, 17 Jun 2026 15:28:41 +0200 Subject: [PATCH 4/6] UNOMI-873: Fix inverted validationErrors check in DefinitionsServiceImpl tracer --- .../unomi/services/impl/definitions/DefinitionsServiceImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/src/main/java/org/apache/unomi/services/impl/definitions/DefinitionsServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/definitions/DefinitionsServiceImpl.java index db2cbd393f..3f828a8928 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/definitions/DefinitionsServiceImpl.java +++ b/services/src/main/java/org/apache/unomi/services/impl/definitions/DefinitionsServiceImpl.java @@ -596,7 +596,7 @@ public boolean resolveConditionType(Condition rootCondition) { RequestTracer tracer = tracerService.getCurrentTracer(); if (tracer != null && tracer.isEnabled()) { tracer.addValidationInfo(validationErrors, "condition-validation"); - tracer.endOperation(!validationErrors.isEmpty(), String.format("Validation completed with %d errors", validationErrors.size())); + tracer.endOperation(validationErrors.isEmpty(), String.format("Validation completed with %d errors", validationErrors.size())); } } From 9de9e0a603252a14233275d3a70e4787b99c10a6 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Wed, 17 Jun 2026 20:24:58 +0200 Subject: [PATCH 5/6] UNOMI-873: Fix tracing thread-safety, resource-leak and correctness issues - DefaultRequestTracer: add droppedOperations counter so endOperation() is a no-op for depth-limited spans instead of corrupting the node stack; add removeThreadLocals() to .remove() all 5 ThreadLocals on cleanup - DefaultTracerService.cleanup(): call removeThreadLocals() before removing the outer ThreadLocal to prevent OSGi classloader leaks - LoggingRequestTracer: override removeThreadLocals() to also remove indentLevel ThreadLocal - ContextJsonEndpoint / EventsCollectorEndpoint: move enableTracing() + startOperation() inside the try block (C1); guard disableTracing() and cleanup() in nested try/finally so neither can mask the original exception (H3); add LOGGER to EventsCollectorEndpoint - DefinitionsServiceImpl.resolveConditionType(): wrap validate() in try/catch so the open trace span is always closed on exception (H2) - SegmentServiceImpl: store getCurrentTracer() in local variable and null-check at all 4 call sites (M1) - GroovyActionDispatcher: make TracerService @Reference optional/dynamic to match the null-guard in execute() (M2); add unsetTracerService() - HttpServletRequestForwardWrapper: null-check getContext("/cxs") with clear 503 error (L2); guard sendError() with isCommitted() check and its own try/catch so secondary IOException is not swallowed (H4) --- .../actions/GroovyActionDispatcher.java | 8 ++++- .../rest/endpoints/ContextJsonEndpoint.java | 23 +++++++++----- .../endpoints/EventsCollectorEndpoint.java | 27 +++++++++++----- .../definitions/DefinitionsServiceImpl.java | 31 ++++++++++--------- .../impl/segments/SegmentServiceImpl.java | 20 +++++++++--- .../tracing/impl/DefaultRequestTracer.java | 19 +++++++++++- .../tracing/impl/DefaultTracerService.java | 4 +++ .../tracing/impl/LoggingRequestTracer.java | 6 ++++ .../HttpServletRequestForwardWrapper.java | 21 ++++++++++--- 9 files changed, 118 insertions(+), 41 deletions(-) diff --git a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java index 08eb876453..9abfbdc374 100644 --- a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java +++ b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java @@ -30,6 +30,8 @@ import org.apache.unomi.tracing.api.RequestTracer; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Reference; +import org.osgi.service.component.annotations.ReferenceCardinality; +import org.osgi.service.component.annotations.ReferencePolicy; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -73,11 +75,15 @@ public void setActionExecutorDispatcher(ActionExecutorDispatcher actionExecutorD this.actionExecutorDispatcher = actionExecutorDispatcher; } - @Reference + @Reference(cardinality = ReferenceCardinality.OPTIONAL, policy = ReferencePolicy.DYNAMIC) public void setTracerService(TracerService tracerService) { this.tracerService = tracerService; } + public void unsetTracerService(TracerService tracerService) { + this.tracerService = null; + } + public String getPrefix() { return GROOVY_PREFIX; } diff --git a/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java b/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java index b8beef26c6..04567327eb 100644 --- a/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java +++ b/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java @@ -161,12 +161,12 @@ public ContextResponse contextJSONAsPost(ContextRequest contextRequest, throw new ForbiddenException("Insufficient privileges to access tracing information"); } - if (explain) { - tracerService.enableTracing(); - tracerService.getCurrentTracer().startOperation("context-request", "Processing context request", contextRequest); - } - try { + if (explain) { + tracerService.enableTracing(); + tracerService.getCurrentTracer().startOperation("context-request", "Processing context request", contextRequest); + } + // Schema validation ObjectNode paramsAsJson = JsonNodeFactory.instance.objectNode(); paramsAsJson.put("personaId", personaId); @@ -221,10 +221,17 @@ public ContextResponse contextJSONAsPost(ContextRequest contextRequest, return contextResponse; } finally { - if (explain) { - tracerService.disableTracing(); + try { + if (explain) { + tracerService.disableTracing(); + } + } finally { + try { + tracerService.cleanup(); + } catch (RuntimeException e) { + LOGGER.warn("Failed to clean up tracer", e); + } } - tracerService.cleanup(); } } diff --git a/rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java b/rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java index 9117e40c9d..69cf0c84c6 100644 --- a/rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java +++ b/rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java @@ -29,6 +29,8 @@ import org.apache.unomi.utils.EventsRequestContext; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Reference; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -47,6 +49,8 @@ @Component(service = EventsCollectorEndpoint.class, property = "osgi.jaxrs.resource=true") public class EventsCollectorEndpoint { + private static final Logger LOGGER = LoggerFactory.getLogger(EventsCollectorEndpoint.class.getName()); + public static final String SYSTEMSCOPE = "systemscope"; @Reference private RestServiceUtils restServiceUtils; @@ -94,12 +98,12 @@ private EventCollectorResponse doEvent(EventsCollectorRequest eventsCollectorReq throw new ForbiddenException("Insufficient privileges to access tracing information"); } - if (explain) { - tracerService.enableTracing(); - tracerService.getCurrentTracer().startOperation("event-collection", "Processing event collection request", eventsCollectorRequest); - } - try { + if (explain) { + tracerService.enableTracing(); + tracerService.getCurrentTracer().startOperation("event-collection", "Processing event collection request", eventsCollectorRequest); + } + Date timestamp = new Date(); if (timestampAsLong != null) { timestamp = new Date(timestampAsLong); @@ -147,10 +151,17 @@ private EventCollectorResponse doEvent(EventsCollectorRequest eventsCollectorReq return response; } finally { - if (explain) { - tracerService.disableTracing(); + try { + if (explain) { + tracerService.disableTracing(); + } + } finally { + try { + tracerService.cleanup(); + } catch (RuntimeException e) { + LOGGER.warn("Failed to clean up tracer", e); + } } - tracerService.cleanup(); } } } diff --git a/services/src/main/java/org/apache/unomi/services/impl/definitions/DefinitionsServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/definitions/DefinitionsServiceImpl.java index 3f828a8928..57500696b0 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/definitions/DefinitionsServiceImpl.java +++ b/services/src/main/java/org/apache/unomi/services/impl/definitions/DefinitionsServiceImpl.java @@ -580,24 +580,27 @@ public boolean resolveConditionType(Condition rootCondition) { // Delegate to TypeResolutionService for resolution boolean resolved = typeResolutionService.resolveConditionType(rootCondition, "condition type " + rootCondition.getConditionTypeId()); if (resolved) { - // Start validation operation in tracer - if (tracerService != null) { - RequestTracer tracer = tracerService.getCurrentTracer(); - if (tracer != null && tracer.isEnabled()) { - tracer.startOperation("condition-validation", "Validating condition: " + rootCondition.getConditionTypeId(), rootCondition); - } + RequestTracer tracer = (tracerService != null) ? tracerService.getCurrentTracer() : null; + boolean tracerActive = tracer != null && tracer.isEnabled(); + + if (tracerActive) { + tracer.startOperation("condition-validation", "Validating condition: " + rootCondition.getConditionTypeId(), rootCondition); } // Validate the condition after resolving its type (validation service will auto-resolve if needed) - List validationErrors = conditionValidationService.validate(rootCondition); - - // Add validation info to tracer - if (tracerService != null) { - RequestTracer tracer = tracerService.getCurrentTracer(); - if (tracer != null && tracer.isEnabled()) { - tracer.addValidationInfo(validationErrors, "condition-validation"); - tracer.endOperation(validationErrors.isEmpty(), String.format("Validation completed with %d errors", validationErrors.size())); + List validationErrors; + try { + validationErrors = conditionValidationService.validate(rootCondition); + } catch (Exception e) { + if (tracerActive) { + tracer.endOperation(false, "Condition validation threw: " + e.getMessage()); } + throw e; + } + + if (tracerActive) { + tracer.addValidationInfo(validationErrors, "condition-validation"); + tracer.endOperation(validationErrors.isEmpty(), String.format("Validation completed with %d errors", validationErrors.size())); } // Separate errors and warnings diff --git a/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java index 5500100ee7..1ebbc0453c 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java +++ b/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java @@ -673,12 +673,18 @@ public long getMatchingIndividualsCount(String segmentID) { public Boolean isProfileInSegment(Profile profile, String segmentId) { if (tracerService != null && tracerService.isTracingEnabled()) { - tracerService.getCurrentTracer().trace("Checking if profile is in segment: " + segmentId, profile.getItemId()); + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null) { + tracer.trace("Checking if profile is in segment: " + segmentId, profile.getItemId()); + } } Set matchingSegments = getSegmentsAndScoresForProfile(profile).getSegments(); boolean isInSegment = matchingSegments.contains(segmentId); if (tracerService != null && tracerService.isTracingEnabled()) { - tracerService.getCurrentTracer().trace("Profile " + profile.getItemId() + " is " + (isInSegment ? "in" : "not in") + " segment: " + segmentId, profile.getItemId()); + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null) { + tracer.trace("Profile " + profile.getItemId() + " is " + (isInSegment ? "in" : "not in") + " segment: " + segmentId, profile.getItemId()); + } } return isInSegment; } @@ -702,7 +708,10 @@ public SegmentsAndScores getSegmentsAndScoresForProfile(Profile profile) { if (segment.getMetadata().isEnabled() && persistenceService.testMatch(segment.getCondition(), profile)) { segments.add(segment.getMetadata().getId()); if (tracerService != null && tracerService.isTracingEnabled()) { - tracerService.getCurrentTracer().trace("Profile matches system segment: " + segment.getMetadata().getId(), profile.getItemId()); + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null) { + tracer.trace("Profile matches system segment: " + segment.getMetadata().getId(), profile.getItemId()); + } } } } @@ -721,7 +730,10 @@ public SegmentsAndScores getSegmentsAndScoresForProfile(Profile profile) { if (segment.getMetadata().isEnabled() && persistenceService.testMatch(segment.getCondition(), profile)) { segments.add(segment.getMetadata().getId()); if (tracerService != null && tracerService.isTracingEnabled()) { - tracerService.getCurrentTracer().trace("Profile matches tenant segment: " + segment.getMetadata().getId(), profile.getItemId()); + RequestTracer tracer = tracerService.getCurrentTracer(); + if (tracer != null) { + tracer.trace("Profile matches tenant segment: " + segment.getMetadata().getId(), profile.getItemId()); + } } } } diff --git a/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultRequestTracer.java b/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultRequestTracer.java index 7bab96e1e2..2021adff8d 100644 --- a/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultRequestTracer.java +++ b/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultRequestTracer.java @@ -31,6 +31,7 @@ public class DefaultRequestTracer implements RequestTracer { private final ThreadLocal currentNode = new ThreadLocal<>(); private final ThreadLocal rootNode = new ThreadLocal<>(); private final ThreadLocal> nodeStack = ThreadLocal.withInitial(Stack::new); + private final ThreadLocal droppedOperations = ThreadLocal.withInitial(() -> 0); private static final int MAX_CONTEXT_STRING_LENGTH = 4096; private static final int MAX_TRACE_DEPTH = 100; @@ -58,6 +59,7 @@ public void startOperation(String operationType, String description, Object cont } if (rootNode.get() != null && nodeStack.get().size() >= MAX_TRACE_DEPTH) { + droppedOperations.set(droppedOperations.get() + 1); return; } @@ -84,6 +86,12 @@ public void endOperation(Object result, String description) { return; } + int dropped = droppedOperations.get(); + if (dropped > 0) { + droppedOperations.set(dropped - 1); + return; + } + TraceNode node = currentNode.get(); if (node != null) { node.setResult(safeContextToString(result)); @@ -146,6 +154,15 @@ public void setEnabled(boolean enabled) { public void reset() { rootNode.remove(); currentNode.remove(); - nodeStack.get().clear(); + nodeStack.remove(); + droppedOperations.set(0); + } + + void removeThreadLocals() { + enabled.remove(); + currentNode.remove(); + rootNode.remove(); + nodeStack.remove(); + droppedOperations.remove(); } } diff --git a/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultTracerService.java b/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultTracerService.java index 43e24aa45f..50cb822ec1 100644 --- a/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultTracerService.java +++ b/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultTracerService.java @@ -64,6 +64,10 @@ public TraceNode getTraceNode() { } public void cleanup() { + RequestTracer tracer = currentTracer.get(); + if (tracer instanceof DefaultRequestTracer) { + ((DefaultRequestTracer) tracer).removeThreadLocals(); + } currentTracer.remove(); } } \ No newline at end of file diff --git a/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/LoggingRequestTracer.java b/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/LoggingRequestTracer.java index 34b7ec0e0b..4cf5fad1fa 100644 --- a/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/LoggingRequestTracer.java +++ b/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/LoggingRequestTracer.java @@ -73,6 +73,12 @@ public void reset() { indentLevel.set(0); } + @Override + void removeThreadLocals() { + super.removeThreadLocals(); + indentLevel.remove(); + } + /** * Format a message for operation start. * diff --git a/web-servlets/src/main/java/org/apache/unomi/web/servlets/HttpServletRequestForwardWrapper.java b/web-servlets/src/main/java/org/apache/unomi/web/servlets/HttpServletRequestForwardWrapper.java index b801754f42..9927e2282f 100644 --- a/web-servlets/src/main/java/org/apache/unomi/web/servlets/HttpServletRequestForwardWrapper.java +++ b/web-servlets/src/main/java/org/apache/unomi/web/servlets/HttpServletRequestForwardWrapper.java @@ -47,14 +47,25 @@ public HttpServletRequestForwardWrapper(HttpServletRequest request) { */ public static void forward(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { try { + javax.servlet.ServletContext cxsContext = request.getServletContext().getContext("/cxs"); + if (cxsContext == null) { + LOGGER.error("Could not obtain /cxs servlet context — ensure cross-context dispatch is enabled and the cxs bundle is deployed"); + if (!response.isCommitted()) { + response.sendError(HttpServletResponse.SC_SERVICE_UNAVAILABLE, "Service unavailable"); + } + return; + } HttpServletRequest requestWrapper = new HttpServletRequestForwardWrapper(request); - requestWrapper.getServletContext() - .getContext("/cxs") - .getRequestDispatcher("/cxs" + request.getRequestURI()) - .forward(requestWrapper, response); + cxsContext.getRequestDispatcher("/cxs" + request.getRequestURI()).forward(requestWrapper, response); } catch (Throwable t) { // Here in order to return generic message instead of the whole stack trace in case of not caught exception LOGGER.error("HttpServletRequestForwardWrapper failed to forward the request", t); - response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "Internal server error"); + if (!response.isCommitted()) { + try { + response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "Internal server error"); + } catch (IOException ioe) { + LOGGER.warn("Could not send error response after forward failure", ioe); + } + } } } From a78ea7585f32f0434013ad61a552fb1601352f30 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Wed, 17 Jun 2026 21:09:47 +0200 Subject: [PATCH 6/6] UNOMI-873: Fix DefaultRequestTracer lifecycle and ThreadLocal leak issues - DefaultTracerService: make getCurrentTracer() lazily allocate so non-explain requests never create a DefaultRequestTracer; make isTracingEnabled(), getTraceNode() and disableTracing() read currentTracer.get() directly to avoid triggering allocation - ContextJsonEndpoint / EventsCollectorEndpoint: null-guard tracerService in finally blocks to avoid silent ThreadLocal leak if the OSGi service is unbound mid-request - ConditionContextHelper: add isTracingEnabled() guard before the three getCurrentTracer() calls on error paths, preventing tracer creation on background threads that have no matching cleanup() call --- .../conditions/ConditionContextHelper.java | 6 ++-- .../rest/endpoints/ContextJsonEndpoint.java | 6 ++-- .../endpoints/EventsCollectorEndpoint.java | 6 ++-- .../tracing/impl/DefaultTracerService.java | 28 +++++++++++-------- 4 files changed, 27 insertions(+), 19 deletions(-) diff --git a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelper.java b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelper.java index 8e0b083138..5e96d2d6dd 100644 --- a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelper.java +++ b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelper.java @@ -243,7 +243,7 @@ private static Object parseParameterWithValidationRecursive( "Possible infinite chain or very deep nesting. Resolution chain: %s", MAX_RESOLUTION_DEPTH, resolutionChain); LOGGER.error(message); - if (tracerService != null) { + if (tracerService != null && tracerService.isTracingEnabled()) { RequestTracer tracer = tracerService.getCurrentTracer(); if (tracer != null && tracer.isEnabled()) { Map traceContext = new HashMap<>(); @@ -266,7 +266,7 @@ private static Object parseParameterWithValidationRecursive( "Circular parameter reference detected: %s. Resolution chain: %s", referenceKey, resolutionChain); LOGGER.warn(message); - if (tracerService != null) { + if (tracerService != null && tracerService.isTracingEnabled()) { RequestTracer tracer = tracerService.getCurrentTracer(); if (tracer != null && tracer.isEnabled()) { Map traceContext = new HashMap<>(); @@ -393,7 +393,7 @@ private static void validateParameterType( LOGGER.warn(message + " (value: {})", resolvedValue); - if (tracerService != null) { + if (tracerService != null && tracerService.isTracingEnabled()) { RequestTracer tracer = tracerService.getCurrentTracer(); if (tracer != null && tracer.isEnabled()) { Map traceContext = new HashMap<>(); diff --git a/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java b/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java index 04567327eb..bb234b9b28 100644 --- a/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java +++ b/rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java @@ -222,12 +222,14 @@ public ContextResponse contextJSONAsPost(ContextRequest contextRequest, return contextResponse; } finally { try { - if (explain) { + if (explain && tracerService != null) { tracerService.disableTracing(); } } finally { try { - tracerService.cleanup(); + if (tracerService != null) { + tracerService.cleanup(); + } } catch (RuntimeException e) { LOGGER.warn("Failed to clean up tracer", e); } diff --git a/rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java b/rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java index 69cf0c84c6..6945b03b63 100644 --- a/rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java +++ b/rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java @@ -152,12 +152,14 @@ private EventCollectorResponse doEvent(EventsCollectorRequest eventsCollectorReq return response; } finally { try { - if (explain) { + if (explain && tracerService != null) { tracerService.disableTracing(); } } finally { try { - tracerService.cleanup(); + if (tracerService != null) { + tracerService.cleanup(); + } } catch (RuntimeException e) { LOGGER.warn("Failed to clean up tracer", e); } diff --git a/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultTracerService.java b/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultTracerService.java index 50cb822ec1..9d2d6d69d6 100644 --- a/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultTracerService.java +++ b/tracing/tracing-impl/src/main/java/org/apache/unomi/tracing/impl/DefaultTracerService.java @@ -27,16 +27,16 @@ @Component(service = TracerService.class, immediate = true) public class DefaultTracerService implements TracerService { - private final ThreadLocal currentTracer = new ThreadLocal() { - @Override - protected RequestTracer initialValue() { - return new DefaultRequestTracer(); - } - }; + private final ThreadLocal currentTracer = new ThreadLocal<>(); @Override public RequestTracer getCurrentTracer() { - return currentTracer.get(); + RequestTracer tracer = currentTracer.get(); + if (tracer == null) { + tracer = new DefaultRequestTracer(); + currentTracer.set(tracer); + } + return tracer; } @Override @@ -48,19 +48,23 @@ public void enableTracing() { @Override public void disableTracing() { - RequestTracer tracer = getCurrentTracer(); - tracer.setEnabled(false); - tracer.reset(); + RequestTracer tracer = currentTracer.get(); + if (tracer != null) { + tracer.setEnabled(false); + tracer.reset(); + } } @Override public boolean isTracingEnabled() { - return getCurrentTracer().isEnabled(); + RequestTracer tracer = currentTracer.get(); + return tracer != null && tracer.isEnabled(); } @Override public TraceNode getTraceNode() { - return getCurrentTracer().getTraceNode(); + RequestTracer tracer = currentTracer.get(); + return tracer != null ? tracer.getTraceNode() : null; } public void cleanup() {