github-featured / comet-ml--opik

发布

0 发布 8,340 Git 标签

RSS 订阅源
  • 1.9.8-3286 3314054743
    比较

    [OPIK-2856] [BE] Use batch calls to reduce test duration (#4034)

    frostbyte_neo 发布于 2025-11-12 12:12:10 +00:00

    • [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

    • [OPIK-2856] [BE] Split Get spans Tests

    • Fix format

    • Revision 2: Extract duplicated span creation logic into createSpanWithTimestamp helper method

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

    • Revision 3: Extract workspace setup duplication into setupTestWorkspace helper method and fix transformTestParams call

    • [OPIK-2856] Refactor TracesResourceTest to use TraceResourceClient instead of direct URL_TEMPLATE calls

    • Replace all direct client.target(URL_TEMPLATE) calls with TraceResourceClient methods
    • Add callFeedbackScoresWithCookie method to TraceResourceClient for session token authentication
    • Add callRetrieveThreadResponseWithCookie method to TraceResourceClient for session token authentication
    • Fix feedback batch endpoint by using callFeedbackScores and callFeedbackScoresWithCookie
    • Add null checks for query parameters to prevent NPE errors
    • Fix API key vs session token usage in authentication tests
    • Rename get__whenApiKeyIsPresent__thenReturnTraceThread to get__whenSessionTokenIsPresent__thenReturnTraceThread in SessionTokenCookie class
    • Add mockGetWorkspaceIdByName() calls for proper workspace mocking
    • Preserve original test assertions and behavior
    • All tests properly refactored to use resource client methods instead of direct HTTP calls
    • [OPIK-2856] Remove duplicate methods from TraceResourceClient
    • Remove callGetTraces() - duplicate of callGetTracesWithQueryParams()
    • Remove callSearchTraces() - duplicate of callSearchTracesStream()
    • Reduced code duplication and maintenance burden

    • Fix tests

    • Revision 2: Address Copilot review comments - remove redundant wrapper method and add clarifying comment

    • Revision 3: Extract duplicated path splitting logic into helper method addPathSegments()

    • Revision 4: Make getWebTarget() private and add callGetTraceThreadsWithSorting() public method

    • Revision 7: Move addPathSegments() and addQueryParameters() helper methods to BaseCommentResourceClient

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

    • Revision 2: Address GitHub Copilot PR review comments

    • Extract conditional UUID generation into generateThreadModelId() method for better readability
    • Rename minTraceTimestamp to earliestTraceTimestamp for clarity
    • Add explanatory comment about UUIDv7 lexicographic ordering in compareTo()

    • Fix

    • Revision 3: Add UUID time filter to SELECT_TRACES_STATS query

    • Revision 4: Fix generateUUIDForTimestamp to manually construct UUIDv7

    • [OPIK-2856] [BE] Implement UUIDv7-based time filtering for project metrics

    • Add uuidFromTime and uuidToTime fields to ProjectMetricRequest
    • Update ProjectMetricsService to enrich requests with UUID bounds using InstantToUUIDMapper
    • Refactor ProjectMetricsDAO SQL queries to use UUID-based filtering (id BETWEEN uuid_from_time AND uuid_to_time)
    • Extract timestamps from UUIDs using UUIDv7ToDateTime for bucketing and WITH FILL clauses
    • Update TraceService to generate UUIDs based on trace startTime when ID is not provided
    • Fix ProjectMetricsResourceTest to generate UUIDs with correct timestamps using TimeBasedEpochGenerator
    • Remove explicit openTraceThread calls in tests to allow traces to create thread metadata with correct timestamps

    All 206 ProjectMetricsResourceTest tests now passing (1 skipped).

    • [OPIK-2856] Fix flaky MultiValueFeedbackScoresE2ETest by ensuring UUID bounds are min/max for timestamp

    • [OPIK-2856] Update InstantToUUIDMapper tests to match new min/max UUID implementation

    • [OPIK-2856] Address Copilot PR review comments: clarify 62-bit constant and update validateProject comment

    • [OPIK-2856] [BE] Extract UUID utility for test reuse

    • Create UUIDTestUtils with generateUUIDForTimestamp method
    • Replace local implementations in ProjectMetricsResourceTest
    • Replace local implementations in FindSpansResourceTest
    • Remove duplicate method definitions and unused imports
    • Centralize UUID generation logic for time-based testing

    Tests verified: ✅ ProjectMetricsResourceTest (206 tests passed, 1 skipped)

    • Revert id changes

    • [OPIK-2856] [BE] Use batch calls to reduce test duration

    下载附件