Add datetime-aware time filter and select functions to JfrPath#87
Add datetime-aware time filter and select functions to JfrPath#87
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…fore/after/between arg handling Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Combined JUnit Test Report
HTML Test ReportsRun artifacts: https://github.com/btraceio/jafar/actions/runs/22456165320
|
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This pull request adds comprehensive datetime-aware time filtering and formatting capabilities to JfrPath, fundamentally changing how timestamps are handled throughout the system. The key architectural change is normalizing timestamps from JFR ticks to epoch nanoseconds at parse time, simplifying all downstream time operations.
Changes:
- Timestamps normalized at parse time:
startTimeanddurationfields converted from JFR ticks to epoch nanoseconds by value builders, enabling consistent time handling - New time filter predicates:
before(),after(),on(), and extendedbetween()to accept datetime strings in addition to numeric values - New datetime functions:
asDateTime()for formatting timestamps,truncate()for time-series bucketing, andformatDuration()for human-readable duration display - Simplified
timerange()operator: removed chunk metadata dependency, outputs now useminEpochNanos/maxEpochNanosinstead ofminTicks/maxTicks - Comprehensive test coverage and documentation updates
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| parser-core/src/main/java/io/jafar/parser/impl/MapValueBuilder.java | Adds timestamp normalization in onComplexValueEnd to convert ticks to epoch nanos |
| parser-core/src/main/java/io/jafar/parser/impl/MapValueBuilderBaseline.java | Adds timestamp normalization for baseline implementation |
| parser-core/src/main/java/io/jafar/parser/impl/LazyMapValueBuilder.java | Adds timestamp normalization for lazy map implementation |
| parser-core/src/main/java/io/jafar/parser/impl/ChunkInfoImpl.java | Implements asEpochNanos() method to support timestamp conversion |
| parser-core/src/main/java/io/jafar/parser/api/Control.java | Adds asEpochNanos() API to ChunkInfo interface |
| parser-core/src/test/java/io/jafar/parser/UntypedJafarParserTest.java | Updates test to use normalized epoch nanos directly |
| jfr-shell-core/src/main/java/io/jafar/shell/jfrpath/JfrPathEvaluator.java | Implements new filter predicates, datetime functions, and simplified timerange |
| jfr-shell-core/src/main/java/io/jafar/shell/jfrpath/JfrPathParser.java | Adds parsing support for asDateTime and formatDuration operators |
| jfr-shell-core/src/main/java/io/jafar/shell/jfrpath/JfrPath.java | Defines AsDateTimeOp and FormatDurationOp pipeline operator classes |
| jfr-shell-core/src/test/java/io/jafar/shell/jfrpath/JfrPathTimeRangeTest.java | Updates tests for renamed output keys (minTicks → minEpochNanos) |
| jfr-shell-core/src/test/java/io/jafar/shell/jfrpath/JfrPathDateTimeFunctionsTest.java | Comprehensive test coverage for all new datetime functions |
| jfr-shell/src/main/java/io/jafar/shell/cli/completion/FunctionRegistry.java | Registers new functions and updates between() parameter types |
| jfr-shell/src/test/java/io/jafar/shell/cli/completion/FunctionRegistryTest.java | Updates tests for new functions and changed parameter types |
| doc/cli/JFRPath.md | Documents all new time filter functions and datetime operations with examples |
Comments suppressed due to low confidence (6)
jfr-shell-core/src/main/java/io/jafar/shell/jfrpath/JfrPathEvaluator.java:954
- The
getFormattermethod callsDateTimeFormatter.ofPattern(format)which can throwIllegalArgumentExceptionif the format string is invalid. This exception will propagate to the user without a clear error message indicating that the datetime format pattern is invalid.
Consider wrapping this in a try-catch block to provide a more user-friendly error message that clearly indicates the issue is with the format string, e.g., "Invalid datetime format pattern: '{format}'".
private static DateTimeFormatter getFormatter(String format) {
if (format == null || format.isEmpty()) {
return DateTimeFormatter.ISO_LOCAL_DATE_TIME;
}
return DateTimeFormatter.ofPattern(format);
}
jfr-shell-core/src/main/java/io/jafar/shell/jfrpath/JfrPathEvaluator.java:1172
- The
truncatefunction with "week" unit usespreviousOrSame(DayOfWeek.MONDAY)which assumes weeks start on Monday. This is ISO-8601 compliant, but may not match user expectations in locales where weeks start on Sunday (e.g., US). Consider documenting this behavior explicitly in the JFRPath.md documentation, or adding a note that week truncation follows ISO-8601 (Monday as first day of week).
case "week" ->
zdt.with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY))
.truncatedTo(ChronoUnit.DAYS);
jfr-shell-core/src/main/java/io/jafar/shell/jfrpath/JfrPathEvaluator.java:2245
- The
onfilter callsLocalDate.parse()which can throwDateTimeParseExceptionif the date string is not in the expected "yyyy-MM-dd" format. This exception will propagate to the user without a clear error message.
Consider wrapping this in a try-catch block to provide a more user-friendly error message, e.g., "Invalid date format for on() filter. Expected yyyy-MM-dd, got: '{value}'".
LocalDate date = LocalDate.parse(String.valueOf(resolveArg(root, args.get(1))));
jfr-shell-core/src/main/java/io/jafar/shell/jfrpath/JfrPathEvaluator.java:2217
- The
betweenfunction implementation has an issue when mixing numeric and datetime string bounds. Currently, if both bounds are strings, it uses long comparison withparseEpochNanos. However, if only one bound is a string (e.g.,between(startTime, 1000, "2024-08-13")), it falls through to the double comparison path which would incorrectly interpret the datetime string as a number viatoDouble(). This could lead to parsing errors or incorrect comparisons.
Consider handling the case where either (but not both) bounds are strings, or document that both bounds must be of the same type (both strings or both numbers).
case "between" -> {
ensureArgs(args, 3);
Object v = resolveArg(root, args.get(0));
if (!(v instanceof Number betweenNum)) return false;
Object loArg = resolveArg(root, args.get(1));
Object hiArg = resolveArg(root, args.get(2));
if (loArg instanceof String && hiArg instanceof String) {
long x = betweenNum.longValue();
return x >= parseEpochNanos(String.valueOf(loArg))
&& x <= parseEpochNanos(String.valueOf(hiArg));
}
double x = betweenNum.doubleValue();
return x >= toDouble(loArg) && x <= toDouble(hiArg);
}
jfr-shell-core/src/main/java/io/jafar/shell/jfrpath/JfrPathEvaluator.java:947
- The
formatDurationmethod doesn't handle negative duration values. If a negative nanosecond value is passed (which could happen due to data corruption or incorrect calculation), the method will produce incorrect output. For example, negative values less than -1000 will fall through to the first condition and be formatted as "-500ns", but larger negative values could produce confusing output like "-5m -30s".
Consider adding a check at the beginning of the method to handle negative values explicitly, either by returning a special string like "invalid" or by formatting them with a negative sign prefix.
private static String formatDuration(long nanos) {
if (nanos < 1_000) {
return nanos + "ns";
} else if (nanos < 1_000_000) {
return String.format("%.2fus", nanos / 1_000.0);
} else if (nanos < 1_000_000_000) {
return String.format("%.2fms", nanos / 1_000_000.0);
} else if (nanos < 60_000_000_000L) {
return String.format("%.2fs", nanos / 1_000_000_000.0);
} else if (nanos < 3_600_000_000_000L) {
long totalSecs = nanos / 1_000_000_000L;
long mins = totalSecs / 60;
long secs = totalSecs % 60;
return String.format("%dm %ds", mins, secs);
} else {
long totalSecs = nanos / 1_000_000_000L;
long hours = totalSecs / 3600;
long mins = (totalSecs % 3600) / 60;
long secs = totalSecs % 60;
return String.format("%dh %dm %ds", hours, mins, secs);
}
}
jfr-shell-core/src/main/java/io/jafar/shell/jfrpath/JfrPathEvaluator.java:2303
- Potential overflow issue when converting
Instantto epoch nanoseconds. The calculationi.getEpochSecond() * 1_000_000_000L + i.getNano()can overflow for dates beyond year 2262 (when epoch seconds exceed Long.MAX_VALUE / 1_000_000_000). While JFR recordings are unlikely to have such far-future timestamps, parsing user-supplied datetime strings in filters likebefore(),after(), orbetween()could potentially trigger this if invalid dates are provided.
Consider adding overflow checks or catching ArithmeticException to provide a clearer error message, or document the supported date range.
private static long parseEpochNanos(String s) {
// ISO instant with timezone offset, e.g. "2024-08-13T16:24:00Z"
try {
Instant i = Instant.parse(s);
return i.getEpochSecond() * 1_000_000_000L + i.getNano();
} catch (DateTimeParseException ignored) {
}
// Local date-time without timezone, e.g. "2024-08-13T16:24:00"
try {
Instant i = LocalDateTime.parse(s).atZone(ZoneId.systemDefault()).toInstant();
return i.getEpochSecond() * 1_000_000_000L + i.getNano();
} catch (DateTimeParseException ignored) {
}
// Date only, e.g. "2024-08-13" — treated as start of day in local zone
try {
Instant i = LocalDate.parse(s).atStartOfDay(ZoneId.systemDefault()).toInstant();
return i.getEpochSecond() * 1_000_000_000L + i.getNano();
} catch (DateTimeParseException ignored) {
}
throw new IllegalArgumentException("Cannot parse datetime: " + s);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
startTimeanddurationare converted from JFR ticks to epoch nanoseconds byMapValueBuilder/MapValueBuilderBaselineusing the newControl.ChunkInfo.asEpochNanos()API, so all downstream query code works with wall-clock valuesasDateTime([path][, format=...]): New pipeline operator andselect()function that formats epoch-nanosecond fields as human-readable datetime stringstruncate(field, unit): Newselect()function that truncates an epoch-nanosecond value to a time boundary (second/minute/hour/day/week/month), enabling time-seriesgroupByformatDuration(field): New pipeline operator andselect()function that formats a nanosecond duration as a human-readable string (123ns,4.56ms,2h 15m 30s)before(field, datetime)/after(field, datetime): New filter predicates for time-based filtering with datetime string parsing (ISO-8601, local datetime, date-only)on(field, "yyyy-MM-dd"): New filter predicate that matches events on a specific calendar date (local timezone)between()datetime support: Extended to accept datetime strings as bounds in addition to numberstimerange()simplified: Removed chunk-metadata dependency; uses normalized epoch-nanos directly; output keys renamed fromminTicks/maxTickstominEpochNanos/maxEpochNanosJFRPath.md,FunctionRegistry.java,FunctionRegistryTest.java, andJfrPathDateTimeFunctionsTest.javaupdated for all new functionsTest plan
./gradlew testto verify all existing and new tests passbefore,after,on,between,asDateTime,truncate,formatDurationevents/jdk.ExecutionSample[on(startTime, "yyyy-MM-dd")]with a real recordingevents/jdk.ExecutionSample | select(asDateTime(truncate(startTime, "minute")) as bucket) | groupBy(bucket, agg=count)events/jdk.JavaMonitorEnter | select(startTime, formatDuration(duration) as dur)formatDurationas pipeline op:events/jdk.JavaMonitorEnter/duration | formatDuration()🤖 Generated with Claude Code