Skip to content

fix(backend): negative avg time to retrieve metrics#140

Merged
silent-cipher merged 5 commits intomainfrom
fix/backend/negative-ipni-time-to-retrieve-ms
Jan 23, 2026
Merged

fix(backend): negative avg time to retrieve metrics#140
silent-cipher merged 5 commits intomainfrom
fix/backend/negative-ipni-time-to-retrieve-ms

Conversation

@silent-cipher
Copy link
Copy Markdown
Collaborator

This PR includes -

  1. We do not defaultuploadEndTime to now. If it is missing, we simply skip the duration calculation (metrics are meaningless without a valid start time).
  2. Added Math.max(0, ...) to ensure that even with slight clock skews, we never record negative durations.
  3. If retrievedAt is missing from sdk, fallback to time when dealbot catches status change to retrieved

Closes #63

Copilot AI review requested due to automatic review settings January 21, 2026 08:42
@vercel
Copy link
Copy Markdown

vercel Bot commented Jan 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
dealbot-web Ready Ready Preview, Comment Jan 22, 2026 2:41pm

Request Review

@FilOzzy FilOzzy added this to FOC Jan 21, 2026
@github-project-automation github-project-automation Bot moved this to 📌 Triage in FOC Jan 21, 2026
@rjan90 rjan90 moved this from 📌 Triage to ⌨️ In Progress in FOC Jan 21, 2026
@rjan90 rjan90 added this to the M4: Filecoin Service Liftoff milestone Jan 21, 2026
@rjan90 rjan90 requested review from SgtPooki and removed request for Copilot January 21, 2026 08:43
@SgtPooki SgtPooki requested a review from Copilot January 21, 2026 13:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes an issue with negative average time-to-retrieve metrics in the IPNI monitoring system. The fix prevents negative durations by using Math.max(0, ...), handles missing uploadEndTime by skipping metric calculations, and provides a fallback timestamp when the SDK doesn't return retrievedAt.

Changes:

  • Modified uploadEndTime handling to skip duration calculations when the value is missing instead of defaulting to current time
  • Added Math.max(0, ...) safeguard to prevent negative durations from clock skew
  • Implemented fallback logic for missing retrievedAt timestamps from the SDK

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/backend/src/deal-addons/strategies/ipni.strategy.ts Outdated
Comment thread apps/backend/src/deal-addons/strategies/ipni.strategy.ts Outdated
Comment thread apps/backend/src/deal-addons/strategies/ipni.strategy.ts Outdated
Copy link
Copy Markdown
Collaborator

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some comments about consistency and goals of the metric..

Comment thread apps/backend/src/deal-addons/strategies/ipni.strategy.ts
Comment thread apps/backend/src/deal-addons/strategies/ipni.strategy.ts
@silent-cipher silent-cipher merged commit 3da277f into main Jan 23, 2026
5 of 6 checks passed
@github-project-automation github-project-automation Bot moved this from ⌨️ In Progress to 🎉 Done in FOC Jan 23, 2026
@SgtPooki SgtPooki deleted the fix/backend/negative-ipni-time-to-retrieve-ms branch January 23, 2026 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🎉 Done

Development

Successfully merging this pull request may close these issues.

IPNI indexing: Average time to retrieve metrics is negative

5 participants