-
Notifications
You must be signed in to change notification settings - Fork 314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: adding sourceId and destinationId in pipeline info metrics #4332
Conversation
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke 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:
Additional details and impacted files@@ Coverage Diff @@
## master #4332 +/- ##
==========================================
- Coverage 74.02% 74.01% -0.02%
==========================================
Files 381 381
Lines 54404 54479 +75
==========================================
+ Hits 40272 40320 +48
- Misses 11849 11875 +26
- Partials 2283 2284 +1 ☔ View full report in Codecov by Sentry. |
router/batchrouter/handle.go
Outdated
@@ -576,6 +576,7 @@ func (brt *Handle) updateJobStatus(batchJobs *BatchedJobs, isWarehouse bool, err | |||
transformedAtMap := make(map[string]string) | |||
statusDetailsMap := make(map[string]*types.StatusDetail) | |||
jobStateCounts := make(map[string]int) | |||
jobIdConnectionDetailsMap := make(map[int64]*jobsdb.ConnectionDetailsT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for using a pointer to a struct? Fro small struct is more efficient to pass them as is.
also (nitpik) jobID
instead of jobId
https://google.github.io/styleguide/go/decisions.html#initialisms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the reason we use the maps, but is a hack that we need to address some day
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will change it for sourceId and destinationId as well
jobsdb/jobsdb.go
Outdated
@@ -365,6 +365,11 @@ type JobStatusT struct { | |||
WorkspaceId string `json:"WorkspaceId"` | |||
} | |||
|
|||
type ConnectionDetailsT struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We no longer use T
suffix for structs
type ConnectionDetailsT struct { | |
type ConnectionDetails struct { |
@lvrach updateProcessedEventsMetrics code for both rt and brt is almost same except module and destType differs. Should i change this function to a common function where rt and brt call them with extra parameters instead of a method function? |
…der-server into feat.addIdsInPipelineInfo
Sounds good. Maybe something like this..?
You can use |
…der-server into feat.addIdsInPipelineInfo
* feat: add sourceid and destination id to pipeline_processed_events metric
Description
Rudder server emits a metric called pipeline_processed_events which has labels state and code. This metric is useful to understand the different status codes when rudder server is processing events to destination. Rudder sources team want to use this info to show it to end users. So that they can know how is the sync performing. For this to achieve we want to add sourceId and destinationId as extra labels to pipeline_processed_events metric.
Linear Ticket
https://linear.app/rudderstack/issue/OBS-299/rudder-server-add-sourceid-and-destinationid-in-pipeline-processed
Security