chore: add response logging support for router delivery destinations #4707
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4707 +/- ##
==========================================
- Coverage 74.65% 74.57% -0.08%
==========================================
Files 414 414
Lines 48549 48649 +100
==========================================
+ Hits 36242 36280 +38
- Misses 9941 9991 +50
- Partials 2366 2378 +12 ☔ View full report in Codecov by Sentry. |
router/network.go
Outdated
| obskit.WorkspaceID(destInfo.WorkspaceID), | ||
| ) | ||
| } | ||
| if destID == destInfo.ID || workspaceID == destInfo.WorkspaceID || destType == destInfo.DefinitionName { |
There was a problem hiding this comment.
do we need a log for all events or a filter for only failed events?
There was a problem hiding this comment.
Log for all events as we would like to know the request IDs to share the information with users when needed
There was a problem hiding this comment.
won't this effect rudder-server performance?
There was a problem hiding this comment.
Logging the requests would definitely affect functionality of any service but the need of the hour is that we need to supply the request information details to users when needed. Let me know if there is any other way of tackling this
There was a problem hiding this comment.
@rudderlabs/server_team is it ok if we enable logging for some duration and log all event responses from the destination
There was a problem hiding this comment.
any other way of tackling this
Admittedly without the entire context, it does look like a good candidate for reporting data.
If we're okay with sampled data, going thru reporting data for sample response with all the mentioned filters(destType, destinationId, or workspaceId) should be good imo.
There was a problem hiding this comment.
Here, we are interested in getting information about the destination's response(when delivered via router) like requestIds, the actual response that we got from destination, any other information that destination sent us.
I don't think sampling actually serves the requirement.
Any user might need these information for a specific set of time range & they would need all the information & not samples.
There was a problem hiding this comment.
@sanpj2292 , I don't think it's a great idea to log each and every response. Also, polluting code and configurations just for logging purposes.
Also, I do see we're logging res body which probably might contain PII information which we should avoid logging.
Can you share any customer requests where customers needed this info?
Is it possible to explore the path Siddharth mentioned above? may be we can enrich reporting sampled data with some more info?
|
This PR is considered to be stale. It has been open 20 days with no further activity thus it is going to be closed in 7 days. To avoid such a case please consider removing the stale label manually or add a comment to the PR. |
Description
Currently, some destinations send events directly from the router, while others send events through transformer. When customers request specific information for debugging, the data is often not readily available. This PR aims to address that issue.
We propose adding logging capabilities for these destinations. The logs will capture the destination's response body, status code, and headers.
This feature will be configurable, requiring the service owner to specify the destType, destinationId, or workspaceId. When an event's data matches any of these criteria, the response information will be logged.
Note: The service owner must enable this logging feature for it to take effect.
Linear Ticket
Resolves INT-2217
Security