[OPIK-2856] [BE] Implement UUIDv7 time-based filtering for traces by thiagohora · Pull Request #3921 · comet-ml/opik

thiagohora · 2025-11-03T16:12:22Z

Details

This PR implements UUIDv7 time-based filtering for the /v1/private/traces and /v1/private/traces/stats API endpoints. The feature enables efficient querying of traces within specific time ranges by leveraging the time-encoded nature of UUIDv7 identifiers.

Implementation

New Components:

InstantToUUIDMapper: Utility class to convert Instant timestamps to UUIDv7 lower and upper bounds for database queries
InstantParamConverter: JAX-RS ParamConverterProvider to automatically parse ISO-8601 and epoch millisecond time parameters

Modified Components:

TracesResource: Added from_time and to_time parameters to both /traces and /traces/stats endpoints with validation
TraceDAO: Added UUID-based time filtering using SQL BETWEEN clause on the id column
TraceSearchCriteria: Added uuidFromTime and uuidToTime fields to support time filtering
ValidationUtils: Added validateTimeRangeParameters method to enforce time range validation rules

Test Coverage:

Comprehensive integration tests covering boundary conditions (traces at exact bounds, within bounds, outside bounds)
ISO-8601 and epoch millisecond format parsing tests
Time range validation tests (missing parameters, invalid ranges)
All tests passing: 10/10 time filtering tests + 2 validation tests

How It Works

The client sends from_time and to_time query parameters in ISO-8601 or epoch millisecond format
InstantParamConverter automatically converts these to Instant objects
TracesResource validates the time range and converts Instant timestamps to UUIDv7 bounds
TraceDAO applies the UUID-based filter using SQL BETWEEN clause
The database returns only traces created within the specified time range

Testing

✅ All 10 time filtering tests passing
✅ All 2 validation tests passing
✅ Comprehensive boundary condition coverage
✅ Format parsing validation (ISO-8601 and epoch milliseconds)

Change checklist

User facing
Documentation update

Issues

Resolves OPIK-2856

Testing

Scenarios covered:

Traces at exact lower bound, upper bound, and between bounds (all included)
Traces outside bounds (correctly excluded)
ISO-8601 format parsing with extended time ranges
Epoch millisecond format support
Validation: Missing from_time parameter returns 400
Validation: Missing to_time parameter returns 400
Validation: from_time after to_time returns 400

How to verify:

cd apps/opik-backend
mvn test -Dtest=GetTracesByProjectResourceTest\$GetTracesByProjectTimeFilteringTests

Documentation

NA

- Add InstantToUUIDMapper to convert Instant timestamps to UUIDv7 bounds - Add InstantParamConverter to parse ISO-8601 and epoch millisecond time parameters - Update TracesResource to accept from_time and to_time parameters on /traces and /traces/stats endpoints - Update TraceDAO to apply UUID-based time filtering using BETWEEN clause on id column - Update TraceSearchCriteria to include uuidFromTime and uuidToTime fields - Add comprehensive integration tests for time filtering with boundary conditions - All tests passing: 10/10 time filtering tests + validation tests

Copilot

Pull Request Overview

This PR implements UUIDv7 time-based filtering for the /v1/private/traces and /v1/private/traces/stats endpoints, enabling efficient querying of traces within specific time ranges. The implementation leverages the time-encoded nature of UUIDv7 identifiers and includes comprehensive test coverage.

Key Changes:

New time-based filtering capability using UUIDv7 boundaries for improved query performance
Automatic conversion of ISO-8601 and epoch millisecond timestamps via JAX-RS parameter converter
Time range validation to ensure both parameters are provided together and in the correct order

Reviewed Changes

Copilot reviewed 52 out of 54 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`MySQLContainerUtils.java`	Updated MySQL container type to use newer testcontainers API
`InstantParamConverter.java`	New converter for automatic Instant parsing from query parameters
`InstantToUUIDMapperTest.java`	Comprehensive unit tests for UUIDv7 boundary generation
`TraceSearchCriteria.java`	Added UUID time boundary fields for filtering
`ValidationUtils.java`	New validation for time range parameters
`OpikApplication.java`	Registered InstantParamConverter with Jersey
`TraceResourceClient.java`	Added support for time query parameters in test client
`TracesResourceTest.java`	Removed large nested test class that was moved elsewhere
`TraceAssertions.java`	Moved EXCLUDE_FUNCTIONS map from test class to utility
Multiple test files	Updated MySQL container imports to use new package

