sergehuber commented on code in PR #775:
URL: https://github.com/apache/unomi/pull/775#discussion_r3428518943
##########
plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/SetEventOccurenceCountAction.java:
##########
@@ -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<Condition> conditions = new ArrayList<Condition>();
-
- 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<Condition> conditions = new ArrayList<Condition>();
- 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);
+
Review Comment:
Pre-existing issue not introduced by this PR —
`conditions.add(eventCondition)` was already outside the null-guard before the
tracing changes. Will be tracked as a separate fix to move it inside the `if
(eventCondition != null)` block.
##########
plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/actions/SetEventOccurenceCountAction.java:
##########
@@ -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<Condition> conditions = new ArrayList<Condition>();
-
- 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<Condition> conditions = new ArrayList<Condition>();
- 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");
Review Comment:
Pre-existing issue not introduced by this PR — the raw `(Integer)` cast
predates the tracing additions. Will be tracked as a separate fix to use
`PropertyHelper.getInteger(...)` coercion instead.
##########
services/src/main/java/org/apache/unomi/services/impl/definitions/DefinitionsServiceImpl.java:
##########
@@ -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<ValidationError> 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()));
+ }
Review Comment:
Fixed in follow-up commit dd6e98829: changed `!validationErrors.isEmpty()`
to `validationErrors.isEmpty()`. This was missed in the initial fix batch
alongside the same correction in `SegmentServiceImpl`, `RulesServiceImpl`, and
`GoalsServiceImpl`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
