sergehuber commented on code in PR #775:
URL: https://github.com/apache/unomi/pull/775#discussion_r3428513705
##########
rest/src/main/java/org/apache/unomi/rest/endpoints/EventsCollectorEndpoint.java:
##########
@@ -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");
+ }
Review Comment:
Fixed: `endOperation()` is now called before `getTraceNode()` in both
endpoints. The root `TraceNode` now has its final `endTime`, `description`, and
`result` set before the trace is captured into the response. Same fix applied
to `ContextJsonEndpoint`.
##########
rest/src/main/java/org/apache/unomi/rest/endpoints/ContextJsonEndpoint.java:
##########
@@ -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");
+ }
Review Comment:
Fixed: `endOperation()` is now called first so the root `TraceNode` has its
final `endTime`, `description`, and `result` set before `getTraceNode()`
captures the trace into the response. Same fix applied to
`EventsCollectorEndpoint`.
##########
services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java:
##########
@@ -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<ValidationError> validationErrors =
definitionsService.getConditionValidationService().validate(segment.getCondition());
- if (definitionsService.getConditionValidationService() !=
null) {
- List<ValidationError> 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()));
+ }
Review Comment:
Fixed: Changed `!validationErrors.isEmpty()` to `validationErrors.isEmpty()`
in the `endOperation` call. The tracer result now correctly reports `true`
(success) when validation produces no errors, and `false` when there are errors.
##########
services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java:
##########
@@ -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");
Review Comment:
Good catch — this is a pre-existing issue that predates the tracing
additions in this PR. The raw `(Integer)` cast was there before and the tracing
code in this PR just added visibility into the method. Will be tracked as a
separate fix to replace the cast with `PropertyHelper.getInteger(...)` coercion.
--
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]
