github-featured / comet-ml--opik

发布

0 发布 8,340 Git 标签

RSS 订阅源
  • 1.9.13-3297 d11e4f41f0
    比较

    [OPIK-2974] [BE] Add metrics-based alerting for cost and latency thresholds (#3974)

    frostbyte_neo 发布于 2025-11-13 17:00:01 +00:00

    • Add MetricsAlertJob

    • add tests

    • change latency threshold to seconds

    • minor improvements

    • address comments

    • adjust timing

    • address comments

    • Address github pr comments (#4047)

    • Revision 3: Address GitHub PR review comments

    • Migrate MetricsAlertJob from ScheduledExecutorService to Quartz with @DisallowConcurrentExecution
    • Extract Unix epoch timestamp as named constant in ProjectMetricsDAO
    • Convert Map to MetricsAlertPayload DTO in SlackWebhookPayloadMapper
    • Add comment explaining event type replacement in AlertResourceTest
    • Change rollback statement to empty in database migration
    • Improve code clarity and maintainability per reviewer feedback

    • Revision 4: Fix migration rollback to 'rollback empty'

    • Revision 5: Make MetricsAlertJob interval configurable

    • Remove hardcoded @Every("5m") annotation from MetricsAlertJob
    • Schedule MetricsAlertJob programmatically in OpikGuiceyLifecycleEventListener
    • Use webhookConfig.metrics.initialDelay and fixedDelay for configurable scheduling
    • Calculate lock duration dynamically based on configured job interval
    • Follow same pattern as TraceThreadsClosingJob for consistency
    • Revision 6: Fix test to match MetricsAlertPayload DTO structure
    • Remove event_type check from test as it's not part of the DTO
    • Event type is redundant in payload since it's known from trigger context
    • Add comment explaining why event_type is not in the payload
    • Revision 7: Revert Unix epoch constant extraction to fix Spotless error
    • Cannot concatenate constants inside text blocks with triple-quote syntax
    • Revert to inline hardcoded values to avoid syntax errors
    • This is a limitation of Java text blocks - constants cannot be embedded mid-block

    • Revision 8: Fix compilation error - use getWebhook() instead of getWebhookConfig()

    • Revision 9: Add defensive error handling for MetricsAlertJob scheduling

    • Add null checks for webhook config and metrics config
    • Add comprehensive exception handling with logging
    • Log warnings if configuration is missing instead of failing
    • Catch unexpected exceptions to prevent app startup failure
    • Revision 10: Explicitly bind MetricsAlertJob in JobModule for Guice
    • Add explicit Guice binding for MetricsAlertJob in JobModule
    • This allows Quartz to instantiate the job with dependencies via Guice
    • Required since job no longer has @Every annotation for auto-registration
    • Revision 14: Move MetricsAlertJob and AlertWebhookSender to com.comet.opik.domain.alerts package
    • Moved MetricsAlertJob.java from com.comet.opik.api.resources.v1.jobs to com.comet.opik.domain.alerts
    • Moved AlertWebhookSender.java from com.comet.opik.api.resources.v1.jobs to com.comet.opik.domain.alerts
    • Updated package name in both files
    • Updated imports in:
      • OpikGuiceyLifecycleEventListener.java (MetricsAlertJob import)
      • AlertJob.java (AlertWebhookSender import)
      • JobModule.java (removed unused import)
    • File paths now align with package structure (domain.alerts)
    • All compilation successful
    • Revision 15: Add metric_name field to MetricsAlertPayload in test payloads and builder
    • Added metricName parameter to MetricsAlertPayload builder in MetricsAlertJob
    • Added metric_name field to TEST_PAYLOAD examples for TRACE_COST and TRACE_LATENCY in AlertService
    • Fixes: "metricName is marked non-null but is null" deserialization error
    • Tests: 135 total, 14 failures/errors to investigate in TestAlertEvents
    • Revision 16: Fix AlertResourceTest event type values to match AlertEventType.getValue()
    • Changed test assertions from "TRACE_COST" to "trace:cost" (matches getValue() return value)
    • Changed test assertions from "TRACE_LATENCY" to "trace:latency"
    • Aligns test expectations with actual AlertEventType enum values
    • Revert "Revision 16: Fix AlertResourceTest event type values to match AlertEventType.getValue()"

    This reverts commit 29a84297ea28f444cb6018b4d5c07370f7b14572.

    • Revert "Revision 15: Add metric_name field to MetricsAlertPayload in test payloads and builder"

    This reverts commit ef8ff2fb965ff915bae939773373c67cff8a7a7f.

    • Reapply "Revision 16: Fix AlertResourceTest event type values to match AlertEventType.getValue()"

    This reverts commit e4cc4cf5a35d660321062ca28782b5375855ad7f.

    • Reapply "Revision 15: Add metric_name field to MetricsAlertPayload in test payloads and builder"

    This reverts commit e60d2646242bee873b9b49f0f69d71dcc357c753.

    • Make metricName field optional in MetricsAlertPayload

    • Make metricName optional and add metric_name to test payloads

    • Fix test payload type handling for numeric values in verifyMetricsPayload

    • Fix: Improve metric value parsing in AlertResourceTest

    Co-authored-by: thiagoh thiagoh@comet.com

    Co-authored-by: Cursor Agent cursoragent@cursor.com

    • OPIK-2974: [BE] Address PR comments: hide workspaceId; interruptible MetricsAlertJob; ms constant

    • NA: Use UUIDv7 id filter for metrics; keep alerts id DESC

    • Fix queries

    • Fix issue

    Co-authored-by: Borys Tkachenko borystkachenko@Boryss-MacBook-Pro.local
    Co-authored-by: Thiago dos Santos Hora thiagoh@comet.com
    Co-authored-by: Cursor Agent cursoragent@cursor.com

    下载附件