Add attemptDequeue span to marqs #2433
Conversation
More telemetry to figure out why dequeuing is slow
|
|
Caution Review failedThe pull request is closed. WalkthroughPer-queue dequeue logic in apps/webapp/app/v3/marqs/index.server.ts was refactored to use nested tracing. The queue iteration is wrapped with this.#trace("attemptDequeue", ...) and each dequeue call is wrapped with this.#trace("callDequeueMessage", ...) (both using SpanKind.CONSUMER). If a message is returned, code reads the message, constructs detailed attributes (ids, concurrencyKey, timestamps, age, priority, enqueueMethod, availability, flattened message.data, attempted_queues/attempted_envs), applies them to spans, starts an inner "messageDequeued" trace to notify the subscriber, and starts a "startHeartbeat" trace for visibility timeout. Errors inside inner traces return null and log a warning, allowing the outer loop to continue. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/webapp/app/v3/marqs/index.server.ts (3)
652-654: Use a standard messaging operation value for better OTel compatibility
The semantic-conventions operation is set to "dequeue", but the common/expected values for a consumer are typically "receive" (and sometimes "process"). Using a non-standard string can fragment metrics/alerts and dashboards.Consider aligning with the rest of this file (e.g., other spans use "receive"):
- [SEMATTRS_MESSAGING_OPERATION]: "dequeue", + [SEMATTRS_MESSAGING_OPERATION]: "receive",Would you like me to scan the codebase to list all current values used for
SEMATTRS_MESSAGING_OPERATIONso we can normalize them in one PR?
619-621: Clarify/count semantics for attempted_ attributes*
attemptedQueues/attemptedEnvsare incremented before each attempt, so the values include the successful attempt. The comment says “before success,” which is misleading.Two options:
- Keep the current counting (includes successful attempt) and update the comments.
- Or report “before success” precisely:
- attempted_queues: attemptedQueues, // How many queues we tried before success - attempted_envs: attemptedEnvs, // How many environments we tried before success + attempted_queues: Math.max(0, attemptedQueues - 1), // Queues tried before this success + attempted_envs: Math.max(0, attemptedEnvs - 1), // Envs tried before this successPick one and apply consistently so downstream dashboards are unambiguous.
626-626: Cap flattened message attributes to avoid exporter/collector limits
Flattening the fullmessage.datawithout a cap can exceed attribute count/size limits and silently drop data. Provide a bounded max to keep spans lightweight and consistent:- ...flattenAttributes(message.data, "message.data"), + ...flattenAttributes(message.data, "message.data", 64),Tune 64 based on your backend limits (e.g., 128/256) and typical payload shapes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/webapp/app/v3/marqs/index.server.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/v3/marqs/index.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/v3/marqs/index.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: In the webapp, all environment variables must be accessed through theenvexport ofenv.server.ts, instead of directly accessingprocess.env.
When importing from@trigger.dev/corein the webapp, never import from the root@trigger.dev/corepath; always use one of the subpath exports as defined in the package's package.json.
Files:
apps/webapp/app/v3/marqs/index.server.ts
🧬 Code graph analysis (1)
apps/webapp/app/v3/marqs/index.server.ts (3)
packages/core/src/v3/utils/flattenAttributes.ts (2)
attributes(23-25)flattenAttributes(6-14)apps/webapp/app/v3/tracer.server.ts (1)
logger(101-106)internal-packages/tracing/src/index.ts (1)
SpanKind(34-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
More telemetry to figure out why dequeuing is slow