@SuppressWarnings

…d type safety - Fix InstantParamConverter to catch specific DateTimeParseException instead of generic Exception - Add debug logging when falling back to epoch milliseconds parsing - Refactor anonymous ParamConverter class to named InstantConverter inner class for clarity - Suppress unchecked cast with @SuppressWarnings annotation - Fix MySQLContainerUtils return type to use MySQLContainer<?> for type safety

- Update tests to reflect that toUpperBound uses next millisecond (+1ms) for inclusive BETWEEN queries - Remove outdated assertions expecting same timestamp in both bounds - Verify upper bound is lexicographically greater than lower bound - All 13 tests now passing

github-actions · 2025-11-03T17:16:34Z

Backend Tests Results

5 320 tests 5 313 ✅ 51m 36s ⏱️
270 suites 7 💤
270 files 0 ❌

Results for commit bdd2249.

♻️ This comment has been updated with latest results.

github-actions · 2025-11-04T09:15:54Z

SDK E2E Tests Results

0 tests ±0 0 ✅ ±0 0s ⏱️ ±0s
0 suites ±0 0 💤 ±0
0 files ±0 0 ❌ ±0

Results for commit 551e9b8. ± Comparison against base commit a3698fa.

♻️ This comment has been updated with latest results.

andrescrz

Let me review this carefully before moving forward, we have some overlap between our custom convertOtelIdToUUIDv7 and the library one getTimeOrderedEpoch.

I'll do it in a sec.

andrescrz

The general idea LGTM.

Before moving forward, let's double check the choice of library to perform the conversion of a timestamp towards a UUID v7.

We have some duplications here and I'd rather rely on a stable library implementation.

andrescrz · 2025-11-04T10:07:58Z

+ */
+@UtilityClass
+@Slf4j
+public class InstantToUUIDMapper {


For this, I'd rather have a single source of truth in IdGenerator. See getTimeOrderedEpoch method which relies on our library com.fasterxml.uuid.java-uuid-generator, specifically on method

Initially, I'd rather rely on a reliable library implementation, than re-inventing the wheel ourselves. That way we don't need to maintain the implementation ourselves.

I propose the following:

Double check that IdGenerator.getTimeOrderedEpoch (based on TimeBasedEpochGenerator.construct.

If all good, use it here.

Optionally substitute our own implementation from OpenTelemetryMapper.convertOtelIdToUUIDv7.

Let me know how this sounds.

Hi @andrescrz,

Thanks for your comment. On why not use getTimeOrderedEpoch:

While IdGenerator.getTimeOrderedEpoch() also creates UUIDs from timestamps using the java-uuid-generator library, it uses random bits for the lower 80 bits of the UUID. For time-range queries, we need:

Lower bound: UUID with ALL ZEROS for random bits (lexicographically smallest UUID at this timestamp)

Upper bound: UUID at (timestamp+1ms) with ALL ZEROS (first UUID AFTER the end time)

This ensures that BETWEEN queries correctly include all traces within the specified time range. IdGenerator's random bits would make the bounds non-deterministic, breaking BETWEEN semantics.

I remember discussing this with @BorisTkachenko and I raised the same concerns, but we came to the conclusion that getTimeOrderedEpoch works fine in practice. Some reasons:

From the UUID v7 RFC, the sub-millisecs 12 bits are optional and depend on the implementation.

Our implementation uses monotonic values instead, so granularly is actually by millisecs.

As you use start and end interval, just with the semantics and/or increasing/decreasing 1 millisecs, you would get correct results if this was an issue (it isn't).

You already do +1 millisec.

With all this in mind, I still recommend going with getTimeOrderedEpoch. Some queries based on getTimeOrderedEpoch are already in place and working for months, without bug reports so far.

But not concern if you prefer to move forward. This would be just a small piece of tech debt to centralise and remove duplications in the future (if we ever do it).

Let me double check

andrescrz

Left some reasoning on why I still recommend the other approach, but not a blocker. It's just a small piece of tech debt in my view. You can move forward.

andrescrz · 2025-11-04T11:18:09Z

+ */
+@UtilityClass
+@Slf4j
+public class InstantToUUIDMapper {


I remember discussing this with @BorisTkachenko and I raised the same concerns, but we came to the conclusion that getTimeOrderedEpoch works fine in practice. Some reasons:

From the UUID v7 RFC, the sub-millisecs 12 bits are optional and depend on the implementation.

Our implementation uses monotonic values instead, so granularly is actually by millisecs.

As you use start and end interval, just with the semantics and/or increasing/decreasing 1 millisecs, you would get correct results if this was an issue (it isn't).

You already do +1 millisec.

With all this in mind, I still recommend going with getTimeOrderedEpoch. Some queries based on getTimeOrderedEpoch are already in place and working for months, without bug reports so far.

But not concern if you prefer to move forward. This would be just a small piece of tech debt to centralise and remove duplications in the future (if we ever do it).

- #7: Add INSTANCE singleton pattern for InstantConverter - #8: Use StringUtils.isEmpty() for null-safe empty check - Note: #2 and #3 already addressed in previous commit

@singleton

- Simplified InstantToUUIDMapper to use IdGenerator.getTimeOrderedEpoch() instead of convertOtelIdToUUIDv7 - Per UUIDv7 RFC, sub-millisecond 12 bits are optional with millisecond granularity - Start/end interval semantics with ±1ms ensures correct BETWEEN query results - This approach has been battle-tested for months without issues per reviewer recommendation - Converted InstantToUUIDMapper to @singleton service for proper DI integration - Updated TracesResource to inject InstantToUUIDMapper dependency - Updated tests to properly mock IdGenerator dependency

andrescrz

LGTM.

andrescrz · 2025-11-04T13:52:51Z

+     * @param timestamp the instant in time
+     * @return the lower bound UUIDv7 (min UUID at this timestamp)
+     */
+    public UUID toLowerBound(Instant timestamp) {


Minor: probably this whole class is no longer needed, as it adds verbosity with little benefit. You could move these to IdGenerator and IdGeneratorImpl as overloaded methods and encapsulate the logic there.

@SuppressWarnings

) * [NA] [BE] Upgrade MySQL container from Testcontainers * Fix imports order * [OPIK-2856] [BE] Implement UUIDv7 time-based filtering for traces - Add InstantToUUIDMapper to convert Instant timestamps to UUIDv7 bounds - Add InstantParamConverter to parse ISO-8601 and epoch millisecond time parameters - Update TracesResource to accept from_time and to_time parameters on /traces and /traces/stats endpoints - Update TraceDAO to apply UUID-based time filtering using BETWEEN clause on id column - Update TraceSearchCriteria to include uuidFromTime and uuidToTime fields - Add comprehensive integration tests for time filtering with boundary conditions - All tests passing: 10/10 time filtering tests + validation tests * [OPIK-2856] Address PR review comments: improve exception handling and type safety - Fix InstantParamConverter to catch specific DateTimeParseException instead of generic Exception - Add debug logging when falling back to epoch milliseconds parsing - Refactor anonymous ParamConverter class to named InstantConverter inner class for clarity - Suppress unchecked cast with @SuppressWarnings annotation - Fix MySQLContainerUtils return type to use MySQLContainer<?> for type safety * [OPIK-2856] Fix InstantToUUIDMapperTest to match implementation - Update tests to reflect that toUpperBound uses next millisecond (+1ms) for inclusive BETWEEN queries - Remove outdated assertions expecting same timestamp in both bounds - Verify upper bound is lexicographically greater than lower bound - All 13 tests now passing * Remove setup duplicated code * Revision 2: Address PR review comments - LOW priority fixes - #7: Add INSTANCE singleton pattern for InstantConverter - #8: Use StringUtils.isEmpty() for null-safe empty check - Note: #2 and #3 already addressed in previous commit * Revision 3: Use IdGenerator.getTimeOrderedEpoch() for UUID bounds - Simplified InstantToUUIDMapper to use IdGenerator.getTimeOrderedEpoch() instead of convertOtelIdToUUIDv7 - Per UUIDv7 RFC, sub-millisecond 12 bits are optional with millisecond granularity - Start/end interval semantics with ±1ms ensures correct BETWEEN query results - This approach has been battle-tested for months without issues per reviewer recommendation - Converted InstantToUUIDMapper to @singleton service for proper DI integration - Updated TracesResource to inject InstantToUUIDMapper dependency - Updated tests to properly mock IdGenerator dependency * Fix tests

thiagohora added 3 commits November 3, 2025 10:08

[NA] [BE] Upgrade MySQL container from Testcontainers

cf912c6

Fix imports order

3d9126a

Copilot AI review requested due to automatic review settings November 3, 2025 16:12

thiagohora requested a review from a team as a code owner November 3, 2025 16:12

github-actions bot assigned thiagohora Nov 3, 2025

Copilot AI reviewed Nov 3, 2025

View reviewed changes

thiagohora added 2 commits November 3, 2025 17:14

Merge branch 'main' into thiaghora/OPIK-2856-uuidv7-time-based-filtering

037c880

thiagohora force-pushed the thiaghora/OPIK-2856-uuidv7-time-based-filtering branch from a5d9353 to de89734 Compare November 3, 2025 16:43

Merge branch 'main' into thiaghora/OPIK-2856-uuidv7-time-based-filtering

551e9b8

Remove setup duplicated code

fc34b96

andrescrz requested changes Nov 4, 2025

View reviewed changes

andrescrz reviewed Nov 4, 2025

View reviewed changes

andrescrz previously approved these changes Nov 4, 2025

View reviewed changes

Revision 2: Address PR review comments - LOW priority fixes

4a0842a

- #7: Add INSTANCE singleton pattern for InstantConverter - #8: Use StringUtils.isEmpty() for null-safe empty check - Note: #2 and #3 already addressed in previous commit

thiagohora dismissed andrescrz’s stale review via 4a0842a November 4, 2025 11:31

thiagohora force-pushed the thiaghora/OPIK-2856-uuidv7-time-based-filtering branch from 0a17ed6 to 4a0842a Compare November 4, 2025 11:31

thiagohora added 2 commits November 4, 2025 12:52

Fix tests

bdd2249

thiagohora requested a review from andrescrz November 4, 2025 12:31

andrescrz approved these changes Nov 4, 2025

View reviewed changes

thiagohora merged commit f9338df into main Nov 4, 2025
14 checks passed

thiagohora deleted the thiaghora/OPIK-2856-uuidv7-time-based-filtering branch November 4, 2025 14:51

This was referenced Nov 4, 2025

[OPIK-2856] [BE] Implement UUIDv7 time-based filtering for spans #3938

Merged

[OPIK-2856] [BE] Add UUIDv7 time-based filtering for trace threads #3953

Merged

Conversation

thiagohora commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details

Implementation

How It Works

Testing

Change checklist

Issues

Testing

Documentation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backend Tests Results

Uh oh!

github-actions bot commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SDK E2E Tests Results

Uh oh!

andrescrz left a comment

Choose a reason for hiding this comment

Uh oh!

andrescrz left a comment

Choose a reason for hiding this comment

Uh oh!

andrescrz Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

thiagohora Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrescrz Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

thiagohora Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrescrz left a comment

Choose a reason for hiding this comment

Uh oh!

andrescrz Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

andrescrz left a comment

Choose a reason for hiding this comment

Uh oh!

andrescrz Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thiagohora commented Nov 3, 2025 •

edited

Loading

github-actions bot commented Nov 3, 2025 •

edited

Loading

github-actions bot commented Nov 4, 2025 •

edited

Loading

thiagohora Nov 4, 2025 •

edited

Loading