chore: Improve incoming webhook logging & add middleware #37948
Conversation
|
WalkthroughAdds middleware (logger, metrics, tracer) to the integrations WebHook API router, exports a shared Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WebHookAPI
participant LoggerMiddleware as Logger/Tracer/Metrics
participant executeIntegrationRest as Executor
participant incomingLogger
Client->>WebHookAPI: POST /hooks/...
WebHookAPI->>LoggerMiddleware: apply logger/metrics/tracer
LoggerMiddleware-->>WebHookAPI: enriched request (trace/metrics/log context)
WebHookAPI->>executeIntegrationRest: process webhook
activate executeIntegrationRest
alt success
executeIntegrationRest-->>WebHookAPI: 200 OK
LoggerMiddleware-->>Client: 200 OK (metrics recorded)
else error
executeIntegrationRest->>incomingLogger: log error (err)
incomingLogger-->>WebHookAPI: logged error details
executeIntegrationRest-->>WebHookAPI: failure(payload: err or message)
LoggerMiddleware-->>Client: error response (metrics/tracing recorded)
end
deactivate executeIntegrationRest
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (7)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37948 +/- ##
===========================================
+ Coverage 70.63% 70.67% +0.04%
===========================================
Files 3145 3145
Lines 108772 108772
Branches 19570 19570
===========================================
+ Hits 76826 76871 +45
+ Misses 29943 29896 -47
- Partials 2003 2005 +2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
📦 Docker Image Size Report➡️ Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 23:26", "12/11 21:56", "12/12 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:38", "12/25 17:51", "12/26 13:18", "12/29 19:01", "12/30 20:52", "01/08 15:34 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.00]
line "omnichannel-transcript-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "queue-worker-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "rocketchat" [0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.00]
Statistics (last 30 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Thu, 08 Jan 2026 15:34:55 GMT |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/meteor/app/integrations/server/api/api.ts (1)
386-389: Remove redundant RegExp constructor.The middleware application correctly enables observability for webhook routes. However,
new RegExp(/^\/hooks\//)is redundant since/^\/hooks\//is already a RegExp literal.🔎 Simplify RegExp usage
Api.router .use(loggerMiddleware(integrationLogger)) - .use(metricsMiddleware({ basePathRegex: new RegExp(/^\/hooks\//), api: Api, settings, summary: metrics.rocketchatRestApi })) + .use(metricsMiddleware({ basePathRegex: /^\/hooks\//, api: Api, settings, summary: metrics.rocketchatRestApi })) .use(tracerSpanMiddleware);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/meteor/app/integrations/server/api/api.tsapps/meteor/app/integrations/server/logger.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/app/integrations/server/api/api.tsapps/meteor/app/integrations/server/logger.ts
🧬 Code graph analysis (1)
apps/meteor/app/integrations/server/api/api.ts (2)
apps/meteor/app/integrations/server/logger.ts (2)
incomingLogger(5-5)integrationLogger(3-3)apps/meteor/app/metrics/server/lib/metrics.ts (1)
metrics(5-249)
🔇 Additional comments (4)
apps/meteor/app/integrations/server/logger.ts (1)
3-6: LGTM! Clean logger hierarchy.The logger structure is well-organized with a base
integrationLoggerand appropriate sections for incoming and outgoing webhooks. This enables better log filtering and contextual logging.apps/meteor/app/integrations/server/api/api.ts (3)
17-19: LGTM! Middleware imports for observability.The imported middleware will enhance observability for webhook requests with logging, metrics, and tracing capabilities.
22-22: LGTM! Metrics import for Prometheus.The metrics import enables Prometheus metric collection for webhook API calls.
25-25: LGTM! Updated import aligns with logger hierarchy.The import now includes
integrationLoggerto support the middleware configuration, aligning with the refactored logger structure.
KevLehman
added
the
stat: QA assured
|
Looks like this PR is ready to merge! 🎉 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/meteor/app/integrations/server/api/api.ts">
<violation number="1" location="apps/meteor/app/integrations/server/api/api.ts:256">
P2: Accessing `err.error` or `err.message` without null-safety could throw if `err` is null/undefined. Consider using optional chaining with a fallback to handle edge cases where non-Error values are thrown.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Proposed changes (including videos or screenshots)
https://rocketchat.atlassian.net/browse/CORE-1716
Issue(s)
Steps to test or reproduce
Further comments
/hooksapi (the one incoming integrations use)/hooksapiCurrently we're on the blind side when these calls fail, after this, we'll be a little bit less blind
Summary by CodeRabbit
New Features
Improvements
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.