Skip to content

Conversation

@dhable
Copy link
Contributor

@dhable dhable commented Nov 13, 2025

Emits success/failure counter values as well as execution duration as a gauge for task execution. This allows monitoring the background task health using HyperDX alerts.

@changeset-bot
Copy link

changeset-bot bot commented Nov 13, 2025

🦋 Changeset detected

Latest commit: a32fdc9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@hyperdx/api Minor
@hyperdx/app Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Nov 13, 2025

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

Project Deployment Preview Comments Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview Comment Nov 13, 2025 11:25pm

@claude
Copy link

claude bot commented Nov 13, 2025

PR Review: Task Metrics Implementation

Overall Assessment: No critical issues found.

The implementation adds OpenTelemetry metrics for background task monitoring with success/failure counters and duration gauges. Code quality is good and follows project patterns.

Minor Observations (optional improvements):

  • ⚠️ Potential timing issue in index.ts:57-63: Duration gauge records even if task throws exception before metrics in main() are recorded → Consider wrapping gauge recording in try-catch or moving success/failure counters to timeExec callback
  • 📝 Missing period in description (metrics.ts:42): '...because of an exception' → Add period for consistency with line 34
  • 🔍 Type safety: Using .get()?.add() with optional chaining relies on Map being pre-populated → Already safe due to initialization loop, but could add runtime check if TaskName enum is extended dynamically

What works well:

  • ✅ Proper use of TaskName enum for type safety
  • ✅ Clean separation of concerns (metrics.ts vs index.ts)
  • ✅ Good test updates to use new enum
  • ✅ Proper OpenTelemetry patterns with meter/counter/gauge
  • ✅ Changeset included for version management

Recommendation: Safe to merge. The minor timing observation is edge-case and doesn't affect normal operation.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 13, 2025

E2E Test Results

All tests passed • 39 passed • 3 skipped • 312s

Status Count
✅ Passed 39
❌ Failed 0
⚠️ Flaky 1
⏭️ Skipped 3

View full report →

Emits success/failure counter values as well as execution duration as
a gauge for task execution. This allows monitoring the background task
health using HyperDX alerts.
@dhable dhable force-pushed the dan/hdx-2808-emit-latencyduration-metrics-in-the-check-alerts-job branch from 8d7be75 to e6b1246 Compare November 13, 2025 22:25
pulpdrew
pulpdrew previously approved these changes Nov 13, 2025
@kodiakhq kodiakhq bot merged commit 94a669d into main Nov 13, 2025
9 checks passed
@kodiakhq kodiakhq bot deleted the dan/hdx-2808-emit-latencyduration-metrics-in-the-check-alerts-job branch November 13, 2025 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants