From 19e9cfb7128a13e68ae4db90ceddd90a3be76629 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Wed, 6 Nov 2024 15:33:52 +0530 Subject: [PATCH 01/44] chore: send multiple reports in a single request to reporting --- enterprise/reporting/reporting.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index 80048b10d8..17f723e7f2 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -74,6 +74,7 @@ type DefaultReporter struct { getReportsQueryTime stats.Measurement requestLatency stats.Measurement stats stats.Stats + maxReportsCountInARequest config.ValueLoader[int] } func NewDefaultReporter(ctx context.Context, log logger.Logger, configSubscriber *configSubscriber, stats stats.Stats) *DefaultReporter { @@ -88,6 +89,7 @@ func NewDefaultReporter(ctx context.Context, log logger.Logger, configSubscriber maxConcurrentRequests := config.GetReloadableIntVar(32, 1, "Reporting.maxConcurrentRequests") maxOpenConnections := config.GetIntVar(32, 1, "Reporting.maxOpenConnections") dbQueryTimeout = config.GetReloadableDurationVar(60, time.Second, "Reporting.dbQueryTimeout") + maxReportsCountInARequest := config.GetReloadableIntVar(1, 1, "Reporting.maxReportsCountInARequest") // only send reports for wh actions sources if whActionsOnly is configured whActionsOnly := config.GetBool("REPORTING_WH_ACTIONS_ONLY", false) if whActionsOnly { @@ -114,6 +116,7 @@ func NewDefaultReporter(ctx context.Context, log logger.Logger, configSubscriber maxOpenConnections: maxOpenConnections, maxConcurrentRequests: maxConcurrentRequests, dbQueryTimeout: dbQueryTimeout, + maxReportsCountInARequest: maxReportsCountInARequest, stats: stats, } } @@ -265,7 +268,7 @@ func (r *DefaultReporter) getReports(currentMs int64, syncerKey string) (reports return metricReports, queryMin.Int64, err } -func (*DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) []*types.Metric { +func (r *DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) []*types.Metric { metricsByGroup := map[string]*types.Metric{} var values []*types.Metric @@ -282,9 +285,6 @@ func (*DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) [] report.ConnectionDetails.TrackingPlanID, strconv.Itoa(report.ConnectionDetails.TrackingPlanVersion), report.PUDetails.InPU, report.PUDetails.PU, - report.StatusDetail.Status, - strconv.Itoa(report.StatusDetail.StatusCode), - report.StatusDetail.EventName, report.StatusDetail.EventType, } return strings.Join(groupingIdentifiers, `::`) } @@ -336,6 +336,10 @@ func (*DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) [] EventType: report.StatusDetail.EventType, ErrorType: report.StatusDetail.ErrorType, }) + + if len(metricsByGroup[identifier].StatusDetails) >= r.maxReportsCountInARequest.Load() { + delete(metricsByGroup, identifier) + } } return values From 1744bbc1686de7ec18bbba787cfc99f2d16d9c71 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Thu, 7 Nov 2024 14:41:14 +0530 Subject: [PATCH 02/44] chore: default batch size set to 10 --- enterprise/reporting/reporting.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index 17f723e7f2..0d8ca81f5a 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -89,7 +89,7 @@ func NewDefaultReporter(ctx context.Context, log logger.Logger, configSubscriber maxConcurrentRequests := config.GetReloadableIntVar(32, 1, "Reporting.maxConcurrentRequests") maxOpenConnections := config.GetIntVar(32, 1, "Reporting.maxOpenConnections") dbQueryTimeout = config.GetReloadableDurationVar(60, time.Second, "Reporting.dbQueryTimeout") - maxReportsCountInARequest := config.GetReloadableIntVar(1, 1, "Reporting.maxReportsCountInARequest") + maxReportsCountInARequest := config.GetReloadableIntVar(10, 1, "Reporting.maxReportsCountInARequest") // only send reports for wh actions sources if whActionsOnly is configured whActionsOnly := config.GetBool("REPORTING_WH_ACTIONS_ONLY", false) if whActionsOnly { From 6eaa4fdf905773d45f54d0a3f6ede2d9ffe2e830 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Thu, 7 Nov 2024 15:53:36 +0530 Subject: [PATCH 03/44] chore: added test --- enterprise/reporting/reporting_test.go | 232 ++++++++++++++++++------- 1 file changed, 167 insertions(+), 65 deletions(-) diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index f1c3a6457e..b8f52fbcae 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -159,81 +159,183 @@ var _ = Describe("Reporting", func() { }, }, } + conf := config.New() - expectedResponse := []*types.Metric{ - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 3, - ViolationCount: 5, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "", + It("Should provide aggregated reports when batch size is 10", func() { + conf.Set("Reporting.maxReportsCountInARequest", 10) + expectedResponse := []*types.Metric{ + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", }, - { - Status: "some-status", - Count: 2, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 5, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "", + }, + { + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, }, }, - }, - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id-2", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, + }, }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id-2", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", + } + configSubscriber := newConfigSubscriber(logger.NOP) + reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) + + aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) + Expect(aggregatedMetrics).To(Equal(expectedResponse)) + }) + + It("Should provide aggregated reports when batch size is 1", func() { + conf.Set("Reporting.maxReportsCountInARequest", 1) + expectedResponse := []*types.Metric{ + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 5, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "", + }, + }, }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, + }, }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 3, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id-2", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, }, }, - }, - } - configSubscriber := newConfigSubscriber(logger.NOP) - reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) + } + configSubscriber := newConfigSubscriber(logger.NOP) + reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) - aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) - Expect(aggregatedMetrics).To(Equal(expectedResponse)) + aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) + Expect(aggregatedMetrics).To(Equal(expectedResponse)) + }) }) }) From ff63e49d3e7a3076a1150e79cf2199ecec134866 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Thu, 7 Nov 2024 16:44:54 +0530 Subject: [PATCH 04/44] fix: failing tests --- enterprise/reporting/reporting_test.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index b8f52fbcae..941d3592eb 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -76,6 +76,9 @@ var _ = Describe("Reporting", func() { }) Context("getAggregatedReports Tests", func() { + conf := config.New() + configSubscriber := newConfigSubscriber(logger.NOP) + reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) inputReports := []*types.ReportByStatus{ { InstanceDetails: types.InstanceDetails{ @@ -159,7 +162,6 @@ var _ = Describe("Reporting", func() { }, }, } - conf := config.New() It("Should provide aggregated reports when batch size is 10", func() { conf.Set("Reporting.maxReportsCountInARequest", 10) @@ -232,8 +234,6 @@ var _ = Describe("Reporting", func() { }, }, } - configSubscriber := newConfigSubscriber(logger.NOP) - reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) Expect(aggregatedMetrics).To(Equal(expectedResponse)) @@ -244,7 +244,7 @@ var _ = Describe("Reporting", func() { expectedResponse := []*types.Metric{ { InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", + WorkspaceID: "some-workspace-id1", }, ConnectionDetails: types.ConnectionDetails{ SourceID: "some-source-id", @@ -330,8 +330,6 @@ var _ = Describe("Reporting", func() { }, }, } - configSubscriber := newConfigSubscriber(logger.NOP) - reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) Expect(aggregatedMetrics).To(Equal(expectedResponse)) From 98ed866bbf8ed298593598bf478115f2985027c3 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Thu, 7 Nov 2024 17:07:50 +0530 Subject: [PATCH 05/44] fix: failing tests --- enterprise/reporting/reporting_test.go | 59 ++++++++++++++------------ 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index 941d3592eb..c9b7cfa1f5 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -76,9 +76,6 @@ var _ = Describe("Reporting", func() { }) Context("getAggregatedReports Tests", func() { - conf := config.New() - configSubscriber := newConfigSubscriber(logger.NOP) - reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) inputReports := []*types.ReportByStatus{ { InstanceDetails: types.InstanceDetails{ @@ -163,8 +160,11 @@ var _ = Describe("Reporting", func() { }, } - It("Should provide aggregated reports when batch size is 10", func() { - conf.Set("Reporting.maxReportsCountInARequest", 10) + It("Should provide aggregated reports when batch size is 1", func() { + conf := config.New() + conf.Set("Reporting.maxReportsCountInARequest", 1) + configSubscriber := newConfigSubscriber(logger.NOP) + reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) expectedResponse := []*types.Metric{ { InstanceDetails: types.InstanceDetails{ @@ -193,6 +193,26 @@ var _ = Describe("Reporting", func() { SampleEvent: []byte(`{}`), ErrorType: "", }, + }, + }, + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ { Status: "some-status", Count: 2, @@ -239,12 +259,15 @@ var _ = Describe("Reporting", func() { Expect(aggregatedMetrics).To(Equal(expectedResponse)) }) - It("Should provide aggregated reports when batch size is 1", func() { - conf.Set("Reporting.maxReportsCountInARequest", 1) + It("Should provide aggregated reports when batch size is 10", func() { + conf := config.New() + conf.Set("Reporting.maxReportsCountInARequest", 10) + configSubscriber := newConfigSubscriber(logger.NOP) + reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) expectedResponse := []*types.Metric{ { InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id1", + WorkspaceID: "some-workspace-id", }, ConnectionDetails: types.ConnectionDetails{ SourceID: "some-source-id", @@ -269,26 +292,6 @@ var _ = Describe("Reporting", func() { SampleEvent: []byte(`{}`), ErrorType: "", }, - }, - }, - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ { Status: "some-status", Count: 2, From 816b21300f3c6245f4e28d646aabba3d97d53538 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Thu, 7 Nov 2024 17:25:18 +0530 Subject: [PATCH 06/44] fix: failing tests --- enterprise/reporting/reporting_test.go | 398 ++++++++++++------------- 1 file changed, 198 insertions(+), 200 deletions(-) diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index c9b7cfa1f5..bd5d8996fa 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -75,8 +75,96 @@ var _ = Describe("Reporting", func() { }) }) - Context("getAggregatedReports Tests", func() { - inputReports := []*types.ReportByStatus{ + inputReports := []*types.ReportByStatus{ + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690, + }, + StatusDetail: &types.StatusDetail{ + Status: "some-status", + Count: 3, + ViolationCount: 5, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "", + }, + }, + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690, + }, + StatusDetail: &types.StatusDetail{ + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, + }, + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id-2", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690, + }, + StatusDetail: &types.StatusDetail{ + Status: "some-status", + Count: 3, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, + }, + } + + Context("getAggregatedReports Tests with batch size 1", func() { + conf := config.New() + conf.Set("Reporting.maxReportsCountInARequest", 1) + configSubscriber := newConfigSubscriber(logger.NOP) + reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) + expectedResponse := []*types.Metric{ { InstanceDetails: types.InstanceDetails{ WorkspaceID: "some-workspace-id", @@ -92,16 +180,18 @@ var _ = Describe("Reporting", func() { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690, - }, - StatusDetail: &types.StatusDetail{ - Status: "some-status", - Count: 3, - ViolationCount: 5, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "", + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 5, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "", + }, }, }, { @@ -119,16 +209,18 @@ var _ = Describe("Reporting", func() { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690, - }, - StatusDetail: &types.StatusDetail{ - Status: "some-status", - Count: 2, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, }, }, { @@ -146,197 +238,103 @@ var _ = Describe("Reporting", func() { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690, - }, - StatusDetail: &types.StatusDetail{ - Status: "some-status", - Count: 3, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, }, }, } - It("Should provide aggregated reports when batch size is 1", func() { - conf := config.New() - conf.Set("Reporting.maxReportsCountInARequest", 1) - configSubscriber := newConfigSubscriber(logger.NOP) - reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) - expectedResponse := []*types.Metric{ - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 3, - ViolationCount: 5, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "", - }, - }, + aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) + Expect(aggregatedMetrics).To(Equal(expectedResponse)) + }) + + Context("getAggregatedReports Tests with batch size 10", func() { + conf := config.New() + conf.Set("Reporting.maxReportsCountInARequest", 10) + configSubscriber := newConfigSubscriber(logger.NOP) + reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) + expectedResponse := []*types.Metric{ + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", }, - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 2, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", - }, - }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", }, - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id-2", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 3, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", - }, - }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", }, - } - - aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) - Expect(aggregatedMetrics).To(Equal(expectedResponse)) - }) - - It("Should provide aggregated reports when batch size is 10", func() { - conf := config.New() - conf.Set("Reporting.maxReportsCountInARequest", 10) - configSubscriber := newConfigSubscriber(logger.NOP) - reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) - expectedResponse := []*types.Metric{ - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 5, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "", }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 3, - ViolationCount: 5, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "", - }, - { - Status: "some-status", - Count: 2, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", - }, + { + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", }, }, - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id-2", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 3, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", - }, + }, + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id-2", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", }, }, - } + }, + } - aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) - Expect(aggregatedMetrics).To(Equal(expectedResponse)) - }) + aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) + Expect(aggregatedMetrics).To(Equal(expectedResponse)) }) }) From 0b926dd2d86c8c9e62f080f4cff68db7b1961d2d Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Thu, 7 Nov 2024 17:42:38 +0530 Subject: [PATCH 07/44] fix: failing tests --- enterprise/reporting/reporting_test.go | 393 +++++++++++++------------ 1 file changed, 197 insertions(+), 196 deletions(-) diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index bd5d8996fa..3fce593535 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -75,96 +75,8 @@ var _ = Describe("Reporting", func() { }) }) - inputReports := []*types.ReportByStatus{ - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690, - }, - StatusDetail: &types.StatusDetail{ - Status: "some-status", - Count: 3, - ViolationCount: 5, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "", - }, - }, - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690, - }, - StatusDetail: &types.StatusDetail{ - Status: "some-status", - Count: 2, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", - }, - }, - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id-2", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690, - }, - StatusDetail: &types.StatusDetail{ - Status: "some-status", - Count: 3, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", - }, - }, - } - - Context("getAggregatedReports Tests with batch size 1", func() { - conf := config.New() - conf.Set("Reporting.maxReportsCountInARequest", 1) - configSubscriber := newConfigSubscriber(logger.NOP) - reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) - expectedResponse := []*types.Metric{ + Context("getAggregatedReports Tests", func() { + inputReports := []*types.ReportByStatus{ { InstanceDetails: types.InstanceDetails{ WorkspaceID: "some-workspace-id", @@ -180,18 +92,16 @@ var _ = Describe("Reporting", func() { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 3, - ViolationCount: 5, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "", - }, + ReportedAt: 28017690, + }, + StatusDetail: &types.StatusDetail{ + Status: "some-status", + Count: 3, + ViolationCount: 5, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "", }, }, { @@ -209,18 +119,16 @@ var _ = Describe("Reporting", func() { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 2, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", - }, + ReportedAt: 28017690, + }, + StatusDetail: &types.StatusDetail{ + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", }, }, { @@ -238,103 +146,196 @@ var _ = Describe("Reporting", func() { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 3, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", - }, + ReportedAt: 28017690, + }, + StatusDetail: &types.StatusDetail{ + Status: "some-status", + Count: 3, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", }, }, } - - aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) - Expect(aggregatedMetrics).To(Equal(expectedResponse)) - }) - - Context("getAggregatedReports Tests with batch size 10", func() { conf := config.New() - conf.Set("Reporting.maxReportsCountInARequest", 10) configSubscriber := newConfigSubscriber(logger.NOP) reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) - expectedResponse := []*types.Metric{ - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 3, - ViolationCount: 5, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "", + + It("Should provide aggregated reports when batch size is 1", func() { + conf.Set("Reporting.maxReportsCountInARequest", 1) + Eventually(func() int { return reportHandle.maxReportsCountInARequest.Load() }).Should(Equal(1)) + expectedResponse := []*types.Metric{ + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", }, - { - Status: "some-status", - Count: 2, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 5, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "", + }, }, }, - }, - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, + }, }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id-2", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id-2", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, + }, }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", + } + + aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) + Expect(aggregatedMetrics).To(Equal(expectedResponse)) + }) + + It("Should provide aggregated reports when batch size is 10", func() { + conf.Set("Reporting.maxReportsCountInARequest", 10) + Eventually(func() int { return reportHandle.maxReportsCountInARequest.Load() }).Should(Equal(10)) + expectedResponse := []*types.Metric{ + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 5, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "", + }, + { + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, + }, }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 3, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id-2", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, }, }, - }, - } + } - aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) - Expect(aggregatedMetrics).To(Equal(expectedResponse)) + aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) + Expect(aggregatedMetrics).To(Equal(expectedResponse)) + }) }) }) From e3185a1f875d785b3325c57acd8d45d86528d4ad Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Mon, 11 Nov 2024 10:50:50 +0530 Subject: [PATCH 08/44] chore: using config from outside --- app/apphandlers/embeddedAppHandler.go | 2 +- app/apphandlers/processorAppHandler.go | 2 +- app/features.go | 2 +- enterprise/reporting/mediator.go | 4 ++-- enterprise/reporting/reporting.go | 4 ++-- enterprise/reporting/reporting_test.go | 2 +- enterprise/reporting/setup.go | 5 +++-- enterprise/reporting/setup_test.go | 14 ++++++-------- warehouse/app.go | 2 +- warehouse/app_test.go | 3 ++- 10 files changed, 20 insertions(+), 20 deletions(-) diff --git a/app/apphandlers/embeddedAppHandler.go b/app/apphandlers/embeddedAppHandler.go index e4d128df30..68141dfd3f 100644 --- a/app/apphandlers/embeddedAppHandler.go +++ b/app/apphandlers/embeddedAppHandler.go @@ -97,7 +97,7 @@ func (a *embeddedApp) StartRudderCore(ctx context.Context, options *app.Options) if err != nil { return fmt.Errorf("could not run tracked users database migration: %w", err) } - reporting := a.app.Features().Reporting.Setup(ctx, backendconfig.DefaultBackendConfig) + reporting := a.app.Features().Reporting.Setup(ctx, config, backendconfig.DefaultBackendConfig) defer reporting.Stop() syncer := reporting.DatabaseSyncer(types.SyncerConfig{ConnInfo: misc.GetConnectionString(config, "reporting")}) g.Go(func() error { diff --git a/app/apphandlers/processorAppHandler.go b/app/apphandlers/processorAppHandler.go index 0e97da041d..92faedb940 100644 --- a/app/apphandlers/processorAppHandler.go +++ b/app/apphandlers/processorAppHandler.go @@ -110,7 +110,7 @@ func (a *processorApp) StartRudderCore(ctx context.Context, options *app.Options return fmt.Errorf("could not run tracked users database migration: %w", err) } - reporting := a.app.Features().Reporting.Setup(ctx, backendconfig.DefaultBackendConfig) + reporting := a.app.Features().Reporting.Setup(ctx, config, backendconfig.DefaultBackendConfig) defer reporting.Stop() syncer := reporting.DatabaseSyncer(types.SyncerConfig{ConnInfo: misc.GetConnectionString(config, "reporting")}) g.Go(crash.Wrapper(func() error { diff --git a/app/features.go b/app/features.go index c5f7b0be4d..ae7c300618 100644 --- a/app/features.go +++ b/app/features.go @@ -32,7 +32,7 @@ Reporting Feature // ReportingFeature handles reporting statuses / errors to reporting service type ReportingFeature interface { - Setup(cxt context.Context, backendConfig backendconfig.BackendConfig) types.Reporting + Setup(cxt context.Context, conf *config.Config, backendConfig backendconfig.BackendConfig) types.Reporting } // Features contains optional implementations of Enterprise only features. diff --git a/enterprise/reporting/mediator.go b/enterprise/reporting/mediator.go index 8146dcb185..599b16b4ee 100644 --- a/enterprise/reporting/mediator.go +++ b/enterprise/reporting/mediator.go @@ -34,7 +34,7 @@ type Mediator struct { cronRunners []flusher.Runner } -func NewReportingMediator(ctx context.Context, log logger.Logger, enterpriseToken string, backendConfig backendconfig.BackendConfig) *Mediator { +func NewReportingMediator(ctx context.Context, conf *config.Config, log logger.Logger, enterpriseToken string, backendConfig backendconfig.BackendConfig) *Mediator { ctx, cancel := context.WithCancel(ctx) g, ctx := errgroup.WithContext(ctx) @@ -58,7 +58,7 @@ func NewReportingMediator(ctx context.Context, log logger.Logger, enterpriseToke }) // default reporting implementation - defaultReporter := NewDefaultReporter(rm.ctx, rm.log, configSubscriber, rm.stats) + defaultReporter := NewDefaultReporter(rm.ctx, conf, rm.log, configSubscriber, rm.stats) rm.reporters = append(rm.reporters, defaultReporter) // error reporting implementation diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index 0d8ca81f5a..bc11f592c7 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -77,7 +77,7 @@ type DefaultReporter struct { maxReportsCountInARequest config.ValueLoader[int] } -func NewDefaultReporter(ctx context.Context, log logger.Logger, configSubscriber *configSubscriber, stats stats.Stats) *DefaultReporter { +func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Logger, configSubscriber *configSubscriber, stats stats.Stats) *DefaultReporter { var dbQueryTimeout *config.Reloadable[time.Duration] reportingServiceURL := config.GetString("REPORTING_URL", "https://reporting.rudderstack.com/") @@ -89,7 +89,7 @@ func NewDefaultReporter(ctx context.Context, log logger.Logger, configSubscriber maxConcurrentRequests := config.GetReloadableIntVar(32, 1, "Reporting.maxConcurrentRequests") maxOpenConnections := config.GetIntVar(32, 1, "Reporting.maxOpenConnections") dbQueryTimeout = config.GetReloadableDurationVar(60, time.Second, "Reporting.dbQueryTimeout") - maxReportsCountInARequest := config.GetReloadableIntVar(10, 1, "Reporting.maxReportsCountInARequest") + maxReportsCountInARequest := conf.GetReloadableIntVar(10, 1, "Reporting.maxReportsCountInARequest") // only send reports for wh actions sources if whActionsOnly is configured whActionsOnly := config.GetBool("REPORTING_WH_ACTIONS_ONLY", false) if whActionsOnly { diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index 3fce593535..0f539b1c64 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -161,7 +161,7 @@ var _ = Describe("Reporting", func() { } conf := config.New() configSubscriber := newConfigSubscriber(logger.NOP) - reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) + reportHandle := NewDefaultReporter(context.Background(), conf, logger.NOP, configSubscriber, stats.NOP) It("Should provide aggregated reports when batch size is 1", func() { conf.Set("Reporting.maxReportsCountInARequest", 1) diff --git a/enterprise/reporting/setup.go b/enterprise/reporting/setup.go index e0deccb7d2..74e2b8aad0 100644 --- a/enterprise/reporting/setup.go +++ b/enterprise/reporting/setup.go @@ -4,6 +4,7 @@ import ( "context" "sync" + "github.com/rudderlabs/rudder-go-kit/config" "github.com/rudderlabs/rudder-go-kit/logger" backendconfig "github.com/rudderlabs/rudder-server/backend-config" "github.com/rudderlabs/rudder-server/utils/types" @@ -18,12 +19,12 @@ type Factory struct { } // Setup initializes Suppress User feature -func (m *Factory) Setup(ctx context.Context, backendConfig backendconfig.BackendConfig) types.Reporting { +func (m *Factory) Setup(ctx context.Context, conf *config.Config, backendConfig backendconfig.BackendConfig) types.Reporting { m.oneInstance.Do(func() { if m.Log == nil { m.Log = logger.NewLogger().Child("enterprise").Child("reporting") } - m.instance = NewReportingMediator(ctx, m.Log, m.EnterpriseToken, backendConfig) + m.instance = NewReportingMediator(ctx, conf, m.Log, m.EnterpriseToken, backendConfig) }) return m.instance } diff --git a/enterprise/reporting/setup_test.go b/enterprise/reporting/setup_test.go index c1c390f29c..a77e590559 100644 --- a/enterprise/reporting/setup_test.go +++ b/enterprise/reporting/setup_test.go @@ -16,16 +16,16 @@ import ( ) func TestFeatureSetup(t *testing.T) { - config.Reset() logger.Reset() + config := config.New() f := &Factory{ EnterpriseToken: "dummy-token", } - instanceA := f.Setup(context.Background(), &backendconfig.NOOP{}) + instanceA := f.Setup(context.Background(), config, &backendconfig.NOOP{}) instanceB := f.instance - instanceC := f.Setup(context.Background(), &backendconfig.NOOP{}) + instanceC := f.Setup(context.Background(), config, &backendconfig.NOOP{}) instanceD := f.instance require.Equal(t, instanceA, instanceB) @@ -33,7 +33,7 @@ func TestFeatureSetup(t *testing.T) { require.Equal(t, instanceC, instanceD) f = &Factory{} - instanceE := f.Setup(context.Background(), &backendconfig.NOOP{}) + instanceE := f.Setup(context.Background(), config, &backendconfig.NOOP{}) instanceF := f.instance require.Equal(t, instanceE, instanceF) require.NotEqual(t, instanceE, backendconfig.NOOP{}) @@ -41,9 +41,7 @@ func TestFeatureSetup(t *testing.T) { func TestSetupForDelegates(t *testing.T) { logger.Reset() - - config.Reset() - defer config.Reset() + config := config.New() pool, err := dockertest.NewPool("") require.NoError(t, err) @@ -119,7 +117,7 @@ func TestSetupForDelegates(t *testing.T) { EnterpriseToken: "dummy-token", } } - med := NewReportingMediator(context.Background(), logger.NOP, f.EnterpriseToken, &backendconfig.NOOP{}) + med := NewReportingMediator(context.Background(), config, logger.NOP, f.EnterpriseToken, &backendconfig.NOOP{}) require.Len(t, med.reporters, tc.expectedDelegates) }) diff --git a/warehouse/app.go b/warehouse/app.go index 9380e5dfc1..01fe48f5eb 100644 --- a/warehouse/app.go +++ b/warehouse/app.go @@ -350,7 +350,7 @@ func (a *App) Run(ctx context.Context) error { } if !mode.IsStandAloneSlave(a.config.mode) { - a.reporting = a.app.Features().Reporting.Setup(gCtx, a.bcConfig) + a.reporting = a.app.Features().Reporting.Setup(gCtx, a.conf, a.bcConfig) defer a.reporting.Stop() syncer := a.reporting.DatabaseSyncer(types.SyncerConfig{ConnInfo: a.connectionString("reporting"), Label: types.WarehouseReportingLabel}) g.Go(crash.NotifyWarehouse(func() error { diff --git a/warehouse/app_test.go b/warehouse/app_test.go index 88ef9090c0..af220bb3f4 100644 --- a/warehouse/app_test.go +++ b/warehouse/app_test.go @@ -43,6 +43,7 @@ import ( func TestApp(t *testing.T) { admin.Init() misc.Init() + conf := config.New() const ( workspaceID = "test_workspace_id" @@ -57,7 +58,7 @@ func TestApp(t *testing.T) { require.NoError(t, err) report := &reporting.Factory{} - report.Setup(context.Background(), &bcConfig.NOOP{}) + report.Setup(context.Background(), conf, &bcConfig.NOOP{}) mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() From 3c645f8e6a23231ff7c15bbac3b1f921254755fb Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Thu, 14 Nov 2024 15:29:40 +0530 Subject: [PATCH 09/44] feat: send sample event and response in one report per label set in the configured duration --- config/config.yaml | 3 + enterprise/reporting/event_sampler.go | 147 +++++++++++++++++++++ enterprise/reporting/event_sampler_test.go | 49 +++++++ enterprise/reporting/label_set.go | 69 ++++++++++ enterprise/reporting/label_set_test.go | 76 +++++++++++ enterprise/reporting/reporting.go | 47 +++++++ 6 files changed, 391 insertions(+) create mode 100644 enterprise/reporting/event_sampler.go create mode 100644 enterprise/reporting/event_sampler_test.go create mode 100644 enterprise/reporting/label_set.go create mode 100644 enterprise/reporting/label_set_test.go diff --git a/config/config.yaml b/config/config.yaml index ccc45785e0..553205958e 100644 --- a/config/config.yaml +++ b/config/config.yaml @@ -249,3 +249,6 @@ PgNotifier: retriggerCount: 500 trackBatchInterval: 2s maxAttempt: 3 +Reporting: + eventSamplingEnabled: true + eventSamplingDurationInMinutes: 60 diff --git a/enterprise/reporting/event_sampler.go b/enterprise/reporting/event_sampler.go new file mode 100644 index 0000000000..908c850d5d --- /dev/null +++ b/enterprise/reporting/event_sampler.go @@ -0,0 +1,147 @@ +package reporting + +import ( + "context" + "fmt" + "os" + "sync" + "time" + + "github.com/dgraph-io/badger/v4" + "github.com/dgraph-io/badger/v4/options" + "github.com/rudderlabs/rudder-go-kit/config" + "github.com/rudderlabs/rudder-go-kit/logger" + "github.com/rudderlabs/rudder-server/utils/misc" +) + +type EventSampler struct { + db *badger.DB + mu sync.Mutex + ttl config.ValueLoader[time.Duration] + ctx context.Context + cancel context.CancelFunc + wg sync.WaitGroup +} + +func DefaultPath(pathName string) string { + tmpDirPath, err := misc.CreateTMPDIR() + if err != nil { + panic(err) + } + return fmt.Sprintf(`%v%v`, tmpDirPath, pathName) +} + +func NewEventSampler(pathName string, ttl config.ValueLoader[time.Duration], conf *config.Config, log logger.Logger) (*EventSampler, error) { + dbPath := DefaultPath(pathName) + opts := badger.DefaultOptions(dbPath). + WithLogger(badgerLogger{log}). + WithCompression(options.None). + WithIndexCacheSize(16 << 20). // 16mb + WithNumGoroutines(1). + WithNumMemtables(conf.GetInt("BadgerDB.numMemtable", 5)). + WithValueThreshold(conf.GetInt64("BadgerDB.valueThreshold", 1048576)). + WithBlockCacheSize(0). + WithNumVersionsToKeep(1). + WithNumLevelZeroTables(conf.GetInt("BadgerDB.numLevelZeroTables", 5)). + WithNumLevelZeroTablesStall(conf.GetInt("BadgerDB.numLevelZeroTablesStall", 15)). + WithSyncWrites(conf.GetBool("BadgerDB.syncWrites", false)). + WithDetectConflicts(conf.GetBool("BadgerDB.detectConflicts", false)) + + ctx, cancel := context.WithCancel(context.Background()) + + db, err := badger.Open(opts) + + es := &EventSampler{ + db: db, + ttl: ttl, + ctx: ctx, + cancel: cancel, + wg: sync.WaitGroup{}, + } + + if err != nil { + return nil, err + } + + go es.gcLoop() + + return es, nil +} + +func (es *EventSampler) Get(key []byte) (bool, error) { + es.mu.Lock() + defer es.mu.Unlock() + + var found bool + + err := es.db.View(func(txn *badger.Txn) error { + item, err := txn.Get(key) + + if err != nil { + return err + } + + found = item != nil + return nil + }) + + if err == badger.ErrKeyNotFound { + return false, nil + } else if err != nil { + return false, err + } + + return found, nil +} + +func (es *EventSampler) Put(key []byte) error { + es.mu.Lock() + defer es.mu.Unlock() + + return es.db.Update(func(txn *badger.Txn) error { + entry := badger.NewEntry(key, []byte{1}).WithTTL(es.ttl.Load()) + return txn.SetEntry(entry) + }) +} + +func (es *EventSampler) gcLoop() { + for { + select { + case <-es.ctx.Done(): + _ = es.db.RunValueLogGC(0.5) + return + case <-time.After(5 * time.Minute): + } + again: + if es.ctx.Err() != nil { + return + } + // One call would only result in removal of at max one log file. + // As an optimization, you could also immediately re-run it whenever it returns nil error + // (this is why `goto again` is used). + err := es.db.RunValueLogGC(0.5) + if err == nil { + goto again + } + } +} + +func (es *EventSampler) Close() { + es.cancel() + es.wg.Wait() + if es.db != nil { + _ = es.db.Close() + } +} + +type badgerLogger struct { + logger.Logger +} + +func (badgerLogger) Errorf(format string, a ...interface{}) { + _, _ = fmt.Fprintf(os.Stderr, format, a...) +} + +func (badgerLogger) Warningf(format string, a ...interface{}) { + _, _ = fmt.Fprintf(os.Stderr, format, a...) +} diff --git a/enterprise/reporting/event_sampler_test.go b/enterprise/reporting/event_sampler_test.go new file mode 100644 index 0000000000..db3efd22af --- /dev/null +++ b/enterprise/reporting/event_sampler_test.go @@ -0,0 +1,49 @@ +package reporting + +import ( + "testing" + "time" + + "github.com/rudderlabs/rudder-go-kit/config" + "github.com/rudderlabs/rudder-go-kit/logger" + "github.com/stretchr/testify/assert" +) + +func TestPutAndGet(t *testing.T) { + conf := config.New() + ttl := conf.GetReloadableDurationVar(1, time.Second, "Reporting.eventSamplingDurationInMinutes") + log := logger.NewLogger() + + es, _ := NewEventSampler("/test-reporting-badger", ttl, conf, log) + + es.Put([]byte("key1")) + es.Put([]byte("key2")) + es.Put([]byte("key3")) + val1, _ := es.Get([]byte("key1")) + val2, _ := es.Get([]byte("key2")) + val3, _ := es.Get([]byte("key3")) + + assert.True(t, val1, "Expected key1 to be present") + assert.True(t, val2, "Expected key2 to be present") + assert.True(t, val3, "Expected key3 to be present") + + es.Close() +} + +func TestEvictionOnTTLExpiry(t *testing.T) { + conf := config.New() + ttl := conf.GetReloadableDurationVar(500, time.Millisecond, "Reporting.eventSamplingDurationInMinutes") + log := logger.NewLogger() + + es, _ := NewEventSampler("/test-reporting-badger", ttl, conf, log) + + es.Put([]byte("key1")) + + time.Sleep(600 * time.Millisecond) + + val1, _ := es.Get([]byte("key1")) + + assert.False(t, val1, "Expected key1 to be evicted") + + es.Close() +} diff --git a/enterprise/reporting/label_set.go b/enterprise/reporting/label_set.go new file mode 100644 index 0000000000..b909592150 --- /dev/null +++ b/enterprise/reporting/label_set.go @@ -0,0 +1,69 @@ +package reporting + +import ( + "encoding/hex" + "strconv" + + "github.com/rudderlabs/rudder-server/utils/types" + "github.com/spaolacci/murmur3" +) + +type LabelSet struct { + WorkspaceID string + // Namespace string + // InstanceID string + SourceDefinitionID string + SourceCategory string + SourceID string + DestinationDefinitionID string + DestinationID string + SourceTaskRunID string + SourceJobID string + SourceJobRunID string + TransformationID string + TransformationVersionID string + TrackingPlanID string + TrackingPlanVersion int + InPU string + PU string + Status string + TerminalState bool + InitialState bool + StatusCode int + EventName string + EventType string + ErrorType string +} + +func NewLabelSet(metric types.PUReportedMetric) LabelSet { + return LabelSet{ + WorkspaceID: metric.ConnectionDetails.SourceID, + SourceDefinitionID: metric.ConnectionDetails.SourceDefinitionId, + SourceCategory: metric.ConnectionDetails.SourceCategory, + SourceID: metric.ConnectionDetails.SourceID, + DestinationDefinitionID: metric.ConnectionDetails.DestinationDefinitionId, + DestinationID: metric.ConnectionDetails.DestinationID, + SourceTaskRunID: metric.ConnectionDetails.SourceTaskRunID, + SourceJobID: metric.ConnectionDetails.SourceJobID, + SourceJobRunID: metric.ConnectionDetails.SourceJobRunID, + TransformationID: metric.ConnectionDetails.TransformationID, + TransformationVersionID: metric.ConnectionDetails.TransformationVersionID, + TrackingPlanID: metric.ConnectionDetails.TrackingPlanID, + TrackingPlanVersion: metric.ConnectionDetails.TrackingPlanVersion, + InPU: metric.PUDetails.InPU, + PU: metric.PUDetails.PU, + Status: metric.StatusDetail.Status, + TerminalState: metric.PUDetails.TerminalPU, + InitialState: metric.PUDetails.InitialPU, + StatusCode: metric.StatusDetail.StatusCode, + EventName: metric.StatusDetail.EventName, + EventType: metric.StatusDetail.EventType, + ErrorType: metric.StatusDetail.ErrorType, + } +} + +func (labelSet LabelSet) generateHash() string { + data := labelSet.WorkspaceID + labelSet.SourceDefinitionID + labelSet.SourceCategory + labelSet.SourceID + labelSet.DestinationDefinitionID + labelSet.DestinationID + labelSet.SourceTaskRunID + labelSet.SourceJobID + labelSet.SourceJobRunID + labelSet.TransformationID + labelSet.TransformationVersionID + labelSet.TrackingPlanID + strconv.Itoa(labelSet.TrackingPlanVersion) + labelSet.InPU + labelSet.PU + labelSet.Status + strconv.FormatBool(labelSet.TerminalState) + strconv.FormatBool(labelSet.InitialState) + strconv.Itoa(labelSet.StatusCode) + labelSet.EventName + labelSet.EventType + labelSet.ErrorType + hash := murmur3.Sum64([]byte(data)) + return hex.EncodeToString([]byte(strconv.FormatUint(hash, 16))) +} diff --git a/enterprise/reporting/label_set_test.go b/enterprise/reporting/label_set_test.go new file mode 100644 index 0000000000..fb0385d469 --- /dev/null +++ b/enterprise/reporting/label_set_test.go @@ -0,0 +1,76 @@ +package reporting + +import ( + "testing" + + "github.com/rudderlabs/rudder-server/utils/types" + "github.com/stretchr/testify/assert" +) + +func createMetricObject(eventName string) types.PUReportedMetric { + return types.PUReportedMetric{ + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + StatusDetail: &types.StatusDetail{ + Status: "some-status", + Count: 3, + StatusCode: 0, + SampleResponse: `{"some-sample-response-key": "some-sample-response-value"}`, + SampleEvent: []byte(`{"some-sample-event-key": "some-sample-event-value"}`), + EventName: eventName, + EventType: "some-event-type", + }, + } +} + +func TestNewLabelSet(t *testing.T) { + t.Run("should create the correct LabelSet from types.PUReportedMetric", func(t *testing.T) { + inputMetric := createMetricObject("some-event-name") + labelSet := NewLabelSet(inputMetric) + + assert.Equal(t, "some-source-id", labelSet.SourceID) + assert.Equal(t, "some-event-name", labelSet.EventName) // Default value + }) + + t.Run("should create the correct LabelSet with custom EventName", func(t *testing.T) { + inputMetric := createMetricObject("custom-event-name") + labelSet := NewLabelSet(inputMetric) + + assert.Equal(t, "some-source-id", labelSet.SourceID) + assert.Equal(t, "custom-event-name", labelSet.EventName) // Custom event name + }) +} + +func TestGenerateHash(t *testing.T) { + t.Run("same hash for same LabelSet", func(t *testing.T) { + inputMetric1 := createMetricObject("some-event-name") + labelSet1 := NewLabelSet(inputMetric1) + + inputMetric2 := createMetricObject("some-event-name") + labelSet2 := NewLabelSet(inputMetric2) + + hash1 := labelSet1.generateHash() + hash2 := labelSet2.generateHash() + + assert.Equal(t, hash1, hash2) + }) + + t.Run("different hash for different LabelSet", func(t *testing.T) { + inputMetric1 := createMetricObject("some-event-name-1") + labelSet1 := NewLabelSet(inputMetric1) + + inputMetric2 := createMetricObject("some-event-name-2") + labelSet2 := NewLabelSet(inputMetric2) + + hash1 := labelSet1.generateHash() + hash2 := labelSet2.generateHash() + + assert.NotEqual(t, hash1, hash2) + }) +} diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index bc11f592c7..8875e8bfdb 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -75,6 +75,9 @@ type DefaultReporter struct { requestLatency stats.Measurement stats stats.Stats maxReportsCountInARequest config.ValueLoader[int] + + eventSamplingEnabled config.ValueLoader[bool] + eventSampler *EventSampler } func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Logger, configSubscriber *configSubscriber, stats stats.Stats) *DefaultReporter { @@ -90,11 +93,17 @@ func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Log maxOpenConnections := config.GetIntVar(32, 1, "Reporting.maxOpenConnections") dbQueryTimeout = config.GetReloadableDurationVar(60, time.Second, "Reporting.dbQueryTimeout") maxReportsCountInARequest := conf.GetReloadableIntVar(10, 1, "Reporting.maxReportsCountInARequest") + eventSamplingEnabled := config.GetReloadableBoolVar(false, "Reporting.eventSamplingEnabled") + eventSamplingDuration := config.GetReloadableDurationVar(5, time.Minute, "Reporting.eventSamplingDurationInMinutes") // only send reports for wh actions sources if whActionsOnly is configured whActionsOnly := config.GetBool("REPORTING_WH_ACTIONS_ONLY", false) if whActionsOnly { log.Info("REPORTING_WH_ACTIONS_ONLY enabled.only sending reports relevant to wh actions.") } + eventSampler, err := NewEventSampler("/reporting-badger", eventSamplingDuration, conf, log) + if err != nil { + panic(err) + } ctx, cancel := context.WithCancel(ctx) g, ctx := errgroup.WithContext(ctx) return &DefaultReporter{ @@ -118,6 +127,8 @@ func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Log dbQueryTimeout: dbQueryTimeout, maxReportsCountInARequest: maxReportsCountInARequest, stats: stats, + eventSamplingEnabled: eventSamplingEnabled, + eventSampler: eventSampler, } } @@ -615,6 +626,34 @@ func transformMetricForPII(metric types.PUReportedMetric, piiColumns []string) t return metric } +func (r *DefaultReporter) transformMetricWithEventSampling(metric types.PUReportedMetric) types.PUReportedMetric { + if r.eventSampler == nil { + return metric + } + + isValidSampleEvent := metric.StatusDetail.SampleEvent != nil && string(metric.StatusDetail.SampleEvent) != "{}" + + if isValidSampleEvent { + hash := NewLabelSet(metric).generateHash() + found, err := r.eventSampler.Get([]byte(hash)) + + if err != nil { + panic(err) + } + + if found { + metric.StatusDetail.SampleEvent = []byte(`{}`) + metric.StatusDetail.SampleResponse = "" + } else { + err := r.eventSampler.Put([]byte(hash)) + if err != nil { + panic(err) + } + } + } + return metric +} + func (r *DefaultReporter) Report(ctx context.Context, metrics []*types.PUReportedMetric, txn *Tx) error { if len(metrics) == 0 { return nil @@ -662,6 +701,10 @@ func (r *DefaultReporter) Report(ctx context.Context, metrics []*types.PUReporte metric = transformMetricForPII(metric, getPIIColumnsToExclude()) } + if r.eventSamplingEnabled.Load() { + metric = r.transformMetricWithEventSampling(metric) + } + runeEventName := []rune(metric.StatusDetail.EventName) if len(runeEventName) > 50 { metric.StatusDetail.EventName = fmt.Sprintf("%s...%s", string(runeEventName[:40]), string(runeEventName[len(runeEventName)-10:])) @@ -713,4 +756,8 @@ func (r *DefaultReporter) getTags(label string) stats.Tags { func (r *DefaultReporter) Stop() { r.cancel() _ = r.g.Wait() + + if r.eventSampler != nil { + r.eventSampler.Close() + } } From 9e6814f874ef70c86eca3edd20ec459fb169d490 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Thu, 14 Nov 2024 15:47:03 +0530 Subject: [PATCH 10/44] fix: lint --- enterprise/reporting/event_sampler.go | 2 +- enterprise/reporting/event_sampler_test.go | 3 ++- enterprise/reporting/label_set.go | 3 ++- enterprise/reporting/label_set_test.go | 3 ++- enterprise/reporting/reporting.go | 1 - 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/enterprise/reporting/event_sampler.go b/enterprise/reporting/event_sampler.go index 908c850d5d..d169779287 100644 --- a/enterprise/reporting/event_sampler.go +++ b/enterprise/reporting/event_sampler.go @@ -9,6 +9,7 @@ import ( "github.com/dgraph-io/badger/v4" "github.com/dgraph-io/badger/v4/options" + "github.com/rudderlabs/rudder-go-kit/config" "github.com/rudderlabs/rudder-go-kit/logger" "github.com/rudderlabs/rudder-server/utils/misc" @@ -76,7 +77,6 @@ func (es *EventSampler) Get(key []byte) (bool, error) { err := es.db.View(func(txn *badger.Txn) error { item, err := txn.Get(key) - if err != nil { return err } diff --git a/enterprise/reporting/event_sampler_test.go b/enterprise/reporting/event_sampler_test.go index db3efd22af..8ff421be28 100644 --- a/enterprise/reporting/event_sampler_test.go +++ b/enterprise/reporting/event_sampler_test.go @@ -4,9 +4,10 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/rudderlabs/rudder-go-kit/config" "github.com/rudderlabs/rudder-go-kit/logger" - "github.com/stretchr/testify/assert" ) func TestPutAndGet(t *testing.T) { diff --git a/enterprise/reporting/label_set.go b/enterprise/reporting/label_set.go index b909592150..6ee14d8f0a 100644 --- a/enterprise/reporting/label_set.go +++ b/enterprise/reporting/label_set.go @@ -4,8 +4,9 @@ import ( "encoding/hex" "strconv" - "github.com/rudderlabs/rudder-server/utils/types" "github.com/spaolacci/murmur3" + + "github.com/rudderlabs/rudder-server/utils/types" ) type LabelSet struct { diff --git a/enterprise/reporting/label_set_test.go b/enterprise/reporting/label_set_test.go index fb0385d469..6e77ecaacb 100644 --- a/enterprise/reporting/label_set_test.go +++ b/enterprise/reporting/label_set_test.go @@ -3,8 +3,9 @@ package reporting import ( "testing" - "github.com/rudderlabs/rudder-server/utils/types" "github.com/stretchr/testify/assert" + + "github.com/rudderlabs/rudder-server/utils/types" ) func createMetricObject(eventName string) types.PUReportedMetric { diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index 8875e8bfdb..333ec322ee 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -636,7 +636,6 @@ func (r *DefaultReporter) transformMetricWithEventSampling(metric types.PUReport if isValidSampleEvent { hash := NewLabelSet(metric).generateHash() found, err := r.eventSampler.Get([]byte(hash)) - if err != nil { panic(err) } From b22ffc612324666fa9b327d41e77a9fd23e84ed0 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Thu, 14 Nov 2024 18:39:33 +0530 Subject: [PATCH 11/44] fix: minor --- config/config.yaml | 2 +- enterprise/reporting/event_sampler.go | 12 ++++++++---- enterprise/reporting/reporting.go | 13 +++++++++---- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/config/config.yaml b/config/config.yaml index 553205958e..e4705af27d 100644 --- a/config/config.yaml +++ b/config/config.yaml @@ -250,5 +250,5 @@ PgNotifier: trackBatchInterval: 2s maxAttempt: 3 Reporting: - eventSamplingEnabled: true + eventSamplingEnabled: false eventSamplingDurationInMinutes: 60 diff --git a/enterprise/reporting/event_sampler.go b/enterprise/reporting/event_sampler.go index d169779287..53318ca24c 100644 --- a/enterprise/reporting/event_sampler.go +++ b/enterprise/reporting/event_sampler.go @@ -24,16 +24,20 @@ type EventSampler struct { wg sync.WaitGroup } -func DefaultPath(pathName string) string { +func DefaultPath(pathName string) (string, error) { tmpDirPath, err := misc.CreateTMPDIR() if err != nil { - panic(err) + return "", err } - return fmt.Sprintf(`%v%v`, tmpDirPath, pathName) + return fmt.Sprintf(`%v%v`, tmpDirPath, pathName), nil } func NewEventSampler(pathName string, ttl config.ValueLoader[time.Duration], conf *config.Config, log logger.Logger) (*EventSampler, error) { - dbPath := DefaultPath(pathName) + dbPath, err := DefaultPath(pathName) + if err != nil || dbPath == "" { + return nil, err + } + opts := badger.DefaultOptions(dbPath). WithLogger(badgerLogger{log}). WithCompression(options.None). diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index 333ec322ee..dac81950ba 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -82,6 +82,7 @@ type DefaultReporter struct { func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Logger, configSubscriber *configSubscriber, stats stats.Stats) *DefaultReporter { var dbQueryTimeout *config.Reloadable[time.Duration] + var eventSampler *EventSampler reportingServiceURL := config.GetString("REPORTING_URL", "https://reporting.rudderstack.com/") reportingServiceURL = strings.TrimSuffix(reportingServiceURL, "/") @@ -94,15 +95,19 @@ func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Log dbQueryTimeout = config.GetReloadableDurationVar(60, time.Second, "Reporting.dbQueryTimeout") maxReportsCountInARequest := conf.GetReloadableIntVar(10, 1, "Reporting.maxReportsCountInARequest") eventSamplingEnabled := config.GetReloadableBoolVar(false, "Reporting.eventSamplingEnabled") - eventSamplingDuration := config.GetReloadableDurationVar(5, time.Minute, "Reporting.eventSamplingDurationInMinutes") + eventSamplingDuration := config.GetReloadableDurationVar(60, time.Minute, "Reporting.eventSamplingDurationInMinutes") // only send reports for wh actions sources if whActionsOnly is configured whActionsOnly := config.GetBool("REPORTING_WH_ACTIONS_ONLY", false) if whActionsOnly { log.Info("REPORTING_WH_ACTIONS_ONLY enabled.only sending reports relevant to wh actions.") } - eventSampler, err := NewEventSampler("/reporting-badger", eventSamplingDuration, conf, log) - if err != nil { - panic(err) + + if eventSamplingEnabled.Load() { + var err error + eventSampler, err = NewEventSampler("/reporting-badger", eventSamplingDuration, conf, log) + if err != nil { + panic(err) + } } ctx, cancel := context.WithCancel(ctx) g, ctx := errgroup.WithContext(ctx) From bc0acdd69e4fdcd17b751f691f0249b056f2e898 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Fri, 15 Nov 2024 10:53:59 +0530 Subject: [PATCH 12/44] fix: minor --- enterprise/reporting/event_sampler_test.go | 8 ++++---- enterprise/reporting/reporting.go | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/enterprise/reporting/event_sampler_test.go b/enterprise/reporting/event_sampler_test.go index 8ff421be28..62d6aa3130 100644 --- a/enterprise/reporting/event_sampler_test.go +++ b/enterprise/reporting/event_sampler_test.go @@ -17,9 +17,9 @@ func TestPutAndGet(t *testing.T) { es, _ := NewEventSampler("/test-reporting-badger", ttl, conf, log) - es.Put([]byte("key1")) - es.Put([]byte("key2")) - es.Put([]byte("key3")) + _ = es.Put([]byte("key1")) + _ = es.Put([]byte("key2")) + _ = es.Put([]byte("key3")) val1, _ := es.Get([]byte("key1")) val2, _ := es.Get([]byte("key2")) val3, _ := es.Get([]byte("key3")) @@ -38,7 +38,7 @@ func TestEvictionOnTTLExpiry(t *testing.T) { es, _ := NewEventSampler("/test-reporting-badger", ttl, conf, log) - es.Put([]byte("key1")) + _ = es.Put([]byte("key1")) time.Sleep(600 * time.Millisecond) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index dac81950ba..43b903bf9a 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -94,8 +94,8 @@ func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Log maxOpenConnections := config.GetIntVar(32, 1, "Reporting.maxOpenConnections") dbQueryTimeout = config.GetReloadableDurationVar(60, time.Second, "Reporting.dbQueryTimeout") maxReportsCountInARequest := conf.GetReloadableIntVar(10, 1, "Reporting.maxReportsCountInARequest") - eventSamplingEnabled := config.GetReloadableBoolVar(false, "Reporting.eventSamplingEnabled") - eventSamplingDuration := config.GetReloadableDurationVar(60, time.Minute, "Reporting.eventSamplingDurationInMinutes") + eventSamplingEnabled := conf.GetReloadableBoolVar(false, "Reporting.eventSamplingEnabled") + eventSamplingDuration := conf.GetReloadableDurationVar(60, time.Minute, "Reporting.eventSamplingDurationInMinutes") // only send reports for wh actions sources if whActionsOnly is configured whActionsOnly := config.GetBool("REPORTING_WH_ACTIONS_ONLY", false) if whActionsOnly { @@ -646,7 +646,7 @@ func (r *DefaultReporter) transformMetricWithEventSampling(metric types.PUReport } if found { - metric.StatusDetail.SampleEvent = []byte(`{}`) + metric.StatusDetail.SampleEvent = json.RawMessage(`{}`) metric.StatusDetail.SampleResponse = "" } else { err := r.eventSampler.Put([]byte(hash)) From e77447de9169b114e2fad6aa04c7d0718a85666f Mon Sep 17 00:00:00 2001 From: Mihir Khambhati Date: Tue, 26 Nov 2024 17:06:11 +0530 Subject: [PATCH 13/44] feat: aggregate reports based on the configured interval before sending to reporting --- enterprise/reporting/reporting.go | 17 +++- enterprise/reporting/reporting_test.go | 125 +++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 3 deletions(-) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index 80048b10d8..60ff30ac27 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -29,6 +29,7 @@ import ( migrator "github.com/rudderlabs/rudder-server/services/sql-migrator" "github.com/rudderlabs/rudder-server/utils/httputil" + "github.com/rudderlabs/rudder-server/utils/timeutil" . "github.com/rudderlabs/rudder-server/utils/tx" //nolint:staticcheck "github.com/rudderlabs/rudder-server/utils/types" ) @@ -51,6 +52,7 @@ const ( type DefaultReporter struct { ctx context.Context cancel context.CancelFunc + now func() time.Time g *errgroup.Group configSubscriber *configSubscriber syncersMu sync.RWMutex @@ -69,6 +71,7 @@ type DefaultReporter struct { maxOpenConnections int maxConcurrentRequests config.ValueLoader[int] vacuumFull config.ValueLoader[bool] + aggregationInterval config.ValueLoader[time.Duration] getMinReportedAtQueryTime stats.Measurement getReportsQueryTime stats.Measurement @@ -88,6 +91,7 @@ func NewDefaultReporter(ctx context.Context, log logger.Logger, configSubscriber maxConcurrentRequests := config.GetReloadableIntVar(32, 1, "Reporting.maxConcurrentRequests") maxOpenConnections := config.GetIntVar(32, 1, "Reporting.maxOpenConnections") dbQueryTimeout = config.GetReloadableDurationVar(60, time.Second, "Reporting.dbQueryTimeout") + aggregationInterval := config.GetReloadableDurationVar(1, time.Minute, "Reporting.aggregationInterval") // only send reports for wh actions sources if whActionsOnly is configured whActionsOnly := config.GetBool("REPORTING_WH_ACTIONS_ONLY", false) if whActionsOnly { @@ -98,6 +102,7 @@ func NewDefaultReporter(ctx context.Context, log logger.Logger, configSubscriber return &DefaultReporter{ ctx: ctx, cancel: cancel, + now: timeutil.Now, g: g, log: log, configSubscriber: configSubscriber, @@ -109,6 +114,7 @@ func NewDefaultReporter(ctx context.Context, log logger.Logger, configSubscriber sleepInterval: sleepInterval, mainLoopSleepInterval: mainLoopSleepInterval, vacuumFull: config.GetReloadableBoolVar(false, "Reporting.vacuumFull"), + aggregationInterval: aggregationInterval, region: config.GetString("region", ""), sourcesWithEventNameTrackingDisabled: sourcesWithEventNameTrackingDisabled, maxOpenConnections: maxOpenConnections, @@ -341,6 +347,11 @@ func (*DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) [] return values } +func (r *DefaultReporter) getAggregationBucketMin() int64 { + currentMin := r.now().Unix() / 60 + return currentMin - (currentMin % int64(r.aggregationInterval.Load().Minutes())) +} + func (r *DefaultReporter) emitLagMetric(ctx context.Context, c types.SyncerConfig, lastReportedAtTime *atomic.Time) error { // for monitoring reports pileups reportingLag := r.stats.NewTaggedStat( @@ -397,10 +408,10 @@ func (r *DefaultReporter) mainLoop(ctx context.Context, c types.SyncerConfig) { } requestChan := make(chan struct{}, r.maxConcurrentRequests.Load()) loopStart := time.Now() - currentMin := time.Now().UTC().Unix() / 60 + aggregationBucketMin := r.getAggregationBucketMin() getReportsStart := time.Now() - reports, reportedAt, err := r.getReports(currentMin, c.ConnInfo) + reports, reportedAt, err := r.getReports(aggregationBucketMin, c.ConnInfo) if err != nil { r.log.Errorw("getting reports", "error", err) select { @@ -645,7 +656,7 @@ func (r *DefaultReporter) Report(ctx context.Context, metrics []*types.PUReporte } defer func() { _ = stmt.Close() }() - reportedAt := time.Now().UTC().Unix() / 60 + reportedAt := r.getAggregationBucketMin() for _, metric := range metrics { workspaceID := r.configSubscriber.WorkspaceIDFromSource(metric.ConnectionDetails.SourceID) metric := *metric diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index f1c3a6457e..89368a3a2c 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "testing" + "time" "github.com/rudderlabs/rudder-go-kit/config" "github.com/rudderlabs/rudder-go-kit/stats" @@ -712,3 +713,127 @@ func TestAggregationLogic(t *testing.T) { require.Equal(t, reportResults, reportingMetrics) } + +func TestGetAggregationBucket(t *testing.T) { + configSubscriber := newConfigSubscriber(logger.NOP) + reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) + t.Run("should return the correct aggregation bucket with default interval of 1 mintue", func(t *testing.T) { + var cases = []struct { + now time.Time + bucket int64 + }{ + { + now: time.Date(2022, 1, 1, 10, 5, 10, 40, time.UTC), + bucket: time.Date(2022, 1, 1, 10, 5, 0, 0, time.UTC).Unix() / 60, + }, + { + now: time.Date(2022, 2, 4, 11, 5, 59, 10, time.UTC), + bucket: time.Date(2022, 2, 4, 11, 5, 0, 0, time.UTC).Unix() / 60, + }, + { + now: time.Date(2022, 3, 5, 12, 59, 59, 59, time.UTC), + bucket: time.Date(2022, 3, 5, 12, 59, 0, 0, time.UTC).Unix() / 60, + }, + { + now: time.Date(2022, 4, 6, 13, 0, 0, 0, time.UTC), + bucket: time.Date(2022, 4, 6, 13, 0, 0, 0, time.UTC).Unix() / 60, + }, + } + + for _, c := range cases { + reportHandle.now = func() time.Time { + return c.now + } + aggregationBucket := reportHandle.getAggregationBucketMin() + require.Equal(t, c.bucket, aggregationBucket) + } + }) + + t.Run("should return the correct aggregation bucket with aggregation interval of 5 mintue", func(t *testing.T) { + config.Set("reporting.aggregationInterval", 5) + var cases = []struct { + now time.Time + bucket int64 + }{ + { + now: time.Date(2022, 1, 1, 10, 5, 10, 40, time.UTC), + bucket: time.Date(2022, 1, 1, 10, 5, 0, 0, time.UTC).Unix() / 60, + }, + { + now: time.Date(2022, 2, 4, 11, 5, 59, 10, time.UTC), + bucket: time.Date(2022, 2, 4, 11, 5, 0, 0, time.UTC).Unix() / 60, + }, + { + now: time.Date(2022, 4, 6, 13, 7, 30, 11, time.UTC), + bucket: time.Date(2022, 4, 6, 13, 5, 0, 0, time.UTC).Unix() / 60, + }, + { + now: time.Date(2022, 4, 6, 13, 8, 50, 30, time.UTC), + bucket: time.Date(2022, 4, 6, 13, 5, 0, 0, time.UTC).Unix() / 60, + }, + { + now: time.Date(2022, 4, 6, 13, 9, 5, 15, time.UTC), + bucket: time.Date(2022, 4, 6, 13, 5, 0, 0, time.UTC).Unix() / 60, + }, + { + now: time.Date(2022, 3, 5, 12, 55, 53, 1, time.UTC), + bucket: time.Date(2022, 3, 5, 12, 55, 0, 0, time.UTC).Unix() / 60, + }, + { + now: time.Date(2022, 3, 5, 12, 57, 53, 1, time.UTC), + bucket: time.Date(2022, 3, 5, 12, 55, 0, 0, time.UTC).Unix() / 60, + }, + { + now: time.Date(2022, 3, 5, 12, 59, 59, 59, time.UTC), + bucket: time.Date(2022, 3, 5, 12, 55, 0, 0, time.UTC).Unix() / 60, + }, + { + now: time.Date(2022, 4, 6, 13, 0, 0, 0, time.UTC), + bucket: time.Date(2022, 4, 6, 13, 0, 0, 0, time.UTC).Unix() / 60, + }, + } + + for _, c := range cases { + reportHandle.now = func() time.Time { + return c.now + } + aggregationBucket := reportHandle.getAggregationBucketMin() + require.Equal(t, c.bucket, aggregationBucket) + } + }) + + t.Run("should return the correct aggregation bucket with aggregation interval of 15 mintue", func(t *testing.T) { + config.Set("reporting.aggregationInterval", 15) + var cases = []struct { + now time.Time + bucket int64 + }{ + { + now: time.Date(2022, 1, 1, 10, 5, 10, 40, time.UTC), + bucket: time.Date(2022, 1, 1, 10, 0, 0, 0, time.UTC).Unix() / 60, + }, + { + now: time.Date(2022, 2, 4, 11, 17, 59, 10, time.UTC), + bucket: time.Date(2022, 2, 4, 11, 15, 0, 0, time.UTC).Unix() / 60, + }, + { + now: time.Date(2022, 4, 6, 13, 39, 10, 59, time.UTC), + bucket: time.Date(2022, 4, 6, 13, 30, 0, 0, time.UTC).Unix() / 60, + }, + { + now: time.Date(2022, 4, 6, 13, 59, 50, 30, time.UTC), + bucket: time.Date(2022, 4, 6, 13, 45, 0, 0, time.UTC).Unix() / 60, + }, + } + + for _, c := range cases { + reportHandle.now = func() time.Time { + return c.now + } + aggregationBucket := reportHandle.getAggregationBucketMin() + require.Equal(t, c.bucket, aggregationBucket) + } + + }) + +} From 7ba69241573e26c0ac98e0bdbf858efacb3fca6c Mon Sep 17 00:00:00 2001 From: Mihir Khambhati Date: Tue, 26 Nov 2024 17:47:51 +0530 Subject: [PATCH 14/44] chore: rename aggregationInterval to aggregationIntervalMinutes --- enterprise/reporting/reporting.go | 2 +- enterprise/reporting/reporting_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index 60ff30ac27..9b22057087 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -91,7 +91,7 @@ func NewDefaultReporter(ctx context.Context, log logger.Logger, configSubscriber maxConcurrentRequests := config.GetReloadableIntVar(32, 1, "Reporting.maxConcurrentRequests") maxOpenConnections := config.GetIntVar(32, 1, "Reporting.maxOpenConnections") dbQueryTimeout = config.GetReloadableDurationVar(60, time.Second, "Reporting.dbQueryTimeout") - aggregationInterval := config.GetReloadableDurationVar(1, time.Minute, "Reporting.aggregationInterval") + aggregationInterval := config.GetReloadableDurationVar(1, time.Minute, "Reporting.aggregationIntervalMinutes") // only send reports for wh actions sources if whActionsOnly is configured whActionsOnly := config.GetBool("REPORTING_WH_ACTIONS_ONLY", false) if whActionsOnly { diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index 89368a3a2c..c9239ffd1b 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -750,7 +750,7 @@ func TestGetAggregationBucket(t *testing.T) { }) t.Run("should return the correct aggregation bucket with aggregation interval of 5 mintue", func(t *testing.T) { - config.Set("reporting.aggregationInterval", 5) + config.Set("reporting.aggregationIntervalMinutes", 5) var cases = []struct { now time.Time bucket int64 @@ -803,7 +803,7 @@ func TestGetAggregationBucket(t *testing.T) { }) t.Run("should return the correct aggregation bucket with aggregation interval of 15 mintue", func(t *testing.T) { - config.Set("reporting.aggregationInterval", 15) + config.Set("reporting.aggregationIntervalMinutes", 15) var cases = []struct { now time.Time bucket int64 From a7008e458e5e55c526b498e6795a7a89e16ded2c Mon Sep 17 00:00:00 2001 From: Mihir Khambhati Date: Wed, 27 Nov 2024 17:15:00 +0530 Subject: [PATCH 15/44] feat: modify query to get reports based on aggregation time --- enterprise/reporting/reporting.go | 33 ++++--- enterprise/reporting/reporting_test.go | 125 ++++++++++++++----------- 2 files changed, 88 insertions(+), 70 deletions(-) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index 9b22057087..70e8672065 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -29,7 +29,6 @@ import ( migrator "github.com/rudderlabs/rudder-server/services/sql-migrator" "github.com/rudderlabs/rudder-server/utils/httputil" - "github.com/rudderlabs/rudder-server/utils/timeutil" . "github.com/rudderlabs/rudder-server/utils/tx" //nolint:staticcheck "github.com/rudderlabs/rudder-server/utils/types" ) @@ -52,7 +51,6 @@ const ( type DefaultReporter struct { ctx context.Context cancel context.CancelFunc - now func() time.Time g *errgroup.Group configSubscriber *configSubscriber syncersMu sync.RWMutex @@ -102,7 +100,6 @@ func NewDefaultReporter(ctx context.Context, log logger.Logger, configSubscriber return &DefaultReporter{ ctx: ctx, cancel: cancel, - now: timeutil.Now, g: g, log: log, configSubscriber: configSubscriber, @@ -219,11 +216,16 @@ func (r *DefaultReporter) getReports(currentMs int64, syncerKey string) (reports return nil, 0, nil } - groupByColumns := "workspace_id, namespace, instance_id, source_definition_id, source_category, source_id, destination_definition_id, destination_id, source_task_run_id, source_job_id, source_job_run_id, transformation_id, transformation_version_id, tracking_plan_id, tracking_plan_version, in_pu, pu, reported_at, status, terminal_state, initial_state, status_code, event_name, event_type, error_type" - sqlStatement = fmt.Sprintf(`SELECT %s, (ARRAY_AGG(sample_response order by id))[1], (ARRAY_AGG(sample_event order by id))[1], SUM(count), SUM(violation_count) FROM %s WHERE reported_at = $1 GROUP BY %s`, groupByColumns, ReportsTable, groupByColumns) + bucketStart, bucketEnd := r.getAggregationBucketMinute(queryMin.Int64) + if bucketEnd > currentMs { + return nil, 0, nil + } + + groupByColumns := "workspace_id, namespace, instance_id, source_definition_id, source_category, source_id, destination_definition_id, destination_id, source_task_run_id, source_job_id, source_job_run_id, transformation_id, transformation_version_id, tracking_plan_id, tracking_plan_version, in_pu, pu, status, terminal_state, initial_state, status_code, event_name, event_type, error_type" + sqlStatement = fmt.Sprintf(`SELECT %s, MAX(reported_at), (ARRAY_AGG(sample_response order by id))[1], (ARRAY_AGG(sample_event order by id))[1], SUM(count), SUM(violation_count) FROM %s WHERE reported_at >= $1 and reported_at < $2 GROUP BY %s`, groupByColumns, ReportsTable, groupByColumns) var rows *sql.Rows queryStart = time.Now() - rows, err = dbHandle.Query(sqlStatement, queryMin.Int64) + rows, err = dbHandle.Query(sqlStatement, bucketStart, bucketEnd) if err != nil { panic(err) } @@ -249,12 +251,12 @@ func (r *DefaultReporter) getReports(currentMs int64, syncerKey string) (reports &metricReport.ConnectionDetails.TrackingPlanID, &metricReport.ConnectionDetails.TrackingPlanVersion, &metricReport.PUDetails.InPU, &metricReport.PUDetails.PU, - &metricReport.ReportedAt, &metricReport.StatusDetail.Status, &metricReport.PUDetails.TerminalPU, &metricReport.PUDetails.InitialPU, &metricReport.StatusDetail.StatusCode, &metricReport.StatusDetail.EventName, &metricReport.StatusDetail.EventType, &metricReport.StatusDetail.ErrorType, + &metricReport.ReportedAt, &metricReport.StatusDetail.SampleResponse, &metricReport.StatusDetail.SampleEvent, &metricReport.StatusDetail.Count, &metricReport.StatusDetail.ViolationCount, ) @@ -347,9 +349,11 @@ func (*DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) [] return values } -func (r *DefaultReporter) getAggregationBucketMin() int64 { - currentMin := r.now().Unix() / 60 - return currentMin - (currentMin % int64(r.aggregationInterval.Load().Minutes())) +func (r *DefaultReporter) getAggregationBucketMinute(timeMin int64) (int64, int64) { + bucketStart := timeMin - (timeMin % int64(r.aggregationInterval.Load().Minutes())) + bucketEnd := bucketStart + int64(r.aggregationInterval.Load().Minutes()) + + return bucketStart, bucketEnd } func (r *DefaultReporter) emitLagMetric(ctx context.Context, c types.SyncerConfig, lastReportedAtTime *atomic.Time) error { @@ -408,10 +412,10 @@ func (r *DefaultReporter) mainLoop(ctx context.Context, c types.SyncerConfig) { } requestChan := make(chan struct{}, r.maxConcurrentRequests.Load()) loopStart := time.Now() - aggregationBucketMin := r.getAggregationBucketMin() + currentMin := time.Now().UTC().Unix() / 60 getReportsStart := time.Now() - reports, reportedAt, err := r.getReports(aggregationBucketMin, c.ConnInfo) + reports, reportedAt, err := r.getReports(currentMin, c.ConnInfo) if err != nil { r.log.Errorw("getting reports", "error", err) select { @@ -470,7 +474,8 @@ func (r *DefaultReporter) mainLoop(ctx context.Context, c types.SyncerConfig) { if err != nil { return err } - _, err = dbHandle.Exec(`DELETE FROM `+ReportsTable+` WHERE reported_at = $1`, reportedAt) + bucketStart, bucketEnd := r.getAggregationBucketMinute(reportedAt) + _, err = dbHandle.Exec(`DELETE FROM `+ReportsTable+` WHERE reported_at >= $1 and reported_at < $2`, bucketStart, bucketEnd) if err != nil { r.log.Errorf(`[ Reporting ]: Error deleting local reports from %s: %v`, ReportsTable, err) } else { @@ -656,7 +661,7 @@ func (r *DefaultReporter) Report(ctx context.Context, metrics []*types.PUReporte } defer func() { _ = stmt.Close() }() - reportedAt := r.getAggregationBucketMin() + reportedAt := time.Now().UTC().Unix() / 60 for _, metric := range metrics { workspaceID := r.configSubscriber.WorkspaceIDFromSource(metric.ConnectionDetails.SourceID) metric := *metric diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index c9239ffd1b..dfaeaf290e 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -719,121 +719,134 @@ func TestGetAggregationBucket(t *testing.T) { reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) t.Run("should return the correct aggregation bucket with default interval of 1 mintue", func(t *testing.T) { var cases = []struct { - now time.Time - bucket int64 + reportedAt int64 + bucketStart int64 + bucketEnd int64 }{ { - now: time.Date(2022, 1, 1, 10, 5, 10, 40, time.UTC), - bucket: time.Date(2022, 1, 1, 10, 5, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 1, 1, 10, 5, 10, 40, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 1, 1, 10, 5, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 1, 1, 10, 6, 0, 0, time.UTC).Unix() / 60, }, { - now: time.Date(2022, 2, 4, 11, 5, 59, 10, time.UTC), - bucket: time.Date(2022, 2, 4, 11, 5, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 2, 4, 11, 5, 59, 10, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 2, 4, 11, 5, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 2, 4, 11, 6, 0, 0, time.UTC).Unix() / 60, }, { - now: time.Date(2022, 3, 5, 12, 59, 59, 59, time.UTC), - bucket: time.Date(2022, 3, 5, 12, 59, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 3, 5, 12, 59, 59, 59, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 3, 5, 12, 59, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 3, 5, 13, 0, 0, 0, time.UTC).Unix() / 60, }, { - now: time.Date(2022, 4, 6, 13, 0, 0, 0, time.UTC), - bucket: time.Date(2022, 4, 6, 13, 0, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 4, 6, 13, 0, 0, 0, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 4, 6, 13, 0, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 4, 6, 13, 1, 0, 0, time.UTC).Unix() / 60, }, } for _, c := range cases { - reportHandle.now = func() time.Time { - return c.now - } - aggregationBucket := reportHandle.getAggregationBucketMin() - require.Equal(t, c.bucket, aggregationBucket) + bs, be := reportHandle.getAggregationBucketMinute(c.reportedAt) + require.Equal(t, c.bucketStart, bs) + require.Equal(t, c.bucketEnd, be) } }) t.Run("should return the correct aggregation bucket with aggregation interval of 5 mintue", func(t *testing.T) { config.Set("reporting.aggregationIntervalMinutes", 5) var cases = []struct { - now time.Time - bucket int64 + reportedAt int64 + bucketStart int64 + bucketEnd int64 }{ { - now: time.Date(2022, 1, 1, 10, 5, 10, 40, time.UTC), - bucket: time.Date(2022, 1, 1, 10, 5, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 1, 1, 10, 5, 10, 40, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 1, 1, 10, 5, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 1, 1, 10, 10, 0, 0, time.UTC).Unix() / 60, }, { - now: time.Date(2022, 2, 4, 11, 5, 59, 10, time.UTC), - bucket: time.Date(2022, 2, 4, 11, 5, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 2, 4, 11, 5, 59, 10, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 2, 4, 11, 5, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 2, 4, 11, 10, 0, 0, time.UTC).Unix() / 60, }, { - now: time.Date(2022, 4, 6, 13, 7, 30, 11, time.UTC), - bucket: time.Date(2022, 4, 6, 13, 5, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 4, 6, 13, 7, 30, 11, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 4, 6, 13, 5, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 4, 6, 13, 10, 0, 0, time.UTC).Unix() / 60, }, { - now: time.Date(2022, 4, 6, 13, 8, 50, 30, time.UTC), - bucket: time.Date(2022, 4, 6, 13, 5, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 4, 6, 13, 8, 50, 30, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 4, 6, 13, 5, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 4, 6, 13, 10, 0, 0, time.UTC).Unix() / 60, }, { - now: time.Date(2022, 4, 6, 13, 9, 5, 15, time.UTC), - bucket: time.Date(2022, 4, 6, 13, 5, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 4, 6, 13, 9, 5, 15, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 4, 6, 13, 5, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 4, 6, 13, 10, 0, 0, time.UTC).Unix() / 60, }, { - now: time.Date(2022, 3, 5, 12, 55, 53, 1, time.UTC), - bucket: time.Date(2022, 3, 5, 12, 55, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 3, 5, 12, 55, 53, 1, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 3, 5, 12, 55, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 3, 5, 13, 0, 0, 0, time.UTC).Unix() / 60, }, { - now: time.Date(2022, 3, 5, 12, 57, 53, 1, time.UTC), - bucket: time.Date(2022, 3, 5, 12, 55, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 3, 5, 12, 57, 53, 1, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 3, 5, 12, 55, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 3, 5, 13, 0, 0, 0, time.UTC).Unix() / 60, }, { - now: time.Date(2022, 3, 5, 12, 59, 59, 59, time.UTC), - bucket: time.Date(2022, 3, 5, 12, 55, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 3, 5, 12, 59, 59, 59, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 3, 5, 12, 55, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 3, 5, 13, 0, 0, 0, time.UTC).Unix() / 60, }, { - now: time.Date(2022, 4, 6, 13, 0, 0, 0, time.UTC), - bucket: time.Date(2022, 4, 6, 13, 0, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 4, 6, 13, 0, 0, 0, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 4, 6, 13, 0, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 4, 6, 13, 5, 0, 0, time.UTC).Unix() / 60, }, } for _, c := range cases { - reportHandle.now = func() time.Time { - return c.now - } - aggregationBucket := reportHandle.getAggregationBucketMin() - require.Equal(t, c.bucket, aggregationBucket) + bs, be := reportHandle.getAggregationBucketMinute(c.reportedAt) + require.Equal(t, c.bucketStart, bs) + require.Equal(t, c.bucketEnd, be) } }) t.Run("should return the correct aggregation bucket with aggregation interval of 15 mintue", func(t *testing.T) { config.Set("reporting.aggregationIntervalMinutes", 15) var cases = []struct { - now time.Time - bucket int64 + reportedAt int64 + bucketStart int64 + bucketEnd int64 }{ { - now: time.Date(2022, 1, 1, 10, 5, 10, 40, time.UTC), - bucket: time.Date(2022, 1, 1, 10, 0, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 1, 1, 10, 5, 10, 40, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 1, 1, 10, 0, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 1, 1, 10, 15, 0, 0, time.UTC).Unix() / 60, }, { - now: time.Date(2022, 2, 4, 11, 17, 59, 10, time.UTC), - bucket: time.Date(2022, 2, 4, 11, 15, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 2, 4, 11, 17, 59, 10, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 2, 4, 11, 15, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 2, 4, 11, 30, 0, 0, time.UTC).Unix() / 60, }, { - now: time.Date(2022, 4, 6, 13, 39, 10, 59, time.UTC), - bucket: time.Date(2022, 4, 6, 13, 30, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 4, 6, 13, 39, 10, 59, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 4, 6, 13, 30, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 4, 6, 13, 45, 0, 0, time.UTC).Unix() / 60, }, { - now: time.Date(2022, 4, 6, 13, 59, 50, 30, time.UTC), - bucket: time.Date(2022, 4, 6, 13, 45, 0, 0, time.UTC).Unix() / 60, + reportedAt: time.Date(2022, 4, 6, 13, 59, 50, 30, time.UTC).Unix() / 60, + bucketStart: time.Date(2022, 4, 6, 13, 45, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 4, 6, 14, 0, 0, 0, time.UTC).Unix() / 60, }, } for _, c := range cases { - reportHandle.now = func() time.Time { - return c.now - } - aggregationBucket := reportHandle.getAggregationBucketMin() - require.Equal(t, c.bucket, aggregationBucket) + bs, be := reportHandle.getAggregationBucketMinute(c.reportedAt) + require.Equal(t, c.bucketStart, bs) + require.Equal(t, c.bucketEnd, be) } - }) } From 57454fe6221d6c5d2ed1d6ddd3c475d57e9c0fba Mon Sep 17 00:00:00 2001 From: Mihir Khambhati Date: Wed, 27 Nov 2024 17:26:50 +0530 Subject: [PATCH 16/44] fix: formatting --- enterprise/reporting/reporting_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index dfaeaf290e..4a1547d429 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -718,7 +718,7 @@ func TestGetAggregationBucket(t *testing.T) { configSubscriber := newConfigSubscriber(logger.NOP) reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) t.Run("should return the correct aggregation bucket with default interval of 1 mintue", func(t *testing.T) { - var cases = []struct { + cases := []struct { reportedAt int64 bucketStart int64 bucketEnd int64 @@ -754,7 +754,7 @@ func TestGetAggregationBucket(t *testing.T) { t.Run("should return the correct aggregation bucket with aggregation interval of 5 mintue", func(t *testing.T) { config.Set("reporting.aggregationIntervalMinutes", 5) - var cases = []struct { + cases := []struct { reportedAt int64 bucketStart int64 bucketEnd int64 @@ -815,7 +815,7 @@ func TestGetAggregationBucket(t *testing.T) { t.Run("should return the correct aggregation bucket with aggregation interval of 15 mintue", func(t *testing.T) { config.Set("reporting.aggregationIntervalMinutes", 15) - var cases = []struct { + cases := []struct { reportedAt int64 bucketStart int64 bucketEnd int64 @@ -848,5 +848,4 @@ func TestGetAggregationBucket(t *testing.T) { require.Equal(t, c.bucketEnd, be) } }) - } From 58b4f0ab57029fc5faed28a7084c91b7fff31ab7 Mon Sep 17 00:00:00 2001 From: Mihir Khambhati Date: Thu, 28 Nov 2024 11:30:31 +0530 Subject: [PATCH 17/44] fix: add aggregation time to the grouping identifiers --- enterprise/reporting/reporting.go | 2 ++ enterprise/reporting/reporting_test.go | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index e2e161edd0..5a163c97fd 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -293,6 +293,8 @@ func (r *DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) report.ConnectionDetails.TrackingPlanID, strconv.Itoa(report.ConnectionDetails.TrackingPlanVersion), report.PUDetails.InPU, report.PUDetails.PU, + strconv.FormatBool(report.TerminalPU), strconv.FormatBool(report.InitialPU), + strconv.FormatInt(report.ReportedAt, 10), } return strings.Join(groupingIdentifiers, `::`) } diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index 03f0892679..5e234f7118 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -817,8 +817,9 @@ func TestAggregationLogic(t *testing.T) { } func TestGetAggregationBucket(t *testing.T) { + conf := config.New() configSubscriber := newConfigSubscriber(logger.NOP) - reportHandle := NewDefaultReporter(context.Background(), logger.NOP, configSubscriber, stats.NOP) + reportHandle := NewDefaultReporter(context.Background(), conf, logger.NOP, configSubscriber, stats.NOP) t.Run("should return the correct aggregation bucket with default interval of 1 mintue", func(t *testing.T) { cases := []struct { reportedAt int64 From ff8fdbd07e68e97efe8970dc1efa220524b3cb5a Mon Sep 17 00:00:00 2001 From: Mihir Khambhati Date: Mon, 2 Dec 2024 23:04:53 +0530 Subject: [PATCH 18/44] fix: use aggregationIntervalMin value that was used to query the reports in getReports() --- enterprise/reporting/reporting.go | 20 ++++++++++---------- enterprise/reporting/reporting_test.go | 8 +++----- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index 5a163c97fd..1e5c70ceee 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -69,7 +69,6 @@ type DefaultReporter struct { maxOpenConnections int maxConcurrentRequests config.ValueLoader[int] vacuumFull config.ValueLoader[bool] - aggregationInterval config.ValueLoader[time.Duration] getMinReportedAtQueryTime stats.Measurement getReportsQueryTime stats.Measurement @@ -90,7 +89,6 @@ func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Log maxConcurrentRequests := config.GetReloadableIntVar(32, 1, "Reporting.maxConcurrentRequests") maxOpenConnections := config.GetIntVar(32, 1, "Reporting.maxOpenConnections") dbQueryTimeout = config.GetReloadableDurationVar(60, time.Second, "Reporting.dbQueryTimeout") - aggregationInterval := config.GetReloadableDurationVar(1, time.Minute, "Reporting.aggregationIntervalMinutes") maxReportsCountInARequest := conf.GetReloadableIntVar(10, 1, "Reporting.maxReportsCountInARequest") // only send reports for wh actions sources if whActionsOnly is configured whActionsOnly := config.GetBool("REPORTING_WH_ACTIONS_ONLY", false) @@ -113,7 +111,6 @@ func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Log sleepInterval: sleepInterval, mainLoopSleepInterval: mainLoopSleepInterval, vacuumFull: config.GetReloadableBoolVar(false, "Reporting.vacuumFull"), - aggregationInterval: aggregationInterval, region: config.GetString("region", ""), sourcesWithEventNameTrackingDisabled: sourcesWithEventNameTrackingDisabled, maxOpenConnections: maxOpenConnections, @@ -194,7 +191,7 @@ func (r *DefaultReporter) getDBHandle(syncerKey string) (*sql.DB, error) { return nil, fmt.Errorf("DBHandle not found for syncer key: %s", syncerKey) } -func (r *DefaultReporter) getReports(currentMs int64, syncerKey string) (reports []*types.ReportByStatus, reportedAt int64, err error) { +func (r *DefaultReporter) getReports(currentMs, aggregationInterval int64, syncerKey string) (reports []*types.ReportByStatus, reportedAt int64, err error) { sqlStatement := fmt.Sprintf(`SELECT min(reported_at) FROM %s WHERE reported_at < $1`, ReportsTable) var queryMin sql.NullInt64 dbHandle, err := r.getDBHandle(syncerKey) @@ -219,7 +216,7 @@ func (r *DefaultReporter) getReports(currentMs int64, syncerKey string) (reports return nil, 0, nil } - bucketStart, bucketEnd := r.getAggregationBucketMinute(queryMin.Int64) + bucketStart, bucketEnd := r.getAggregationBucketMinute(queryMin.Int64, aggregationInterval) if bucketEnd > currentMs { return nil, 0, nil } @@ -355,9 +352,9 @@ func (r *DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) return values } -func (r *DefaultReporter) getAggregationBucketMinute(timeMin int64) (int64, int64) { - bucketStart := timeMin - (timeMin % int64(r.aggregationInterval.Load().Minutes())) - bucketEnd := bucketStart + int64(r.aggregationInterval.Load().Minutes()) +func (*DefaultReporter) getAggregationBucketMinute(timeMin, aggregationIntervalMin int64) (int64, int64) { + bucketStart := timeMin - (timeMin % aggregationIntervalMin) + bucketEnd := bucketStart + aggregationIntervalMin return bucketStart, bucketEnd } @@ -410,6 +407,7 @@ func (r *DefaultReporter) mainLoop(ctx context.Context, c types.SyncerConfig) { lastVacuum time.Time vacuumInterval = config.GetReloadableDurationVar(15, time.Minute, "Reporting.vacuumInterval") vacuumThresholdBytes = config.GetReloadableInt64Var(10*bytesize.GB, 1, "Reporting.vacuumThresholdBytes") + aggregationInterval = config.GetReloadableDurationVar(1, time.Minute, "Reporting.aggregationIntervalMinutes") ) for { if ctx.Err() != nil { @@ -421,7 +419,8 @@ func (r *DefaultReporter) mainLoop(ctx context.Context, c types.SyncerConfig) { currentMin := time.Now().UTC().Unix() / 60 getReportsStart := time.Now() - reports, reportedAt, err := r.getReports(currentMin, c.ConnInfo) + aggregationIntervalMin := int64(aggregationInterval.Load().Minutes()) + reports, reportedAt, err := r.getReports(currentMin, aggregationIntervalMin, c.ConnInfo) if err != nil { r.log.Errorw("getting reports", "error", err) select { @@ -480,7 +479,8 @@ func (r *DefaultReporter) mainLoop(ctx context.Context, c types.SyncerConfig) { if err != nil { return err } - bucketStart, bucketEnd := r.getAggregationBucketMinute(reportedAt) + // Use the same aggregationIntervalMin value that was used to query the reports in getReports() + bucketStart, bucketEnd := r.getAggregationBucketMinute(reportedAt, aggregationIntervalMin) _, err = dbHandle.Exec(`DELETE FROM `+ReportsTable+` WHERE reported_at >= $1 and reported_at < $2`, bucketStart, bucketEnd) if err != nil { r.log.Errorf(`[ Reporting ]: Error deleting local reports from %s: %v`, ReportsTable, err) diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index 5e234f7118..ab9be03e07 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -849,14 +849,13 @@ func TestGetAggregationBucket(t *testing.T) { } for _, c := range cases { - bs, be := reportHandle.getAggregationBucketMinute(c.reportedAt) + bs, be := reportHandle.getAggregationBucketMinute(c.reportedAt, 1) require.Equal(t, c.bucketStart, bs) require.Equal(t, c.bucketEnd, be) } }) t.Run("should return the correct aggregation bucket with aggregation interval of 5 mintue", func(t *testing.T) { - config.Set("reporting.aggregationIntervalMinutes", 5) cases := []struct { reportedAt int64 bucketStart int64 @@ -910,14 +909,13 @@ func TestGetAggregationBucket(t *testing.T) { } for _, c := range cases { - bs, be := reportHandle.getAggregationBucketMinute(c.reportedAt) + bs, be := reportHandle.getAggregationBucketMinute(c.reportedAt, 5) require.Equal(t, c.bucketStart, bs) require.Equal(t, c.bucketEnd, be) } }) t.Run("should return the correct aggregation bucket with aggregation interval of 15 mintue", func(t *testing.T) { - config.Set("reporting.aggregationIntervalMinutes", 15) cases := []struct { reportedAt int64 bucketStart int64 @@ -946,7 +944,7 @@ func TestGetAggregationBucket(t *testing.T) { } for _, c := range cases { - bs, be := reportHandle.getAggregationBucketMinute(c.reportedAt) + bs, be := reportHandle.getAggregationBucketMinute(c.reportedAt, 15) require.Equal(t, c.bucketStart, bs) require.Equal(t, c.bucketEnd, be) } From c1c89e7c418ac314a3a7f59a74a54d40641a63e2 Mon Sep 17 00:00:00 2001 From: Mihir Khambhati Date: Mon, 2 Dec 2024 23:07:28 +0530 Subject: [PATCH 19/44] chore: rename aggregationInterval to aggregationIntervalMin --- enterprise/reporting/reporting.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index 1e5c70ceee..dc325a5eba 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -191,7 +191,7 @@ func (r *DefaultReporter) getDBHandle(syncerKey string) (*sql.DB, error) { return nil, fmt.Errorf("DBHandle not found for syncer key: %s", syncerKey) } -func (r *DefaultReporter) getReports(currentMs, aggregationInterval int64, syncerKey string) (reports []*types.ReportByStatus, reportedAt int64, err error) { +func (r *DefaultReporter) getReports(currentMs, aggregationIntervalMin int64, syncerKey string) (reports []*types.ReportByStatus, reportedAt int64, err error) { sqlStatement := fmt.Sprintf(`SELECT min(reported_at) FROM %s WHERE reported_at < $1`, ReportsTable) var queryMin sql.NullInt64 dbHandle, err := r.getDBHandle(syncerKey) @@ -216,7 +216,7 @@ func (r *DefaultReporter) getReports(currentMs, aggregationInterval int64, synce return nil, 0, nil } - bucketStart, bucketEnd := r.getAggregationBucketMinute(queryMin.Int64, aggregationInterval) + bucketStart, bucketEnd := r.getAggregationBucketMinute(queryMin.Int64, aggregationIntervalMin) if bucketEnd > currentMs { return nil, 0, nil } From ae9fcdff6cd1f6aa8ed4aec91cacbe38452cf55d Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Tue, 3 Dec 2024 12:34:59 +0530 Subject: [PATCH 20/44] chore: addressed review comments --- config/config.yaml | 3 --- enterprise/reporting/event_sampler.go | 12 ++++++------ enterprise/reporting/reporting.go | 24 +++++++++++++++++++++--- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/config/config.yaml b/config/config.yaml index e4705af27d..ccc45785e0 100644 --- a/config/config.yaml +++ b/config/config.yaml @@ -249,6 +249,3 @@ PgNotifier: retriggerCount: 500 trackBatchInterval: 2s maxAttempt: 3 -Reporting: - eventSamplingEnabled: false - eventSamplingDurationInMinutes: 60 diff --git a/enterprise/reporting/event_sampler.go b/enterprise/reporting/event_sampler.go index 53318ca24c..134b56a2bc 100644 --- a/enterprise/reporting/event_sampler.go +++ b/enterprise/reporting/event_sampler.go @@ -43,14 +43,14 @@ func NewEventSampler(pathName string, ttl config.ValueLoader[time.Duration], con WithCompression(options.None). WithIndexCacheSize(16 << 20). // 16mb WithNumGoroutines(1). - WithNumMemtables(conf.GetInt("BadgerDB.numMemtable", 5)). - WithValueThreshold(conf.GetInt64("BadgerDB.valueThreshold", 1048576)). WithBlockCacheSize(0). WithNumVersionsToKeep(1). - WithNumLevelZeroTables(conf.GetInt("BadgerDB.numLevelZeroTables", 5)). - WithNumLevelZeroTablesStall(conf.GetInt("BadgerDB.numLevelZeroTablesStall", 15)). - WithSyncWrites(conf.GetBool("BadgerDB.syncWrites", false)). - WithDetectConflicts(conf.GetBool("BadgerDB.detectConflicts", false)) + WithNumMemtables(conf.GetInt("Reporting.eventSampling.badgerDB.numMemtable", 5)). + WithValueThreshold(conf.GetInt64("Reporting.eventSampling.badgerDB.valueThreshold", 1048576)). + WithNumLevelZeroTables(conf.GetInt("Reporting.eventSampling.badgerDB.numLevelZeroTables", 5)). + WithNumLevelZeroTablesStall(conf.GetInt("Reporting.eventSampling.badgerDB.numLevelZeroTablesStall", 15)). + WithSyncWrites(conf.GetBool("Reporting.eventSampling.badgerDB.syncWrites", false)). + WithDetectConflicts(conf.GetBool("Reporting.eventSampling.badgerDB.detectConflicts", false)) ctx, cancel := context.WithCancel(context.Background()) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index 43b903bf9a..189405be0e 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -94,8 +94,8 @@ func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Log maxOpenConnections := config.GetIntVar(32, 1, "Reporting.maxOpenConnections") dbQueryTimeout = config.GetReloadableDurationVar(60, time.Second, "Reporting.dbQueryTimeout") maxReportsCountInARequest := conf.GetReloadableIntVar(10, 1, "Reporting.maxReportsCountInARequest") - eventSamplingEnabled := conf.GetReloadableBoolVar(false, "Reporting.eventSamplingEnabled") - eventSamplingDuration := conf.GetReloadableDurationVar(60, time.Minute, "Reporting.eventSamplingDurationInMinutes") + eventSamplingEnabled := conf.GetReloadableBoolVar(false, "Reporting.eventSampling.enabled") + eventSamplingDuration := conf.GetReloadableDurationVar(60, time.Minute, "Reporting.eventSampling.durationInMinutes") // only send reports for wh actions sources if whActionsOnly is configured whActionsOnly := config.GetBool("REPORTING_WH_ACTIONS_ONLY", false) if whActionsOnly { @@ -233,7 +233,25 @@ func (r *DefaultReporter) getReports(currentMs int64, syncerKey string) (reports } groupByColumns := "workspace_id, namespace, instance_id, source_definition_id, source_category, source_id, destination_definition_id, destination_id, source_task_run_id, source_job_id, source_job_run_id, transformation_id, transformation_version_id, tracking_plan_id, tracking_plan_version, in_pu, pu, reported_at, status, terminal_state, initial_state, status_code, event_name, event_type, error_type" - sqlStatement = fmt.Sprintf(`SELECT %s, (ARRAY_AGG(sample_response order by id))[1], (ARRAY_AGG(sample_event order by id))[1], SUM(count), SUM(violation_count) FROM %s WHERE reported_at = $1 GROUP BY %s`, groupByColumns, ReportsTable, groupByColumns) + sqlStatement = fmt.Sprintf(` + SELECT + %s, + COALESCE( + (ARRAY_AGG(sample_response ORDER BY id DESC) FILTER (WHERE sample_event != '{}'::jsonb))[1], + '' + ) AS sample_response, + COALESCE( + (ARRAY_AGG(sample_event ORDER BY id DESC) FILTER (WHERE sample_event != '{}'::jsonb))[1], + '{}'::jsonb + ) AS sample_event, + SUM(count), + SUM(violation_count) + FROM + %s + WHERE + reported_at = $1 + GROUP BY + %s`, groupByColumns, ReportsTable, groupByColumns) var rows *sql.Rows queryStart = time.Now() rows, err = dbHandle.Query(sqlStatement, queryMin.Int64) From 4af228c96c88771ffea13407448babc6eb4c8331 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Tue, 3 Dec 2024 12:37:16 +0530 Subject: [PATCH 21/44] fix: tests --- enterprise/reporting/event_sampler_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/enterprise/reporting/event_sampler_test.go b/enterprise/reporting/event_sampler_test.go index 62d6aa3130..37446c142c 100644 --- a/enterprise/reporting/event_sampler_test.go +++ b/enterprise/reporting/event_sampler_test.go @@ -12,7 +12,7 @@ import ( func TestPutAndGet(t *testing.T) { conf := config.New() - ttl := conf.GetReloadableDurationVar(1, time.Second, "Reporting.eventSamplingDurationInMinutes") + ttl := conf.GetReloadableDurationVar(1, time.Second, "Reporting.eventSampling.durationInMinutes") log := logger.NewLogger() es, _ := NewEventSampler("/test-reporting-badger", ttl, conf, log) @@ -33,7 +33,7 @@ func TestPutAndGet(t *testing.T) { func TestEvictionOnTTLExpiry(t *testing.T) { conf := config.New() - ttl := conf.GetReloadableDurationVar(500, time.Millisecond, "Reporting.eventSamplingDurationInMinutes") + ttl := conf.GetReloadableDurationVar(500, time.Millisecond, "Reporting.eventSampling.durationInMinutes") log := logger.NewLogger() es, _ := NewEventSampler("/test-reporting-badger", ttl, conf, log) From 864436446f572b1620ede2b6d6952b23922235b7 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Tue, 3 Dec 2024 18:15:56 +0530 Subject: [PATCH 22/44] chore: addressed review comments --- enterprise/reporting/reporting.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index bc11f592c7..b4d1ae4211 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -291,7 +291,7 @@ func (r *DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) for _, report := range reports { identifier := reportIdentifier(report) - if _, ok := metricsByGroup[identifier]; !ok { + if _, ok := metricsByGroup[identifier]; !ok || len(metricsByGroup[identifier].StatusDetails) >= r.maxReportsCountInARequest.Load() { metricsByGroup[identifier] = &types.Metric{ InstanceDetails: types.InstanceDetails{ WorkspaceID: report.WorkspaceID, @@ -336,10 +336,6 @@ func (r *DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) EventType: report.StatusDetail.EventType, ErrorType: report.StatusDetail.ErrorType, }) - - if len(metricsByGroup[identifier].StatusDetails) >= r.maxReportsCountInARequest.Load() { - delete(metricsByGroup, identifier) - } } return values From 931ff6c2bac80678d41d63897a5ee20bfba68c6e Mon Sep 17 00:00:00 2001 From: Mihir Khambhati Date: Wed, 4 Dec 2024 13:55:58 +0530 Subject: [PATCH 23/44] chore: add validation for aggregationIntervalMinutes --- enterprise/reporting/reporting.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index dc325a5eba..0066e32ed6 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -407,7 +407,7 @@ func (r *DefaultReporter) mainLoop(ctx context.Context, c types.SyncerConfig) { lastVacuum time.Time vacuumInterval = config.GetReloadableDurationVar(15, time.Minute, "Reporting.vacuumInterval") vacuumThresholdBytes = config.GetReloadableInt64Var(10*bytesize.GB, 1, "Reporting.vacuumThresholdBytes") - aggregationInterval = config.GetReloadableDurationVar(1, time.Minute, "Reporting.aggregationIntervalMinutes") + aggregationInterval = config.GetReloadableDurationVar(1, time.Minute, "Reporting.aggregationIntervalMinutes") // Values should be a factor of 60 or else we will panic, for example 1, 2, 3, 4, 5, 6, 10, 12, 15, 20, 30, 60 ) for { if ctx.Err() != nil { @@ -420,6 +420,10 @@ func (r *DefaultReporter) mainLoop(ctx context.Context, c types.SyncerConfig) { getReportsStart := time.Now() aggregationIntervalMin := int64(aggregationInterval.Load().Minutes()) + if aggregationIntervalMin == 0 || 60%aggregationIntervalMin != 0 { + panic(fmt.Errorf("[ Reporting ]: Error aggregationIntervalMinutes should be a factor of 60, got %d", aggregationIntervalMin)) + } + reports, reportedAt, err := r.getReports(currentMin, aggregationIntervalMin, c.ConnInfo) if err != nil { r.log.Errorw("getting reports", "error", err) From 2d904d6f54546d3bdbc33205d56338efd6718162 Mon Sep 17 00:00:00 2001 From: Mihir Khambhati Date: Wed, 4 Dec 2024 14:20:00 +0530 Subject: [PATCH 24/44] chore: check if aggregationIntervalMin is less than or equal to 0 --- enterprise/reporting/reporting.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index 31f4caca26..4a529834ab 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -416,7 +416,7 @@ func (r *DefaultReporter) mainLoop(ctx context.Context, c types.SyncerConfig) { getReportsStart := time.Now() aggregationIntervalMin := int64(aggregationInterval.Load().Minutes()) - if aggregationIntervalMin == 0 || 60%aggregationIntervalMin != 0 { + if aggregationIntervalMin <= 0 || 60%aggregationIntervalMin != 0 { panic(fmt.Errorf("[ Reporting ]: Error aggregationIntervalMinutes should be a factor of 60, got %d", aggregationIntervalMin)) } From dca10bdb69ae9ac53efd9612781fc7922245fc68 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Wed, 4 Dec 2024 16:17:43 +0530 Subject: [PATCH 25/44] chore: add in memory cache implementation --- .../badger_event_sampler.go} | 48 ++--- .../reporting/event_sampler/event_sampler.go | 46 +++++ .../event_sampler/event_sampler_test.go | 164 ++++++++++++++++++ .../in_memory_cache_event_sampler.go | 56 ++++++ enterprise/reporting/event_sampler_test.go | 50 ------ enterprise/reporting/reporting.go | 28 +-- 6 files changed, 309 insertions(+), 83 deletions(-) rename enterprise/reporting/{event_sampler.go => event_sampler/badger_event_sampler.go} (75%) create mode 100644 enterprise/reporting/event_sampler/event_sampler.go create mode 100644 enterprise/reporting/event_sampler/event_sampler_test.go create mode 100644 enterprise/reporting/event_sampler/in_memory_cache_event_sampler.go delete mode 100644 enterprise/reporting/event_sampler_test.go diff --git a/enterprise/reporting/event_sampler.go b/enterprise/reporting/event_sampler/badger_event_sampler.go similarity index 75% rename from enterprise/reporting/event_sampler.go rename to enterprise/reporting/event_sampler/badger_event_sampler.go index 134b56a2bc..8f3ff382f5 100644 --- a/enterprise/reporting/event_sampler.go +++ b/enterprise/reporting/event_sampler/badger_event_sampler.go @@ -1,4 +1,4 @@ -package reporting +package event_sampler import ( "context" @@ -15,7 +15,7 @@ import ( "github.com/rudderlabs/rudder-server/utils/misc" ) -type EventSampler struct { +type BadgerEventSampler struct { db *badger.DB mu sync.Mutex ttl config.ValueLoader[time.Duration] @@ -32,7 +32,7 @@ func DefaultPath(pathName string) (string, error) { return fmt.Sprintf(`%v%v`, tmpDirPath, pathName), nil } -func NewEventSampler(pathName string, ttl config.ValueLoader[time.Duration], conf *config.Config, log logger.Logger) (*EventSampler, error) { +func NewBadgerEventSampler(pathName string, ttl config.ValueLoader[time.Duration], conf *config.Config, log logger.Logger) (*BadgerEventSampler, error) { dbPath, err := DefaultPath(pathName) if err != nil || dbPath == "" { return nil, err @@ -56,7 +56,7 @@ func NewEventSampler(pathName string, ttl config.ValueLoader[time.Duration], con db, err := badger.Open(opts) - es := &EventSampler{ + es := &BadgerEventSampler{ db: db, ttl: ttl, ctx: ctx, @@ -73,14 +73,14 @@ func NewEventSampler(pathName string, ttl config.ValueLoader[time.Duration], con return es, nil } -func (es *EventSampler) Get(key []byte) (bool, error) { +func (es *BadgerEventSampler) Get(key string) (bool, error) { es.mu.Lock() defer es.mu.Unlock() var found bool err := es.db.View(func(txn *badger.Txn) error { - item, err := txn.Get(key) + item, err := txn.Get([]byte(key)) if err != nil { return err } @@ -98,39 +98,43 @@ func (es *EventSampler) Get(key []byte) (bool, error) { return found, nil } -func (es *EventSampler) Put(key []byte) error { +func (es *BadgerEventSampler) Put(key string) error { es.mu.Lock() defer es.mu.Unlock() return es.db.Update(func(txn *badger.Txn) error { - entry := badger.NewEntry(key, []byte{1}).WithTTL(es.ttl.Load()) + entry := badger.NewEntry([]byte(key), []byte{1}).WithTTL(es.ttl.Load()) return txn.SetEntry(entry) }) } -func (es *EventSampler) gcLoop() { +func (es *BadgerEventSampler) performGC() { + es.wg.Add(1) + defer es.wg.Done() + + // One call would only result in removal of at max one log file. + // As an optimization, we can call it in a loop until it returns an error. + for { + if err := es.db.RunValueLogGC(0.5); err != nil { + break + } + } +} + +func (es *BadgerEventSampler) gcLoop() { for { select { case <-es.ctx.Done(): - _ = es.db.RunValueLogGC(0.5) + es.performGC() return + case <-time.After(5 * time.Minute): - } - again: - if es.ctx.Err() != nil { - return - } - // One call would only result in removal of at max one log file. - // As an optimization, you could also immediately re-run it whenever it returns nil error - // (this is why `goto again` is used). - err := es.db.RunValueLogGC(0.5) - if err == nil { - goto again + es.performGC() } } } -func (es *EventSampler) Close() { +func (es *BadgerEventSampler) Close() { es.cancel() es.wg.Wait() if es.db != nil { diff --git a/enterprise/reporting/event_sampler/event_sampler.go b/enterprise/reporting/event_sampler/event_sampler.go new file mode 100644 index 0000000000..a91d49c685 --- /dev/null +++ b/enterprise/reporting/event_sampler/event_sampler.go @@ -0,0 +1,46 @@ +package event_sampler + +import ( + "fmt" + "time" + + "github.com/rudderlabs/rudder-go-kit/config" + "github.com/rudderlabs/rudder-go-kit/logger" +) + +const ( + BadgerTypeEventSampler = "badger" + InMemoryCacheTypeEventSampler = "in_memory_cache" + BadgerEventSamplerPathName = "/reporting-badger" +) + +type EventSampler interface { + Put(key string) error + Get(key string) (bool, error) + Close() +} + +func NewEventSampler( + ttl config.ValueLoader[time.Duration], + eventSamplerType config.ValueLoader[string], + eventSamplingCardinality config.ValueLoader[int], + conf *config.Config, + log logger.Logger, +) (EventSampler, error) { + var es EventSampler + var err error + + switch eventSamplerType.Load() { + case BadgerTypeEventSampler: + es, err = NewBadgerEventSampler(BadgerEventSamplerPathName, ttl, conf, log) + case InMemoryCacheTypeEventSampler: + es, err = NewInMemoryCacheEventSampler(ttl, eventSamplingCardinality) + default: + err = fmt.Errorf("invalid event sampler type: %s", eventSamplerType.Load()) + } + + if err != nil { + return nil, err + } + return es, nil +} diff --git a/enterprise/reporting/event_sampler/event_sampler_test.go b/enterprise/reporting/event_sampler/event_sampler_test.go new file mode 100644 index 0000000000..dfcb3f264c --- /dev/null +++ b/enterprise/reporting/event_sampler/event_sampler_test.go @@ -0,0 +1,164 @@ +package event_sampler + +import ( + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/rudderlabs/rudder-go-kit/config" + "github.com/rudderlabs/rudder-go-kit/logger" +) + +func TestBadger(t *testing.T) { + conf := config.New() + ttl := conf.GetReloadableDurationVar(500, time.Millisecond, "Reporting.eventSampling.durationInMinutes") + eventSamplerType := conf.GetReloadableStringVar("badger", "Reporting.eventSampling.type") + eventSamplingCardinality := conf.GetReloadableIntVar(10, 1, "Reporting.eventSampling.cardinality") + log := logger.NewLogger() + + t.Run("should put and get keys", func(t *testing.T) { + es, _ := NewEventSampler(ttl, eventSamplerType, eventSamplingCardinality, conf, log) + _ = es.Put("key1") + _ = es.Put("key2") + _ = es.Put("key3") + val1, _ := es.Get("key1") + val2, _ := es.Get("key2") + val3, _ := es.Get("key3") + val4, _ := es.Get("key4") + + assert.True(t, val1, "Expected key1 to be present") + assert.True(t, val2, "Expected key2 to be present") + assert.True(t, val3, "Expected key3 to be present") + assert.False(t, val4, "Expected key4 to not be present") + es.Close() + }) + + t.Run("should not get evicted keys", func(t *testing.T) { + es, _ := NewEventSampler(ttl, eventSamplerType, eventSamplingCardinality, conf, log) + _ = es.Put("key1") + + time.Sleep(600 * time.Millisecond) + val1, _ := es.Get("key1") + + assert.False(t, val1, "Expected key1 to be evicted") + es.Close() + }) +} + +func TestInMemoryCache(t *testing.T) { + conf := config.New() + ttl := conf.GetReloadableDurationVar(500, time.Millisecond, "Reporting.eventSampling.durationInMinutes") + eventSamplerType := conf.GetReloadableStringVar("in_memory_cache", "Reporting.eventSampling.type") + eventSamplingCardinality := conf.GetReloadableIntVar(3, 1, "Reporting.eventSampling.cardinality") + log := logger.NewLogger() + + t.Run("should put and get keys", func(t *testing.T) { + es, _ := NewEventSampler(ttl, eventSamplerType, eventSamplingCardinality, conf, log) + _ = es.Put("key1") + _ = es.Put("key2") + _ = es.Put("key3") + val1, _ := es.Get("key1") + val2, _ := es.Get("key2") + val3, _ := es.Get("key3") + val4, _ := es.Get("key4") + + assert.True(t, val1, "Expected key1 to be present") + assert.True(t, val2, "Expected key2 to be present") + assert.True(t, val3, "Expected key3 to be present") + assert.False(t, val4, "Expected key4 to not be present") + }) + + t.Run("should not get evicted keys", func(t *testing.T) { + es, _ := NewEventSampler(ttl, eventSamplerType, eventSamplingCardinality, conf, log) + _ = es.Put("key1") + + time.Sleep(600 * time.Millisecond) + + val1, _ := es.Get("key1") + + assert.False(t, val1, "Expected key1 to be evicted") + }) + + t.Run("should not add keys if length exceeds", func(t *testing.T) { + es, _ := NewEventSampler(ttl, eventSamplerType, eventSamplingCardinality, conf, log) + _ = es.Put("key1") + _ = es.Put("key2") + _ = es.Put("key3") + _ = es.Put("key4") + _ = es.Put("key5") + + val1, _ := es.Get("key1") + val2, _ := es.Get("key2") + val3, _ := es.Get("key3") + val4, _ := es.Get("key4") + val5, _ := es.Get("key5") + + assert.True(t, val1, "Expected key1 to be present") + assert.True(t, val2, "Expected key2 to be present") + assert.True(t, val3, "Expected key3 to be present") + assert.False(t, val4, "Expected key4 to not be added") + assert.False(t, val5, "Expected key5 to not be added") + }) +} + +func BenchmarkEventSampler(b *testing.B) { + testCases := []struct { + name string + eventSamplerType string + }{ + { + name: "Badger", + eventSamplerType: "badger", + }, + { + name: "InMemoryCache", + eventSamplerType: "in_memory_cache", + }, + } + + conf := config.New() + ttl := conf.GetReloadableDurationVar(1, time.Minute, "Reporting.eventSampling.durationInMinutes") + eventSamplerType := conf.GetReloadableStringVar("default", "Reporting.eventSampling.type") + eventSamplingCardinality := conf.GetReloadableIntVar(10, 1, "Reporting.eventSampling.cardinality") + log := logger.NewLogger() + + for _, tc := range testCases { + b.Run(tc.name, func(b *testing.B) { + conf.Set("Reporting.eventSampling.type", tc.eventSamplerType) + + eventSampler, err := NewEventSampler( + ttl, + eventSamplerType, + eventSamplingCardinality, + conf, + log, + ) + require.NoError(b, err) + + b.Run("Put", func(b *testing.B) { + for i := 0; i < b.N; i++ { + key := uuid.New().String() + err := eventSampler.Put(key) + require.NoError(b, err) + } + }) + + b.Run("Get", func(b *testing.B) { + for i := 0; i < b.N; i++ { + key := uuid.New().String() + + err := eventSampler.Put(key) + require.NoError(b, err) + + _, err = eventSampler.Get(key) + require.NoError(b, err) + } + }) + + eventSampler.Close() + }) + } +} diff --git a/enterprise/reporting/event_sampler/in_memory_cache_event_sampler.go b/enterprise/reporting/event_sampler/in_memory_cache_event_sampler.go new file mode 100644 index 0000000000..9267b5455c --- /dev/null +++ b/enterprise/reporting/event_sampler/in_memory_cache_event_sampler.go @@ -0,0 +1,56 @@ +package event_sampler + +import ( + "sync" + "time" + + "github.com/rudderlabs/rudder-go-kit/cachettl" + "github.com/rudderlabs/rudder-go-kit/config" +) + +type InMemoryCacheEventSampler struct { + cache *cachettl.Cache[string, bool] + mu sync.Mutex + ttl config.ValueLoader[time.Duration] + limit config.ValueLoader[int] + length int +} + +func NewInMemoryCacheEventSampler(ttl config.ValueLoader[time.Duration], limit config.ValueLoader[int]) (*InMemoryCacheEventSampler, error) { + c := cachettl.New[string, bool](cachettl.WithNoRefreshTTL) + + es := &InMemoryCacheEventSampler{ + cache: c, + ttl: ttl, + limit: limit, + length: 0, + } + + es.cache.OnEvicted(func(key string, value bool) { + es.length-- + }) + + return es, nil +} + +func (es *InMemoryCacheEventSampler) Get(key string) (bool, error) { + es.mu.Lock() + defer es.mu.Unlock() + value := es.cache.Get(key) + return value, nil +} + +func (es *InMemoryCacheEventSampler) Put(key string) error { + es.mu.Lock() + defer es.mu.Unlock() + if es.length >= es.limit.Load() { + return nil + } + + es.cache.Put(key, true, es.ttl.Load()) + es.length++ + return nil +} + +func (es *InMemoryCacheEventSampler) Close() { +} diff --git a/enterprise/reporting/event_sampler_test.go b/enterprise/reporting/event_sampler_test.go deleted file mode 100644 index 37446c142c..0000000000 --- a/enterprise/reporting/event_sampler_test.go +++ /dev/null @@ -1,50 +0,0 @@ -package reporting - -import ( - "testing" - "time" - - "github.com/stretchr/testify/assert" - - "github.com/rudderlabs/rudder-go-kit/config" - "github.com/rudderlabs/rudder-go-kit/logger" -) - -func TestPutAndGet(t *testing.T) { - conf := config.New() - ttl := conf.GetReloadableDurationVar(1, time.Second, "Reporting.eventSampling.durationInMinutes") - log := logger.NewLogger() - - es, _ := NewEventSampler("/test-reporting-badger", ttl, conf, log) - - _ = es.Put([]byte("key1")) - _ = es.Put([]byte("key2")) - _ = es.Put([]byte("key3")) - val1, _ := es.Get([]byte("key1")) - val2, _ := es.Get([]byte("key2")) - val3, _ := es.Get([]byte("key3")) - - assert.True(t, val1, "Expected key1 to be present") - assert.True(t, val2, "Expected key2 to be present") - assert.True(t, val3, "Expected key3 to be present") - - es.Close() -} - -func TestEvictionOnTTLExpiry(t *testing.T) { - conf := config.New() - ttl := conf.GetReloadableDurationVar(500, time.Millisecond, "Reporting.eventSampling.durationInMinutes") - log := logger.NewLogger() - - es, _ := NewEventSampler("/test-reporting-badger", ttl, conf, log) - - _ = es.Put([]byte("key1")) - - time.Sleep(600 * time.Millisecond) - - val1, _ := es.Get([]byte("key1")) - - assert.False(t, val1, "Expected key1 to be evicted") - - es.Close() -} diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index 189405be0e..893492810a 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -27,6 +27,7 @@ import ( "github.com/rudderlabs/rudder-go-kit/stats" obskit "github.com/rudderlabs/rudder-observability-kit/go/labels" + "github.com/rudderlabs/rudder-server/enterprise/reporting/event_sampler" migrator "github.com/rudderlabs/rudder-server/services/sql-migrator" "github.com/rudderlabs/rudder-server/utils/httputil" . "github.com/rudderlabs/rudder-server/utils/tx" //nolint:staticcheck @@ -77,12 +78,12 @@ type DefaultReporter struct { maxReportsCountInARequest config.ValueLoader[int] eventSamplingEnabled config.ValueLoader[bool] - eventSampler *EventSampler + eventSampler event_sampler.EventSampler } func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Logger, configSubscriber *configSubscriber, stats stats.Stats) *DefaultReporter { var dbQueryTimeout *config.Reloadable[time.Duration] - var eventSampler *EventSampler + var eventSampler event_sampler.EventSampler reportingServiceURL := config.GetString("REPORTING_URL", "https://reporting.rudderstack.com/") reportingServiceURL = strings.TrimSuffix(reportingServiceURL, "/") @@ -96,6 +97,8 @@ func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Log maxReportsCountInARequest := conf.GetReloadableIntVar(10, 1, "Reporting.maxReportsCountInARequest") eventSamplingEnabled := conf.GetReloadableBoolVar(false, "Reporting.eventSampling.enabled") eventSamplingDuration := conf.GetReloadableDurationVar(60, time.Minute, "Reporting.eventSampling.durationInMinutes") + eventSamplerType := conf.GetReloadableStringVar("badger", "Reporting.eventSampling.type") + eventSamplingCardinality := conf.GetReloadableIntVar(1000000, 1, "Reporting.eventSampling.cardinality") // only send reports for wh actions sources if whActionsOnly is configured whActionsOnly := config.GetBool("REPORTING_WH_ACTIONS_ONLY", false) if whActionsOnly { @@ -104,7 +107,7 @@ func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Log if eventSamplingEnabled.Load() { var err error - eventSampler, err = NewEventSampler("/reporting-badger", eventSamplingDuration, conf, log) + eventSampler, err = event_sampler.NewEventSampler(eventSamplingDuration, eventSamplerType, eventSamplingCardinality, conf, log) if err != nil { panic(err) } @@ -649,31 +652,31 @@ func transformMetricForPII(metric types.PUReportedMetric, piiColumns []string) t return metric } -func (r *DefaultReporter) transformMetricWithEventSampling(metric types.PUReportedMetric) types.PUReportedMetric { +func (r *DefaultReporter) transformMetricWithEventSampling(metric types.PUReportedMetric) (types.PUReportedMetric, error) { if r.eventSampler == nil { - return metric + return metric, nil } isValidSampleEvent := metric.StatusDetail.SampleEvent != nil && string(metric.StatusDetail.SampleEvent) != "{}" if isValidSampleEvent { hash := NewLabelSet(metric).generateHash() - found, err := r.eventSampler.Get([]byte(hash)) + found, err := r.eventSampler.Get(hash) if err != nil { - panic(err) + return metric, err } if found { metric.StatusDetail.SampleEvent = json.RawMessage(`{}`) metric.StatusDetail.SampleResponse = "" } else { - err := r.eventSampler.Put([]byte(hash)) + err := r.eventSampler.Put(hash) if err != nil { - panic(err) + return metric, err } } } - return metric + return metric, nil } func (r *DefaultReporter) Report(ctx context.Context, metrics []*types.PUReportedMetric, txn *Tx) error { @@ -724,7 +727,10 @@ func (r *DefaultReporter) Report(ctx context.Context, metrics []*types.PUReporte } if r.eventSamplingEnabled.Load() { - metric = r.transformMetricWithEventSampling(metric) + metric, err = r.transformMetricWithEventSampling(metric) + if err != nil { + return err + } } runeEventName := []rune(metric.StatusDetail.EventName) From 5e0e7e3b745817512d97d95639311486c031fec7 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Wed, 4 Dec 2024 16:42:39 +0530 Subject: [PATCH 26/44] fix: minor --- enterprise/reporting/label_set.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/enterprise/reporting/label_set.go b/enterprise/reporting/label_set.go index 6ee14d8f0a..d308c2495d 100644 --- a/enterprise/reporting/label_set.go +++ b/enterprise/reporting/label_set.go @@ -39,10 +39,10 @@ type LabelSet struct { func NewLabelSet(metric types.PUReportedMetric) LabelSet { return LabelSet{ WorkspaceID: metric.ConnectionDetails.SourceID, - SourceDefinitionID: metric.ConnectionDetails.SourceDefinitionId, + SourceDefinitionID: metric.ConnectionDetails.SourceDefinitionID, SourceCategory: metric.ConnectionDetails.SourceCategory, SourceID: metric.ConnectionDetails.SourceID, - DestinationDefinitionID: metric.ConnectionDetails.DestinationDefinitionId, + DestinationDefinitionID: metric.ConnectionDetails.DestinationDefinitionID, DestinationID: metric.ConnectionDetails.DestinationID, SourceTaskRunID: metric.ConnectionDetails.SourceTaskRunID, SourceJobID: metric.ConnectionDetails.SourceJobID, From 0c8a3b428adf8ba8331ad1935c5cf9f7e3c731f9 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Thu, 5 Dec 2024 15:35:53 +0530 Subject: [PATCH 27/44] chore: add bucket to label set and send it to reporting --- enterprise/reporting/label_set.go | 6 ++++-- enterprise/reporting/label_set_test.go | 16 ++++++++++------ enterprise/reporting/reporting.go | 9 ++++++--- utils/types/reporting_types.go | 1 + 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/enterprise/reporting/label_set.go b/enterprise/reporting/label_set.go index d308c2495d..eb491e568e 100644 --- a/enterprise/reporting/label_set.go +++ b/enterprise/reporting/label_set.go @@ -34,9 +34,10 @@ type LabelSet struct { EventName string EventType string ErrorType string + Bucket int64 } -func NewLabelSet(metric types.PUReportedMetric) LabelSet { +func NewLabelSet(metric types.PUReportedMetric, bucket int64) LabelSet { return LabelSet{ WorkspaceID: metric.ConnectionDetails.SourceID, SourceDefinitionID: metric.ConnectionDetails.SourceDefinitionID, @@ -60,11 +61,12 @@ func NewLabelSet(metric types.PUReportedMetric) LabelSet { EventName: metric.StatusDetail.EventName, EventType: metric.StatusDetail.EventType, ErrorType: metric.StatusDetail.ErrorType, + Bucket: bucket, } } func (labelSet LabelSet) generateHash() string { - data := labelSet.WorkspaceID + labelSet.SourceDefinitionID + labelSet.SourceCategory + labelSet.SourceID + labelSet.DestinationDefinitionID + labelSet.DestinationID + labelSet.SourceTaskRunID + labelSet.SourceJobID + labelSet.SourceJobRunID + labelSet.TransformationID + labelSet.TransformationVersionID + labelSet.TrackingPlanID + strconv.Itoa(labelSet.TrackingPlanVersion) + labelSet.InPU + labelSet.PU + labelSet.Status + strconv.FormatBool(labelSet.TerminalState) + strconv.FormatBool(labelSet.InitialState) + strconv.Itoa(labelSet.StatusCode) + labelSet.EventName + labelSet.EventType + labelSet.ErrorType + data := labelSet.WorkspaceID + labelSet.SourceDefinitionID + labelSet.SourceCategory + labelSet.SourceID + labelSet.DestinationDefinitionID + labelSet.DestinationID + labelSet.SourceTaskRunID + labelSet.SourceJobID + labelSet.SourceJobRunID + labelSet.TransformationID + labelSet.TransformationVersionID + labelSet.TrackingPlanID + strconv.Itoa(labelSet.TrackingPlanVersion) + labelSet.InPU + labelSet.PU + labelSet.Status + strconv.FormatBool(labelSet.TerminalState) + strconv.FormatBool(labelSet.InitialState) + strconv.Itoa(labelSet.StatusCode) + labelSet.EventName + labelSet.EventType + labelSet.ErrorType + strconv.FormatInt(labelSet.Bucket, 10) hash := murmur3.Sum64([]byte(data)) return hex.EncodeToString([]byte(strconv.FormatUint(hash, 16))) } diff --git a/enterprise/reporting/label_set_test.go b/enterprise/reporting/label_set_test.go index 6e77ecaacb..c4722761a0 100644 --- a/enterprise/reporting/label_set_test.go +++ b/enterprise/reporting/label_set_test.go @@ -33,7 +33,8 @@ func createMetricObject(eventName string) types.PUReportedMetric { func TestNewLabelSet(t *testing.T) { t.Run("should create the correct LabelSet from types.PUReportedMetric", func(t *testing.T) { inputMetric := createMetricObject("some-event-name") - labelSet := NewLabelSet(inputMetric) + bucket := int64(28889820) + labelSet := NewLabelSet(inputMetric, bucket) assert.Equal(t, "some-source-id", labelSet.SourceID) assert.Equal(t, "some-event-name", labelSet.EventName) // Default value @@ -41,7 +42,8 @@ func TestNewLabelSet(t *testing.T) { t.Run("should create the correct LabelSet with custom EventName", func(t *testing.T) { inputMetric := createMetricObject("custom-event-name") - labelSet := NewLabelSet(inputMetric) + bucket := int64(28889820) + labelSet := NewLabelSet(inputMetric, bucket) assert.Equal(t, "some-source-id", labelSet.SourceID) assert.Equal(t, "custom-event-name", labelSet.EventName) // Custom event name @@ -51,10 +53,11 @@ func TestNewLabelSet(t *testing.T) { func TestGenerateHash(t *testing.T) { t.Run("same hash for same LabelSet", func(t *testing.T) { inputMetric1 := createMetricObject("some-event-name") - labelSet1 := NewLabelSet(inputMetric1) + bucket := int64(28889820) + labelSet1 := NewLabelSet(inputMetric1, bucket) inputMetric2 := createMetricObject("some-event-name") - labelSet2 := NewLabelSet(inputMetric2) + labelSet2 := NewLabelSet(inputMetric2, bucket) hash1 := labelSet1.generateHash() hash2 := labelSet2.generateHash() @@ -64,10 +67,11 @@ func TestGenerateHash(t *testing.T) { t.Run("different hash for different LabelSet", func(t *testing.T) { inputMetric1 := createMetricObject("some-event-name-1") - labelSet1 := NewLabelSet(inputMetric1) + bucket := int64(28889820) + labelSet1 := NewLabelSet(inputMetric1, bucket) inputMetric2 := createMetricObject("some-event-name-2") - labelSet2 := NewLabelSet(inputMetric2) + labelSet2 := NewLabelSet(inputMetric2, bucket) hash1 := labelSet1.generateHash() hash2 := labelSet2.generateHash() diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index 5d255fd2e9..c6092374a1 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -312,6 +312,7 @@ func (r *DefaultReporter) getReports(currentMs, aggregationIntervalMin int64, sy func (r *DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) []*types.Metric { metricsByGroup := map[string]*types.Metric{} + bucket, _ := r.getAggregationBucketMinute(reports[0].ReportedAt, 1) var values []*types.Metric reportIdentifier := func(report *types.ReportByStatus) string { @@ -364,6 +365,7 @@ func (r *DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) }, ReportMetadata: types.ReportMetadata{ ReportedAt: report.ReportedAt * 60 * 1000, // send reportedAt in milliseconds + Bucket: bucket * 60 * 1000, }, } values = append(values, metricsByGroup[identifier]) @@ -670,7 +672,7 @@ func transformMetricForPII(metric types.PUReportedMetric, piiColumns []string) t return metric } -func (r *DefaultReporter) transformMetricWithEventSampling(metric types.PUReportedMetric) (types.PUReportedMetric, error) { +func (r *DefaultReporter) transformMetricWithEventSampling(metric types.PUReportedMetric, reportedAt int64) (types.PUReportedMetric, error) { if r.eventSampler == nil { return metric, nil } @@ -678,7 +680,8 @@ func (r *DefaultReporter) transformMetricWithEventSampling(metric types.PUReport isValidSampleEvent := metric.StatusDetail.SampleEvent != nil && string(metric.StatusDetail.SampleEvent) != "{}" if isValidSampleEvent { - hash := NewLabelSet(metric).generateHash() + bucket, _ := r.getAggregationBucketMinute(reportedAt, 1) + hash := NewLabelSet(metric, bucket).generateHash() found, err := r.eventSampler.Get(hash) if err != nil { return metric, err @@ -745,7 +748,7 @@ func (r *DefaultReporter) Report(ctx context.Context, metrics []*types.PUReporte } if r.eventSamplingEnabled.Load() { - metric, err = r.transformMetricWithEventSampling(metric) + metric, err = r.transformMetricWithEventSampling(metric, reportedAt) if err != nil { return err } diff --git a/utils/types/reporting_types.go b/utils/types/reporting_types.go index 2486dd1ace..e0f00a4853 100644 --- a/utils/types/reporting_types.go +++ b/utils/types/reporting_types.go @@ -82,6 +82,7 @@ type InstanceDetails struct { type ReportMetadata struct { ReportedAt int64 `json:"reportedAt"` + Bucket int64 `json:"bucket"` } type Metric struct { From 5f212c7afe6ed65325173d0d2f4142c05b8636a6 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Thu, 5 Dec 2024 16:28:09 +0530 Subject: [PATCH 28/44] chore: addressed review comments --- enterprise/reporting/reporting.go | 3 +- enterprise/reporting/reporting_test.go | 136 ++++++++++++++++++++++++- 2 files changed, 137 insertions(+), 2 deletions(-) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index abc9726933..e411c1079f 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -270,6 +270,7 @@ func (r *DefaultReporter) getReports(currentMs int64, syncerKey string) (reports func (r *DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) []*types.Metric { metricsByGroup := map[string]*types.Metric{} + maxReportsCountInARequest := r.maxReportsCountInARequest.Load() var values []*types.Metric reportIdentifier := func(report *types.ReportByStatus) string { @@ -291,7 +292,7 @@ func (r *DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) for _, report := range reports { identifier := reportIdentifier(report) - if _, ok := metricsByGroup[identifier]; !ok || len(metricsByGroup[identifier].StatusDetails) >= r.maxReportsCountInARequest.Load() { + if _, ok := metricsByGroup[identifier]; !ok || len(metricsByGroup[identifier].StatusDetails) >= maxReportsCountInARequest { metricsByGroup[identifier] = &types.Metric{ InstanceDetails: types.InstanceDetails{ WorkspaceID: report.WorkspaceID, diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index 3e6e90b9d2..9b326c8226 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -260,7 +260,7 @@ var _ = Describe("Reporting", func() { Expect(aggregatedMetrics).To(Equal(expectedResponse)) }) - It("Should provide aggregated reports when batch size is 10", func() { + It("Should provide aggregated reports when batch size more than 1", func() { conf.Set("Reporting.maxReportsCountInARequest", 10) Eventually(func() int { return reportHandle.maxReportsCountInARequest.Load() }).Should(Equal(10)) expectedResponse := []*types.Metric{ @@ -336,6 +336,140 @@ var _ = Describe("Reporting", func() { aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) Expect(aggregatedMetrics).To(Equal(expectedResponse)) }) + + It("Should provide aggregated reports when batch size is more than 1 and reports with same identifier are more then batch size", func() { + conf.Set("Reporting.maxReportsCountInARequest", 2) + Eventually(func() int { return reportHandle.maxReportsCountInARequest.Load() }).Should(Equal(2)) + extraReport := &types.ReportByStatus{ + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690, + }, + StatusDetail: &types.StatusDetail{ + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "another-error-type", + }, + } + newInputReports := append(inputReports, extraReport) + expectedResponse := []*types.Metric{ + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 5, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "", + }, + { + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, + }, + }, + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "another-error-type", + }, + }, + }, + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id-2", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, + }, + }, + } + + aggregatedMetrics := reportHandle.getAggregatedReports(newInputReports) + Expect(aggregatedMetrics).To(Equal(expectedResponse)) + }) }) }) From 0b56a899f6d3f130209975f61d1f0cb019fb171d Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Thu, 5 Dec 2024 17:06:26 +0530 Subject: [PATCH 29/44] chore: fix tests --- enterprise/reporting/reporting_test.go | 615 +++++++++++++------------ 1 file changed, 308 insertions(+), 307 deletions(-) diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index 9b326c8226..8bfbf7b3b7 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -10,6 +10,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/rudderlabs/rudder-go-kit/logger" @@ -74,9 +75,100 @@ var _ = Describe("Reporting", func() { assertReportMetric(expectedResponse, transformedMetric) }) }) +}) - Context("getAggregatedReports Tests", func() { - inputReports := []*types.ReportByStatus{ +func TestGetAggregatedReports(t *testing.T) { + inputReports := []*types.ReportByStatus{ + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690, + }, + StatusDetail: &types.StatusDetail{ + Status: "some-status", + Count: 3, + ViolationCount: 5, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "", + }, + }, + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690, + }, + StatusDetail: &types.StatusDetail{ + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, + }, + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id-2", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690, + }, + StatusDetail: &types.StatusDetail{ + Status: "some-status", + Count: 3, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, + }, + } + conf := config.New() + configSubscriber := newConfigSubscriber(logger.NOP) + reportHandle := NewDefaultReporter(context.Background(), conf, logger.NOP, configSubscriber, stats.NOP) + + t.Run("Should provide aggregated reports when batch size is 1", func(t *testing.T) { + conf.Set("Reporting.maxReportsCountInARequest", 1) + assert.Equal(t, 1, reportHandle.maxReportsCountInARequest.Load()) + expectedResponse := []*types.Metric{ { InstanceDetails: types.InstanceDetails{ WorkspaceID: "some-workspace-id", @@ -92,16 +184,18 @@ var _ = Describe("Reporting", func() { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690, + ReportedAt: 28017690 * 60 * 1000, }, - StatusDetail: &types.StatusDetail{ - Status: "some-status", - Count: 3, - ViolationCount: 5, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "", + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 5, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "", + }, }, }, { @@ -119,16 +213,18 @@ var _ = Describe("Reporting", func() { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690, + ReportedAt: 28017690 * 60 * 1000, }, - StatusDetail: &types.StatusDetail{ - Status: "some-status", - Count: 2, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, }, }, { @@ -146,201 +242,136 @@ var _ = Describe("Reporting", func() { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690, + ReportedAt: 28017690 * 60 * 1000, }, - StatusDetail: &types.StatusDetail{ - Status: "some-status", - Count: 3, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", + }, }, }, } - conf := config.New() - configSubscriber := newConfigSubscriber(logger.NOP) - reportHandle := NewDefaultReporter(context.Background(), conf, logger.NOP, configSubscriber, stats.NOP) - It("Should provide aggregated reports when batch size is 1", func() { - conf.Set("Reporting.maxReportsCountInARequest", 1) - Eventually(func() int { return reportHandle.maxReportsCountInARequest.Load() }).Should(Equal(1)) - expectedResponse := []*types.Metric{ - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 3, - ViolationCount: 5, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "", - }, - }, + aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) + assert.Equal(t, expectedResponse, aggregatedMetrics) + }) + + t.Run("Should provide aggregated reports when batch size more than 1", func(t *testing.T) { + conf.Set("Reporting.maxReportsCountInARequest", 10) + assert.Equal(t, 10, reportHandle.maxReportsCountInARequest.Load()) + expectedResponse := []*types.Metric{ + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", }, - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 2, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", - }, - }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", }, - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id-2", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 3, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", - }, - }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", }, - } - - aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) - Expect(aggregatedMetrics).To(Equal(expectedResponse)) - }) - - It("Should provide aggregated reports when batch size more than 1", func() { - conf.Set("Reporting.maxReportsCountInARequest", 10) - Eventually(func() int { return reportHandle.maxReportsCountInARequest.Load() }).Should(Equal(10)) - expectedResponse := []*types.Metric{ - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 3, - ViolationCount: 5, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "", - }, - { - Status: "some-status", - Count: 2, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", - }, - }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, }, - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id-2", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 5, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "", }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, + { + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 3, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", - }, + }, + }, + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id-2", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", }, }, - } + }, + } - aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) - Expect(aggregatedMetrics).To(Equal(expectedResponse)) - }) + aggregatedMetrics := reportHandle.getAggregatedReports(inputReports) + assert.Equal(t, expectedResponse, aggregatedMetrics) + }) - It("Should provide aggregated reports when batch size is more than 1 and reports with same identifier are more then batch size", func() { - conf.Set("Reporting.maxReportsCountInARequest", 2) - Eventually(func() int { return reportHandle.maxReportsCountInARequest.Load() }).Should(Equal(2)) - extraReport := &types.ReportByStatus{ + t.Run("Should provide aggregated reports when batch size is more than 1 and reports with same identifier are more then batch size", func(t *testing.T) { + conf.Set("Reporting.maxReportsCountInARequest", 2) + assert.Equal(t, 2, reportHandle.maxReportsCountInARequest.Load()) + extraReport := &types.ReportByStatus{ + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690, + }, + StatusDetail: &types.StatusDetail{ + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "another-error-type", + }, + } + newInputReports := append(inputReports, extraReport) + expectedResponse := []*types.Metric{ + { InstanceDetails: types.InstanceDetails{ WorkspaceID: "some-workspace-id", }, @@ -355,123 +386,93 @@ var _ = Describe("Reporting", func() { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690, - }, - StatusDetail: &types.StatusDetail{ - Status: "some-status", - Count: 2, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "another-error-type", + ReportedAt: 28017690 * 60 * 1000, }, - } - newInputReports := append(inputReports, extraReport) - expectedResponse := []*types.Metric{ - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 5, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "", }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 3, - ViolationCount: 5, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "", - }, - { - Status: "some-status", - Count: 2, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", - }, + { + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", }, }, - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 2, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "another-error-type", - }, - }, + }, + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", }, - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id-2", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id-2", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 3, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "some-error-type", }, - StatusDetails: []*types.StatusDetail{ - { - Status: "some-status", - Count: 3, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", - }, + }, + }, + { + InstanceDetails: types.InstanceDetails{ + WorkspaceID: "some-workspace-id", + }, + ConnectionDetails: types.ConnectionDetails{ + SourceID: "some-source-id", + DestinationID: "some-destination-id", + TransformationID: "some-transformation-id", + TrackingPlanID: "some-tracking-plan-id", + }, + PUDetails: types.PUDetails{ + InPU: "some-in-pu", + PU: "some-pu", + }, + ReportMetadata: types.ReportMetadata{ + ReportedAt: 28017690 * 60 * 1000, + }, + StatusDetails: []*types.StatusDetail{ + { + Status: "some-status", + Count: 2, + ViolationCount: 10, + StatusCode: 200, + SampleResponse: "", + SampleEvent: []byte(`{}`), + ErrorType: "another-error-type", }, }, - } + }, + } - aggregatedMetrics := reportHandle.getAggregatedReports(newInputReports) - Expect(aggregatedMetrics).To(Equal(expectedResponse)) - }) + aggregatedMetrics := reportHandle.getAggregatedReports(newInputReports) + assert.Equal(t, expectedResponse, aggregatedMetrics) }) -}) +} func assertReportMetric(expectedMetric, actualMetric types.PUReportedMetric) { Expect(expectedMetric.ConnectionDetails.SourceID).To(Equal(actualMetric.ConnectionDetails.SourceID)) From e738f735556f698ff737cc15483665dbe2e79c5d Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Thu, 5 Dec 2024 17:21:29 +0530 Subject: [PATCH 30/44] fix: tests --- enterprise/reporting/reporting_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index cfe1672682..578f3162f6 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -163,12 +163,14 @@ func TestGetAggregatedReports(t *testing.T) { }, } conf := config.New() + conf.Set("Reporting.aggregationIntervalMinutes", 10) configSubscriber := newConfigSubscriber(logger.NOP) reportHandle := NewDefaultReporter(context.Background(), conf, logger.NOP, configSubscriber, stats.NOP) t.Run("Should provide aggregated reports when batch size is 1", func(t *testing.T) { conf.Set("Reporting.maxReportsCountInARequest", 1) assert.Equal(t, 1, reportHandle.maxReportsCountInARequest.Load()) + bucket, _ := reportHandle.getAggregationBucketMinute(28017690, 10) expectedResponse := []*types.Metric{ { InstanceDetails: types.InstanceDetails{ @@ -186,6 +188,7 @@ func TestGetAggregatedReports(t *testing.T) { }, ReportMetadata: types.ReportMetadata{ ReportedAt: 28017690 * 60 * 1000, + Bucket: bucket * 60 * 1000, }, StatusDetails: []*types.StatusDetail{ { @@ -215,6 +218,7 @@ func TestGetAggregatedReports(t *testing.T) { }, ReportMetadata: types.ReportMetadata{ ReportedAt: 28017690 * 60 * 1000, + Bucket: bucket * 60 * 1000, }, StatusDetails: []*types.StatusDetail{ { @@ -244,6 +248,7 @@ func TestGetAggregatedReports(t *testing.T) { }, ReportMetadata: types.ReportMetadata{ ReportedAt: 28017690 * 60 * 1000, + Bucket: bucket * 60 * 1000, }, StatusDetails: []*types.StatusDetail{ { @@ -266,6 +271,7 @@ func TestGetAggregatedReports(t *testing.T) { t.Run("Should provide aggregated reports when batch size more than 1", func(t *testing.T) { conf.Set("Reporting.maxReportsCountInARequest", 10) assert.Equal(t, 10, reportHandle.maxReportsCountInARequest.Load()) + bucket, _ := reportHandle.getAggregationBucketMinute(28017690, 10) expectedResponse := []*types.Metric{ { InstanceDetails: types.InstanceDetails{ @@ -283,6 +289,7 @@ func TestGetAggregatedReports(t *testing.T) { }, ReportMetadata: types.ReportMetadata{ ReportedAt: 28017690 * 60 * 1000, + Bucket: bucket * 60 * 1000, }, StatusDetails: []*types.StatusDetail{ { @@ -321,6 +328,7 @@ func TestGetAggregatedReports(t *testing.T) { }, ReportMetadata: types.ReportMetadata{ ReportedAt: 28017690 * 60 * 1000, + Bucket: bucket * 60 * 1000, }, StatusDetails: []*types.StatusDetail{ { @@ -343,6 +351,7 @@ func TestGetAggregatedReports(t *testing.T) { t.Run("Should provide aggregated reports when batch size is more than 1 and reports with same identifier are more then batch size", func(t *testing.T) { conf.Set("Reporting.maxReportsCountInARequest", 2) assert.Equal(t, 2, reportHandle.maxReportsCountInARequest.Load()) + bucket, _ := reportHandle.getAggregationBucketMinute(28017690, 10) extraReport := &types.ReportByStatus{ InstanceDetails: types.InstanceDetails{ WorkspaceID: "some-workspace-id", @@ -388,6 +397,7 @@ func TestGetAggregatedReports(t *testing.T) { }, ReportMetadata: types.ReportMetadata{ ReportedAt: 28017690 * 60 * 1000, + Bucket: bucket * 60 * 1000, }, StatusDetails: []*types.StatusDetail{ { @@ -426,6 +436,7 @@ func TestGetAggregatedReports(t *testing.T) { }, ReportMetadata: types.ReportMetadata{ ReportedAt: 28017690 * 60 * 1000, + Bucket: bucket * 60 * 1000, }, StatusDetails: []*types.StatusDetail{ { @@ -455,6 +466,7 @@ func TestGetAggregatedReports(t *testing.T) { }, ReportMetadata: types.ReportMetadata{ ReportedAt: 28017690 * 60 * 1000, + Bucket: bucket * 60 * 1000, }, StatusDetails: []*types.StatusDetail{ { From 7492a63a276b6a0118236891513f52c5e3e30874 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Thu, 5 Dec 2024 18:44:20 +0530 Subject: [PATCH 31/44] fix: conflicts --- enterprise/reporting/label_set.go | 4 +- enterprise/reporting/reporting_test.go | 87 -------------------------- 2 files changed, 1 insertion(+), 90 deletions(-) diff --git a/enterprise/reporting/label_set.go b/enterprise/reporting/label_set.go index eb491e568e..8910763212 100644 --- a/enterprise/reporting/label_set.go +++ b/enterprise/reporting/label_set.go @@ -10,9 +10,7 @@ import ( ) type LabelSet struct { - WorkspaceID string - // Namespace string - // InstanceID string + WorkspaceID string SourceDefinitionID string SourceCategory string SourceID string diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index 2cbf11d2f9..578f3162f6 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -78,93 +78,6 @@ var _ = Describe("Reporting", func() { }) }) -func TestGetAggregatedReports(t *testing.T) { - inputReports := []*types.ReportByStatus{ - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690, - }, - StatusDetail: &types.StatusDetail{ - Status: "some-status", - Count: 3, - ViolationCount: 5, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "", - }, - }, - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690, - }, - StatusDetail: &types.StatusDetail{ - Status: "some-status", - Count: 2, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", - }, - }, - { - InstanceDetails: types.InstanceDetails{ - WorkspaceID: "some-workspace-id", - }, - ConnectionDetails: types.ConnectionDetails{ - SourceID: "some-source-id-2", - DestinationID: "some-destination-id", - TransformationID: "some-transformation-id", - TrackingPlanID: "some-tracking-plan-id", - }, - PUDetails: types.PUDetails{ - InPU: "some-in-pu", - PU: "some-pu", - }, - ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690, - }, - StatusDetail: &types.StatusDetail{ - Status: "some-status", - Count: 3, - ViolationCount: 10, - StatusCode: 200, - SampleResponse: "", - SampleEvent: []byte(`{}`), - ErrorType: "some-error-type", - }, - }, - } - conf := config.New() - configSubscriber := newConfigSubscriber(logger.NOP) - reportHandle := NewDefaultReporter(context.Background(), conf, logger.NOP, configSubscriber, stats.NOP) func TestGetAggregatedReports(t *testing.T) { inputReports := []*types.ReportByStatus{ { From c9e235adfc3f3b92ff469901f104e3b1870dbb1f Mon Sep 17 00:00:00 2001 From: Mihir Khambhati Date: Fri, 6 Dec 2024 12:47:25 +0530 Subject: [PATCH 32/44] feat: round aggregation interval to the nearest factor of 60 --- enterprise/reporting/reporting.go | 27 ++++++--- enterprise/reporting/reporting_test.go | 80 ++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 7 deletions(-) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index 1022830711..f4d52e245c 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -217,6 +217,7 @@ func (r *DefaultReporter) getReports(currentMs, aggregationIntervalMin int64, sy } bucketStart, bucketEnd := r.getAggregationBucketMinute(queryMin.Int64, aggregationIntervalMin) + // we don't want to flush partial buckets, so we wait for the current bucket to be complete if bucketEnd > currentMs { return nil, 0, nil } @@ -349,9 +350,25 @@ func (r *DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) return values } -func (*DefaultReporter) getAggregationBucketMinute(timeMin, aggregationIntervalMin int64) (int64, int64) { - bucketStart := timeMin - (timeMin % aggregationIntervalMin) - bucketEnd := bucketStart + aggregationIntervalMin +func (*DefaultReporter) getAggregationBucketMinute(timeMs, intervalMs int64) (int64, int64) { + // If interval is not a factor of 60, then the bucket start will not be aligned to hour start + // For example, if intervalMs is 7, and timeMs is 28891085 (6:05) then the bucket start will be 28891079 (5:59) + // To avoid this, we round the intervalMs to the nearest factor of 60. + if intervalMs <= 0 || 60%intervalMs != 0 { + factors := []int64{1, 2, 3, 4, 5, 6, 10, 12, 15, 20, 30, 60} + closestFactor := factors[0] + for _, factor := range factors { + if factor < intervalMs { + closestFactor = factor + } else { + break + } + } + intervalMs = closestFactor + } + + bucketStart := timeMs - (timeMs % intervalMs) + bucketEnd := bucketStart + intervalMs return bucketStart, bucketEnd } @@ -417,10 +434,6 @@ func (r *DefaultReporter) mainLoop(ctx context.Context, c types.SyncerConfig) { getReportsStart := time.Now() aggregationIntervalMin := int64(aggregationInterval.Load().Minutes()) - if aggregationIntervalMin <= 0 || 60%aggregationIntervalMin != 0 { - panic(fmt.Errorf("[ Reporting ]: Error aggregationIntervalMinutes should be a factor of 60, got %d", aggregationIntervalMin)) - } - reports, reportedAt, err := r.getReports(currentMin, aggregationIntervalMin, c.ConnInfo) if err != nil { r.log.Errorw("getting reports", "error", err) diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index cfe1672682..03403226fe 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -1097,4 +1097,84 @@ func TestGetAggregationBucket(t *testing.T) { require.Equal(t, c.bucketEnd, be) } }) + + t.Run("should choose closest factor of 60 if interval is non positive and return the correct aggregation bucket", func(t *testing.T) { + cases := []struct { + reportedAt int64 + interval int64 + bucketStart int64 + bucketEnd int64 + }{ + { + reportedAt: time.Date(2022, 1, 1, 12, 5, 10, 40, time.UTC).Unix() / 60, + interval: -1, // it should round to 1 + bucketStart: time.Date(2022, 1, 1, 12, 5, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 1, 1, 12, 6, 0, 0, time.UTC).Unix() / 60, + }, + { + reportedAt: time.Date(2022, 2, 29, 10, 0, 2, 59, time.UTC).Unix() / 60, + interval: -1, // it should round to 1 + bucketStart: time.Date(2022, 2, 29, 10, 0, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 2, 29, 10, 1, 0, 0, time.UTC).Unix() / 60, + }, + { + reportedAt: time.Date(2022, 2, 10, 0, 0, 0, 40, time.UTC).Unix() / 60, + interval: 0, // it should round to 1 + bucketStart: time.Date(2022, 2, 10, 0, 0, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 2, 10, 0, 1, 0, 0, time.UTC).Unix() / 60, + }, + { + reportedAt: time.Date(2022, 11, 27, 23, 59, 59, 40, time.UTC).Unix() / 60, + interval: 0, // it should round to 1 + bucketStart: time.Date(2022, 11, 27, 23, 59, 59, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 11, 28, 0, 0, 0, 0, time.UTC).Unix() / 60, + }, + } + + for _, c := range cases { + bs, be := reportHandle.getAggregationBucketMinute(c.reportedAt, c.interval) + require.Equal(t, c.bucketStart, bs) + require.Equal(t, c.bucketEnd, be) + } + }) + + t.Run("should choose closest factor of 60 if interval is not a factor of 60 and return the correct aggregation bucket", func(t *testing.T) { + cases := []struct { + reportedAt int64 + interval int64 + bucketStart int64 + bucketEnd int64 + }{ + { + reportedAt: time.Date(2022, 1, 1, 10, 23, 10, 40, time.UTC).Unix() / 60, + interval: 7, // it should round to 6 + bucketStart: time.Date(2022, 1, 1, 10, 18, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 1, 1, 10, 24, 0, 0, time.UTC).Unix() / 60, + }, + { + reportedAt: time.Date(2022, 1, 1, 10, 5, 10, 40, time.UTC).Unix() / 60, + interval: 14, // it should round to 12 + bucketStart: time.Date(2022, 1, 1, 10, 0, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 1, 1, 10, 12, 0, 0, time.UTC).Unix() / 60, + }, + { + reportedAt: time.Date(2022, 1, 1, 10, 39, 10, 40, time.UTC).Unix() / 60, + interval: 59, // it should round to 30 + bucketStart: time.Date(2022, 1, 1, 10, 30, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 1, 1, 11, 0, 0, 0, time.UTC).Unix() / 60, + }, + { + reportedAt: time.Date(2022, 1, 1, 10, 5, 10, 40, time.UTC).Unix() / 60, + interval: 63, // it should round to 60 + bucketStart: time.Date(2022, 1, 1, 10, 0, 0, 0, time.UTC).Unix() / 60, + bucketEnd: time.Date(2022, 1, 1, 11, 0, 0, 0, time.UTC).Unix() / 60, + }, + } + + for _, c := range cases { + bs, be := reportHandle.getAggregationBucketMinute(c.reportedAt, c.interval) + require.Equal(t, c.bucketStart, bs) + require.Equal(t, c.bucketEnd, be) + } + }) } From b1bf7790793da0c994afb659fb663d7a8c9190fe Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Fri, 6 Dec 2024 13:15:36 +0530 Subject: [PATCH 33/44] chore: addressed review comments --- .../event_sampler/badger_event_sampler.go | 16 ++++----- .../event_sampler/event_sampler_test.go | 33 ++++++++++++------- enterprise/reporting/reporting.go | 10 +++--- 3 files changed, 34 insertions(+), 25 deletions(-) diff --git a/enterprise/reporting/event_sampler/badger_event_sampler.go b/enterprise/reporting/event_sampler/badger_event_sampler.go index 8f3ff382f5..fa5696b4a1 100644 --- a/enterprise/reporting/event_sampler/badger_event_sampler.go +++ b/enterprise/reporting/event_sampler/badger_event_sampler.go @@ -21,7 +21,6 @@ type BadgerEventSampler struct { ttl config.ValueLoader[time.Duration] ctx context.Context cancel context.CancelFunc - wg sync.WaitGroup } func DefaultPath(pathName string) (string, error) { @@ -61,7 +60,6 @@ func NewBadgerEventSampler(pathName string, ttl config.ValueLoader[time.Duration ttl: ttl, ctx: ctx, cancel: cancel, - wg: sync.WaitGroup{}, } if err != nil { @@ -109,14 +107,16 @@ func (es *BadgerEventSampler) Put(key string) error { } func (es *BadgerEventSampler) performGC() { - es.wg.Add(1) - defer es.wg.Done() - // One call would only result in removal of at max one log file. // As an optimization, we can call it in a loop until it returns an error. for { - if err := es.db.RunValueLogGC(0.5); err != nil { - break + select { + case <-es.ctx.Done(): + return + default: + if err := es.db.RunValueLogGC(0.5); err != nil { + return + } } } } @@ -125,7 +125,6 @@ func (es *BadgerEventSampler) gcLoop() { for { select { case <-es.ctx.Done(): - es.performGC() return case <-time.After(5 * time.Minute): @@ -136,7 +135,6 @@ func (es *BadgerEventSampler) gcLoop() { func (es *BadgerEventSampler) Close() { es.cancel() - es.wg.Wait() if es.db != nil { _ = es.db.Close() } diff --git a/enterprise/reporting/event_sampler/event_sampler_test.go b/enterprise/reporting/event_sampler/event_sampler_test.go index dfcb3f264c..e4b4d646bd 100644 --- a/enterprise/reporting/event_sampler/event_sampler_test.go +++ b/enterprise/reporting/event_sampler/event_sampler_test.go @@ -14,12 +14,13 @@ import ( func TestBadger(t *testing.T) { conf := config.New() - ttl := conf.GetReloadableDurationVar(500, time.Millisecond, "Reporting.eventSampling.durationInMinutes") + ttl := conf.GetReloadableDurationVar(3000, time.Millisecond, "Reporting.eventSampling.durationInMinutes") eventSamplerType := conf.GetReloadableStringVar("badger", "Reporting.eventSampling.type") eventSamplingCardinality := conf.GetReloadableIntVar(10, 1, "Reporting.eventSampling.cardinality") log := logger.NewLogger() t.Run("should put and get keys", func(t *testing.T) { + assert.Equal(t, 3000*time.Millisecond, ttl.Load()) es, _ := NewEventSampler(ttl, eventSamplerType, eventSamplingCardinality, conf, log) _ = es.Put("key1") _ = es.Put("key2") @@ -37,25 +38,30 @@ func TestBadger(t *testing.T) { }) t.Run("should not get evicted keys", func(t *testing.T) { + conf.Set("Reporting.eventSampling.durationInMinutes", 100) + assert.Equal(t, 100*time.Millisecond, ttl.Load()) + es, _ := NewEventSampler(ttl, eventSamplerType, eventSamplingCardinality, conf, log) - _ = es.Put("key1") + defer es.Close() - time.Sleep(600 * time.Millisecond) - val1, _ := es.Get("key1") + _ = es.Put("key1") - assert.False(t, val1, "Expected key1 to be evicted") - es.Close() + require.Eventually(t, func() bool { + val1, _ := es.Get("key1") + return !val1 + }, 1*time.Second, 50*time.Millisecond) }) } func TestInMemoryCache(t *testing.T) { conf := config.New() - ttl := conf.GetReloadableDurationVar(500, time.Millisecond, "Reporting.eventSampling.durationInMinutes") eventSamplerType := conf.GetReloadableStringVar("in_memory_cache", "Reporting.eventSampling.type") eventSamplingCardinality := conf.GetReloadableIntVar(3, 1, "Reporting.eventSampling.cardinality") + ttl := conf.GetReloadableDurationVar(3000, time.Millisecond, "Reporting.eventSampling.durationInMinutes") log := logger.NewLogger() t.Run("should put and get keys", func(t *testing.T) { + assert.Equal(t, 3000*time.Millisecond, ttl.Load()) es, _ := NewEventSampler(ttl, eventSamplerType, eventSamplingCardinality, conf, log) _ = es.Put("key1") _ = es.Put("key2") @@ -72,17 +78,20 @@ func TestInMemoryCache(t *testing.T) { }) t.Run("should not get evicted keys", func(t *testing.T) { + conf.Set("Reporting.eventSampling.durationInMinutes", 100) + assert.Equal(t, 100*time.Millisecond, ttl.Load()) es, _ := NewEventSampler(ttl, eventSamplerType, eventSamplingCardinality, conf, log) _ = es.Put("key1") - time.Sleep(600 * time.Millisecond) - - val1, _ := es.Get("key1") - - assert.False(t, val1, "Expected key1 to be evicted") + require.Eventually(t, func() bool { + val1, _ := es.Get("key1") + return !val1 + }, 1*time.Second, 50*time.Millisecond) }) t.Run("should not add keys if length exceeds", func(t *testing.T) { + conf.Set("Reporting.eventSampling.durationInMinutes", 3000) + assert.Equal(t, 3000*time.Millisecond, ttl.Load()) es, _ := NewEventSampler(ttl, eventSamplerType, eventSamplingCardinality, conf, log) _ = es.Put("key1") _ = es.Put("key2") diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index ca21d965d0..1ea25e1a0e 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -77,8 +77,9 @@ type DefaultReporter struct { stats stats.Stats maxReportsCountInARequest config.ValueLoader[int] - eventSamplingEnabled config.ValueLoader[bool] - eventSampler event_sampler.EventSampler + eventSamplingEnabled config.ValueLoader[bool] + eventSamplingDuration config.ValueLoader[time.Duration] + eventSampler event_sampler.EventSampler } func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Logger, configSubscriber *configSubscriber, stats stats.Stats) *DefaultReporter { @@ -136,6 +137,7 @@ func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Log maxReportsCountInARequest: maxReportsCountInARequest, stats: stats, eventSamplingEnabled: eventSamplingEnabled, + eventSamplingDuration: eventSamplingDuration, eventSampler: eventSampler, } } @@ -681,8 +683,8 @@ func (r *DefaultReporter) transformMetricWithEventSampling(metric types.PUReport isValidSampleEvent := metric.StatusDetail.SampleEvent != nil && string(metric.StatusDetail.SampleEvent) != "{}" if isValidSampleEvent { - bucket, _ := r.getAggregationBucketMinute(reportedAt, 1) - hash := NewLabelSet(metric, bucket).generateHash() + sampleEventBucket, _ := r.getAggregationBucketMinute(reportedAt, int64(r.eventSamplingDuration.Load().Minutes())) + hash := NewLabelSet(metric, sampleEventBucket).generateHash() found, err := r.eventSampler.Get(hash) if err != nil { return metric, err From 177b83fc7cfc9095c07e0fa3237ec72db650043b Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Fri, 6 Dec 2024 13:21:23 +0530 Subject: [PATCH 34/44] fix: minor --- enterprise/reporting/reporting.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index c276c45486..058f559dbd 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -316,7 +316,7 @@ func (r *DefaultReporter) getReports(currentMs, aggregationIntervalMin int64, sy func (r *DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) []*types.Metric { metricsByGroup := map[string]*types.Metric{} maxReportsCountInARequest := r.maxReportsCountInARequest.Load() - bucket, _ := r.getAggregationBucketMinute(reports[0].ReportedAt, 1) + sampleEventBucket, _ := r.getAggregationBucketMinute(reports[0].ReportedAt, int64(r.eventSamplingDuration.Load().Minutes())) var values []*types.Metric reportIdentifier := func(report *types.ReportByStatus) string { @@ -369,7 +369,7 @@ func (r *DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) }, ReportMetadata: types.ReportMetadata{ ReportedAt: report.ReportedAt * 60 * 1000, // send reportedAt in milliseconds - Bucket: bucket * 60 * 1000, + Bucket: sampleEventBucket * 60 * 1000, }, } values = append(values, metricsByGroup[identifier]) From fad3dda70ca0648bad2b7d7cf528dd912e29bdfe Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Fri, 6 Dec 2024 13:28:48 +0530 Subject: [PATCH 35/44] fix: tests --- enterprise/reporting/reporting_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index e445ae57c4..b108ab1a1e 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -163,7 +163,7 @@ func TestGetAggregatedReports(t *testing.T) { }, } conf := config.New() - conf.Set("Reporting.aggregationIntervalMinutes", 10) + conf.Set("Reporting.eventSampling.durationInMinutes", 10) configSubscriber := newConfigSubscriber(logger.NOP) reportHandle := NewDefaultReporter(context.Background(), conf, logger.NOP, configSubscriber, stats.NOP) From 6f9665cecf6d92b03fa7c5b13a5c03f1f7e60891 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Fri, 6 Dec 2024 13:31:24 +0530 Subject: [PATCH 36/44] chore: addressed review comments --- docker-compose.yml | 31 +++---- enterprise/reporting/reporting.go | 4 +- enterprise/reporting/reporting_test.go | 32 ++++---- scripts/generate-event | 107 ++++++++++++++++++++++--- utils/types/reporting_types.go | 4 +- 5 files changed, 135 insertions(+), 43 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 157caa9913..e99e97a35a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,20 +7,20 @@ services: - build/docker.env ports: - "6432:5432" - backend: - build: - context: ./ - dockerfile: Dockerfile - depends_on: - - db - - transformer - entrypoint: sh -c '/wait-for db:5432 -- ./rudder-server' - ports: - - "8080:8080" - env_file: - - build/docker.env - environment: - - JOBS_DB_HOST=db + # backend: + # build: + # context: ./ + # dockerfile: Dockerfile + # depends_on: + # - db + # - transformer + # entrypoint: sh -c '/wait-for db:5432 -- ./rudder-server' + # ports: + # - "8080:8080" + # env_file: + # - build/docker.env + # environment: + # - JOBS_DB_HOST=db # Uncomment the following lines to mount workspaceConfig file # volumes: # - :/etc/rudderstack/workspaceConfig.json @@ -28,6 +28,9 @@ services: image: "rudderstack/rudder-transformer:latest" ports: - "9090:9090" + environment: + - JOBS_DB_HOST=db + - CONFIG_BACKEND_URL=https://api.dev.rudderlabs.com minio: image: minio/minio profiles: diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index 058f559dbd..46a9987f6f 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -368,8 +368,8 @@ func (r *DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) InitialPU: report.InitialPU, }, ReportMetadata: types.ReportMetadata{ - ReportedAt: report.ReportedAt * 60 * 1000, // send reportedAt in milliseconds - Bucket: sampleEventBucket * 60 * 1000, + ReportedAt: report.ReportedAt * 60 * 1000, // send reportedAt in milliseconds + SampleEventBucket: sampleEventBucket * 60 * 1000, }, } values = append(values, metricsByGroup[identifier]) diff --git a/enterprise/reporting/reporting_test.go b/enterprise/reporting/reporting_test.go index b108ab1a1e..63464de5c6 100644 --- a/enterprise/reporting/reporting_test.go +++ b/enterprise/reporting/reporting_test.go @@ -187,8 +187,8 @@ func TestGetAggregatedReports(t *testing.T) { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - Bucket: bucket * 60 * 1000, + ReportedAt: 28017690 * 60 * 1000, + SampleEventBucket: bucket * 60 * 1000, }, StatusDetails: []*types.StatusDetail{ { @@ -217,8 +217,8 @@ func TestGetAggregatedReports(t *testing.T) { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - Bucket: bucket * 60 * 1000, + ReportedAt: 28017690 * 60 * 1000, + SampleEventBucket: bucket * 60 * 1000, }, StatusDetails: []*types.StatusDetail{ { @@ -247,8 +247,8 @@ func TestGetAggregatedReports(t *testing.T) { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - Bucket: bucket * 60 * 1000, + ReportedAt: 28017690 * 60 * 1000, + SampleEventBucket: bucket * 60 * 1000, }, StatusDetails: []*types.StatusDetail{ { @@ -288,8 +288,8 @@ func TestGetAggregatedReports(t *testing.T) { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - Bucket: bucket * 60 * 1000, + ReportedAt: 28017690 * 60 * 1000, + SampleEventBucket: bucket * 60 * 1000, }, StatusDetails: []*types.StatusDetail{ { @@ -327,8 +327,8 @@ func TestGetAggregatedReports(t *testing.T) { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - Bucket: bucket * 60 * 1000, + ReportedAt: 28017690 * 60 * 1000, + SampleEventBucket: bucket * 60 * 1000, }, StatusDetails: []*types.StatusDetail{ { @@ -396,8 +396,8 @@ func TestGetAggregatedReports(t *testing.T) { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - Bucket: bucket * 60 * 1000, + ReportedAt: 28017690 * 60 * 1000, + SampleEventBucket: bucket * 60 * 1000, }, StatusDetails: []*types.StatusDetail{ { @@ -435,8 +435,8 @@ func TestGetAggregatedReports(t *testing.T) { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - Bucket: bucket * 60 * 1000, + ReportedAt: 28017690 * 60 * 1000, + SampleEventBucket: bucket * 60 * 1000, }, StatusDetails: []*types.StatusDetail{ { @@ -465,8 +465,8 @@ func TestGetAggregatedReports(t *testing.T) { PU: "some-pu", }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 28017690 * 60 * 1000, - Bucket: bucket * 60 * 1000, + ReportedAt: 28017690 * 60 * 1000, + SampleEventBucket: bucket * 60 * 1000, }, StatusDetails: []*types.StatusDetail{ { diff --git a/scripts/generate-event b/scripts/generate-event index 777db3d42f..d760f1e558 100755 --- a/scripts/generate-event +++ b/scripts/generate-event @@ -1,16 +1,105 @@ #!/bin/bash -if [[ $# != 2 ]]; then - echo "Usage: ./generate-event " - echo "Example: ./generate-event 1S0ibSaDlSSGkaQuHLi9feJqIUBNAE https:///v1/batch" - exit +# Default values +endPoint="" +repeatCount=1 +repeatDelay=1 +eventsPerRun=1 +concurrency=1 + +# Parse key=value arguments +for arg in "$@"; do + case $arg in + endPoint=*) + endPoint="${arg#*=}" + ;; + repeatCount=*) + repeatCount="${arg#*=}" + ;; + repeatDelay=*) + repeatDelay="${arg#*=}" + ;; + eventsPerRun=*) + eventsPerRun="${arg#*=}" + ;; + concurrency=*) + concurrency="${arg#*=}" + ;; + *) + echo "Unknown argument: $arg" + echo "Usage: ./generate-event endPoint= repeatCount= eventsPerRun= repeatDelay= concurrency=" + exit 1 + ;; + esac +done + +# Validate required arguments +if [[ -z $endPoint ]]; then + echo "Error: endPoint is required" + echo "Usage: ./generate-event endPoint= repeatCount= eventsPerRun= repeatDelay= concurrency=" + exit 1 fi +# write_keys=("2ZoCQEI1Y9LNLe4QnEG0Vsmp9ld" "2jjvcYgnaxCj1FKgzgulcQk3S25") +write_keys=("2SEMHBD56KYXEunktzr05UCrML6" "2TkVKAcJrChq0cLj3jNh5CcUmZD" "2V78NnazcshJ0zUgoiZqrTjMYmZ" "2VGCj19GVl1bfhsbDo85WM1R9vG" "2VxIFVQoWZHe3jXiLQPZ2AOnWQz" "2WJ5XHakhGfErt43DBff4TVUmg3" "2WWk95uCTzlIVbQXHRqRezVTavX" "2ZoCQEI1Y9LNLe4QnEG0Vsmp9ld" "2alVVJNW06KUdzSWUaO9uXMkMvm" "2dJhnoarKUWd9VgwwJK6rCP3lv7" "2drI9z15G2cET90dRQ5wYCFBItd" "2hMKV5bl3VY9AuxOWcky3x1ErMv" "2hMKi07eig0P566vxyvbVBIRN4B" "2hMKivYpdowjEePK6umqwjZt759" "2jN54jqq8itACcShVY2IErrW6ES" "2jPRsPmdqNt0GgvLFRUsFTsRsyd" "2jjvcYgnaxCj1FKgzgulcQk3S25") + dir=$( - cd "$(dirname "${BASH_SOURCE[0]}")" || exit - pwd -P + cd "$(dirname "${BASH_SOURCE[0]}")" || exit + pwd -P ) +jsonFile="track.json" +tempDir="${dir}/temp" +# Check if the tempDir exists. If it does, skip the creation step. +if [ ! -d "$tempDir" ]; then + mkdir -p "$tempDir" # Create the directory if it doesn't exist +fi + +# Define the cleanup function +cleanup() { + echo "Cancellation requested. Cleaning up..." + cancelFlag=1 + # You can also perform any cleanup tasks here, such as removing temp files. +} + +# Trap SIGINT (Ctrl+C) signal and call cleanup +trap cleanup SIGINT + +# Main loop +for run in $(seq 1 "$repeatCount"); do + echo "" + echo "" + echo "Run #$run ----- $(date -u +"%Y-%m-%dT%H:%M:%SZ")" + eventSuffix="event_" + + for concurrencyIndex in $(seq 1 "$concurrency"); do + { + eventSuffix="event_${concurrencyIndex}_" + + for i in $(seq 1 "$eventsPerRun"); do + for j in "${!write_keys[@]}"; do + if [[ $cancelFlag -eq 1 ]]; then + echo "Process cancelled, exiting..." + exit 0 + fi + + current_time=$(date -u +"%Y-%m-%dT%H:%M:%S.%3NZ") + + jq --arg j "$j" --arg i "$i" --arg eventSuffix "$eventSuffix" --arg current_time "$current_time" \ + '.event = $eventSuffix + $j + "_" + $i + | .sentAt = $current_time + | .originalTimestamp = $current_time' \ + "${dir}/${jsonFile}" > "${tempDir}/${eventSuffix}_${jsonFile}" + + curl -u "${write_keys[$j]}": -X POST "$endPoint" -d @"${tempDir}/${eventSuffix}_${jsonFile}" --header "Content-Type: application/json" + done + done + } & # Run in the background + done + + # Wait for all background jobs to finish + wait -#curl -u $1: -X POST http://$2:8080/v1/track -d @$dir/track.json --header "Content-Type: application/json" -#curl -u $1: -X POST $2 -d @$dir/track.json --header "Content-Type: application/json" -curl -u "$1": -X POST "$2" -d @"${dir}"/batch.json --header "Content-Type: application/json" # give absolute path for batch.json to test on local machine + if [[ $run -lt $repeatCount ]]; then + sleep "$repeatDelay" + fi +done diff --git a/utils/types/reporting_types.go b/utils/types/reporting_types.go index 897e2519a0..f7334968bb 100644 --- a/utils/types/reporting_types.go +++ b/utils/types/reporting_types.go @@ -81,8 +81,8 @@ type InstanceDetails struct { } type ReportMetadata struct { - ReportedAt int64 `json:"reportedAt"` - Bucket int64 `json:"bucket"` + ReportedAt int64 `json:"reportedAt"` + SampleEventBucket int64 `json:"bucket"` } type Metric struct { From c3d0db97981af3135877ca25576c4f5964fa8c35 Mon Sep 17 00:00:00 2001 From: Mihir Khambhati Date: Fri, 6 Dec 2024 13:34:52 +0530 Subject: [PATCH 37/44] chore: remove bucketStart from getReports and delete query --- enterprise/reporting/reporting.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index f4d52e245c..cbe390c243 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -216,17 +216,17 @@ func (r *DefaultReporter) getReports(currentMs, aggregationIntervalMin int64, sy return nil, 0, nil } - bucketStart, bucketEnd := r.getAggregationBucketMinute(queryMin.Int64, aggregationIntervalMin) + _, bucketEnd := r.getAggregationBucketMinute(queryMin.Int64, aggregationIntervalMin) // we don't want to flush partial buckets, so we wait for the current bucket to be complete if bucketEnd > currentMs { return nil, 0, nil } groupByColumns := "workspace_id, namespace, instance_id, source_definition_id, source_category, source_id, destination_definition_id, destination_id, source_task_run_id, source_job_id, source_job_run_id, transformation_id, transformation_version_id, tracking_plan_id, tracking_plan_version, in_pu, pu, status, terminal_state, initial_state, status_code, event_name, event_type, error_type" - sqlStatement = fmt.Sprintf(`SELECT %s, MAX(reported_at), (ARRAY_AGG(sample_response order by id))[1], (ARRAY_AGG(sample_event order by id))[1], SUM(count), SUM(violation_count) FROM %s WHERE reported_at >= $1 and reported_at < $2 GROUP BY %s`, groupByColumns, ReportsTable, groupByColumns) + sqlStatement = fmt.Sprintf(`SELECT %s, MAX(reported_at), (ARRAY_AGG(sample_response order by id))[1], (ARRAY_AGG(sample_event order by id))[1], SUM(count), SUM(violation_count) FROM %s WHERE reported_at < $1 GROUP BY %s`, groupByColumns, ReportsTable, groupByColumns) var rows *sql.Rows queryStart = time.Now() - rows, err = dbHandle.Query(sqlStatement, bucketStart, bucketEnd) + rows, err = dbHandle.Query(sqlStatement, bucketEnd) if err != nil { panic(err) } @@ -494,8 +494,8 @@ func (r *DefaultReporter) mainLoop(ctx context.Context, c types.SyncerConfig) { return err } // Use the same aggregationIntervalMin value that was used to query the reports in getReports() - bucketStart, bucketEnd := r.getAggregationBucketMinute(reportedAt, aggregationIntervalMin) - _, err = dbHandle.Exec(`DELETE FROM `+ReportsTable+` WHERE reported_at >= $1 and reported_at < $2`, bucketStart, bucketEnd) + _, bucketEnd := r.getAggregationBucketMinute(reportedAt, aggregationIntervalMin) + _, err = dbHandle.Exec(`DELETE FROM `+ReportsTable+` WHERE reported_at < $1`, bucketEnd) if err != nil { r.log.Errorf(`[ Reporting ]: Error deleting local reports from %s: %v`, ReportsTable, err) } else { From e74a24176c2820780be6d2a0c17b48770be9e85f Mon Sep 17 00:00:00 2001 From: Mihir Khambhati Date: Fri, 6 Dec 2024 13:35:50 +0530 Subject: [PATCH 38/44] chore: add comment to explain the rounding of intervalMs to the nearest factor of 60 --- enterprise/reporting/reporting.go | 1 + 1 file changed, 1 insertion(+) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index cbe390c243..efcb6ed928 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -353,6 +353,7 @@ func (r *DefaultReporter) getAggregatedReports(reports []*types.ReportByStatus) func (*DefaultReporter) getAggregationBucketMinute(timeMs, intervalMs int64) (int64, int64) { // If interval is not a factor of 60, then the bucket start will not be aligned to hour start // For example, if intervalMs is 7, and timeMs is 28891085 (6:05) then the bucket start will be 28891079 (5:59) + // and current bucket will contain the data of 2 different hourly buckets, which is should not have happened. // To avoid this, we round the intervalMs to the nearest factor of 60. if intervalMs <= 0 || 60%intervalMs != 0 { factors := []int64{1, 2, 3, 4, 5, 6, 10, 12, 15, 20, 30, 60} From 8be59d6e0570e1f15af9b385f9cd72af4c3b0a77 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Fri, 6 Dec 2024 14:26:21 +0530 Subject: [PATCH 39/44] fix: tests --- utils/types/reporting_types_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/utils/types/reporting_types_test.go b/utils/types/reporting_types_test.go index a950d32e4e..01501902c5 100644 --- a/utils/types/reporting_types_test.go +++ b/utils/types/reporting_types_test.go @@ -32,6 +32,7 @@ func TestMetricJSONMarshaling(t *testing.T) { "reportedAt": 1730712600000, "trackingPlanId": "1", "trackingPlanVersion": 1, + "bucket": 1730712600000, "reports": [ { "state": "failed", @@ -86,7 +87,8 @@ func TestMetricJSONMarshaling(t *testing.T) { InitialPU: false, }, ReportMetadata: types.ReportMetadata{ - ReportedAt: 1730712600000, + ReportedAt: 1730712600000, + SampleEventBucket: 1730712600000, }, StatusDetails: []*types.StatusDetail{ { From 1507ca7136a39c0e727e52ab65b243bd0cfce527 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Fri, 6 Dec 2024 14:29:24 +0530 Subject: [PATCH 40/44] fix: revert --- docker-compose.yml | 31 ++++++------ scripts/generate-event | 107 ++++------------------------------------- 2 files changed, 23 insertions(+), 115 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index e99e97a35a..157caa9913 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,20 +7,20 @@ services: - build/docker.env ports: - "6432:5432" - # backend: - # build: - # context: ./ - # dockerfile: Dockerfile - # depends_on: - # - db - # - transformer - # entrypoint: sh -c '/wait-for db:5432 -- ./rudder-server' - # ports: - # - "8080:8080" - # env_file: - # - build/docker.env - # environment: - # - JOBS_DB_HOST=db + backend: + build: + context: ./ + dockerfile: Dockerfile + depends_on: + - db + - transformer + entrypoint: sh -c '/wait-for db:5432 -- ./rudder-server' + ports: + - "8080:8080" + env_file: + - build/docker.env + environment: + - JOBS_DB_HOST=db # Uncomment the following lines to mount workspaceConfig file # volumes: # - :/etc/rudderstack/workspaceConfig.json @@ -28,9 +28,6 @@ services: image: "rudderstack/rudder-transformer:latest" ports: - "9090:9090" - environment: - - JOBS_DB_HOST=db - - CONFIG_BACKEND_URL=https://api.dev.rudderlabs.com minio: image: minio/minio profiles: diff --git a/scripts/generate-event b/scripts/generate-event index d760f1e558..777db3d42f 100755 --- a/scripts/generate-event +++ b/scripts/generate-event @@ -1,105 +1,16 @@ #!/bin/bash -# Default values -endPoint="" -repeatCount=1 -repeatDelay=1 -eventsPerRun=1 -concurrency=1 - -# Parse key=value arguments -for arg in "$@"; do - case $arg in - endPoint=*) - endPoint="${arg#*=}" - ;; - repeatCount=*) - repeatCount="${arg#*=}" - ;; - repeatDelay=*) - repeatDelay="${arg#*=}" - ;; - eventsPerRun=*) - eventsPerRun="${arg#*=}" - ;; - concurrency=*) - concurrency="${arg#*=}" - ;; - *) - echo "Unknown argument: $arg" - echo "Usage: ./generate-event endPoint= repeatCount= eventsPerRun= repeatDelay= concurrency=" - exit 1 - ;; - esac -done - -# Validate required arguments -if [[ -z $endPoint ]]; then - echo "Error: endPoint is required" - echo "Usage: ./generate-event endPoint= repeatCount= eventsPerRun= repeatDelay= concurrency=" - exit 1 +if [[ $# != 2 ]]; then + echo "Usage: ./generate-event " + echo "Example: ./generate-event 1S0ibSaDlSSGkaQuHLi9feJqIUBNAE https:///v1/batch" + exit fi -# write_keys=("2ZoCQEI1Y9LNLe4QnEG0Vsmp9ld" "2jjvcYgnaxCj1FKgzgulcQk3S25") -write_keys=("2SEMHBD56KYXEunktzr05UCrML6" "2TkVKAcJrChq0cLj3jNh5CcUmZD" "2V78NnazcshJ0zUgoiZqrTjMYmZ" "2VGCj19GVl1bfhsbDo85WM1R9vG" "2VxIFVQoWZHe3jXiLQPZ2AOnWQz" "2WJ5XHakhGfErt43DBff4TVUmg3" "2WWk95uCTzlIVbQXHRqRezVTavX" "2ZoCQEI1Y9LNLe4QnEG0Vsmp9ld" "2alVVJNW06KUdzSWUaO9uXMkMvm" "2dJhnoarKUWd9VgwwJK6rCP3lv7" "2drI9z15G2cET90dRQ5wYCFBItd" "2hMKV5bl3VY9AuxOWcky3x1ErMv" "2hMKi07eig0P566vxyvbVBIRN4B" "2hMKivYpdowjEePK6umqwjZt759" "2jN54jqq8itACcShVY2IErrW6ES" "2jPRsPmdqNt0GgvLFRUsFTsRsyd" "2jjvcYgnaxCj1FKgzgulcQk3S25") - dir=$( - cd "$(dirname "${BASH_SOURCE[0]}")" || exit - pwd -P + cd "$(dirname "${BASH_SOURCE[0]}")" || exit + pwd -P ) -jsonFile="track.json" -tempDir="${dir}/temp" -# Check if the tempDir exists. If it does, skip the creation step. -if [ ! -d "$tempDir" ]; then - mkdir -p "$tempDir" # Create the directory if it doesn't exist -fi - -# Define the cleanup function -cleanup() { - echo "Cancellation requested. Cleaning up..." - cancelFlag=1 - # You can also perform any cleanup tasks here, such as removing temp files. -} - -# Trap SIGINT (Ctrl+C) signal and call cleanup -trap cleanup SIGINT - -# Main loop -for run in $(seq 1 "$repeatCount"); do - echo "" - echo "" - echo "Run #$run ----- $(date -u +"%Y-%m-%dT%H:%M:%SZ")" - eventSuffix="event_" - - for concurrencyIndex in $(seq 1 "$concurrency"); do - { - eventSuffix="event_${concurrencyIndex}_" - - for i in $(seq 1 "$eventsPerRun"); do - for j in "${!write_keys[@]}"; do - if [[ $cancelFlag -eq 1 ]]; then - echo "Process cancelled, exiting..." - exit 0 - fi - - current_time=$(date -u +"%Y-%m-%dT%H:%M:%S.%3NZ") - - jq --arg j "$j" --arg i "$i" --arg eventSuffix "$eventSuffix" --arg current_time "$current_time" \ - '.event = $eventSuffix + $j + "_" + $i - | .sentAt = $current_time - | .originalTimestamp = $current_time' \ - "${dir}/${jsonFile}" > "${tempDir}/${eventSuffix}_${jsonFile}" - - curl -u "${write_keys[$j]}": -X POST "$endPoint" -d @"${tempDir}/${eventSuffix}_${jsonFile}" --header "Content-Type: application/json" - done - done - } & # Run in the background - done - - # Wait for all background jobs to finish - wait - if [[ $run -lt $repeatCount ]]; then - sleep "$repeatDelay" - fi -done +#curl -u $1: -X POST http://$2:8080/v1/track -d @$dir/track.json --header "Content-Type: application/json" +#curl -u $1: -X POST $2 -d @$dir/track.json --header "Content-Type: application/json" +curl -u "$1": -X POST "$2" -d @"${dir}"/batch.json --header "Content-Type: application/json" # give absolute path for batch.json to test on local machine From 11ee19908cee495595079799d3d21441ef2fdf42 Mon Sep 17 00:00:00 2001 From: Mihir Khambhati Date: Fri, 6 Dec 2024 14:34:41 +0530 Subject: [PATCH 41/44] Revert "chore: remove bucketStart from getReports and delete query" This reverts commit c3d0db97981af3135877ca25576c4f5964fa8c35. --- enterprise/reporting/reporting.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index efcb6ed928..2c1f9bfb1d 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -216,17 +216,17 @@ func (r *DefaultReporter) getReports(currentMs, aggregationIntervalMin int64, sy return nil, 0, nil } - _, bucketEnd := r.getAggregationBucketMinute(queryMin.Int64, aggregationIntervalMin) + bucketStart, bucketEnd := r.getAggregationBucketMinute(queryMin.Int64, aggregationIntervalMin) // we don't want to flush partial buckets, so we wait for the current bucket to be complete if bucketEnd > currentMs { return nil, 0, nil } groupByColumns := "workspace_id, namespace, instance_id, source_definition_id, source_category, source_id, destination_definition_id, destination_id, source_task_run_id, source_job_id, source_job_run_id, transformation_id, transformation_version_id, tracking_plan_id, tracking_plan_version, in_pu, pu, status, terminal_state, initial_state, status_code, event_name, event_type, error_type" - sqlStatement = fmt.Sprintf(`SELECT %s, MAX(reported_at), (ARRAY_AGG(sample_response order by id))[1], (ARRAY_AGG(sample_event order by id))[1], SUM(count), SUM(violation_count) FROM %s WHERE reported_at < $1 GROUP BY %s`, groupByColumns, ReportsTable, groupByColumns) + sqlStatement = fmt.Sprintf(`SELECT %s, MAX(reported_at), (ARRAY_AGG(sample_response order by id))[1], (ARRAY_AGG(sample_event order by id))[1], SUM(count), SUM(violation_count) FROM %s WHERE reported_at >= $1 and reported_at < $2 GROUP BY %s`, groupByColumns, ReportsTable, groupByColumns) var rows *sql.Rows queryStart = time.Now() - rows, err = dbHandle.Query(sqlStatement, bucketEnd) + rows, err = dbHandle.Query(sqlStatement, bucketStart, bucketEnd) if err != nil { panic(err) } @@ -495,8 +495,8 @@ func (r *DefaultReporter) mainLoop(ctx context.Context, c types.SyncerConfig) { return err } // Use the same aggregationIntervalMin value that was used to query the reports in getReports() - _, bucketEnd := r.getAggregationBucketMinute(reportedAt, aggregationIntervalMin) - _, err = dbHandle.Exec(`DELETE FROM `+ReportsTable+` WHERE reported_at < $1`, bucketEnd) + bucketStart, bucketEnd := r.getAggregationBucketMinute(reportedAt, aggregationIntervalMin) + _, err = dbHandle.Exec(`DELETE FROM `+ReportsTable+` WHERE reported_at >= $1 and reported_at < $2`, bucketStart, bucketEnd) if err != nil { r.log.Errorf(`[ Reporting ]: Error deleting local reports from %s: %v`, ReportsTable, err) } else { From 842848409f83b3f3d0162aca3a908e50c6644192 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Fri, 6 Dec 2024 16:22:07 +0530 Subject: [PATCH 42/44] chore: addressed review comments --- .../event_sampler/badger_event_sampler.go | 7 ++++--- .../reporting/event_sampler/event_sampler.go | 15 +++++++-------- .../in_memory_cache_event_sampler.go | 15 ++++++++------- enterprise/reporting/reporting.go | 4 ++-- 4 files changed, 21 insertions(+), 20 deletions(-) diff --git a/enterprise/reporting/event_sampler/badger_event_sampler.go b/enterprise/reporting/event_sampler/badger_event_sampler.go index fa5696b4a1..89c2a23e52 100644 --- a/enterprise/reporting/event_sampler/badger_event_sampler.go +++ b/enterprise/reporting/event_sampler/badger_event_sampler.go @@ -12,6 +12,7 @@ import ( "github.com/rudderlabs/rudder-go-kit/config" "github.com/rudderlabs/rudder-go-kit/logger" + "github.com/rudderlabs/rudder-server/rruntime" "github.com/rudderlabs/rudder-server/utils/misc" ) @@ -31,7 +32,7 @@ func DefaultPath(pathName string) (string, error) { return fmt.Sprintf(`%v%v`, tmpDirPath, pathName), nil } -func NewBadgerEventSampler(pathName string, ttl config.ValueLoader[time.Duration], conf *config.Config, log logger.Logger) (*BadgerEventSampler, error) { +func NewBadgerEventSampler(ctx context.Context, pathName string, ttl config.ValueLoader[time.Duration], conf *config.Config, log logger.Logger) (*BadgerEventSampler, error) { dbPath, err := DefaultPath(pathName) if err != nil || dbPath == "" { return nil, err @@ -51,7 +52,7 @@ func NewBadgerEventSampler(pathName string, ttl config.ValueLoader[time.Duration WithSyncWrites(conf.GetBool("Reporting.eventSampling.badgerDB.syncWrites", false)). WithDetectConflicts(conf.GetBool("Reporting.eventSampling.badgerDB.detectConflicts", false)) - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(ctx) db, err := badger.Open(opts) @@ -66,7 +67,7 @@ func NewBadgerEventSampler(pathName string, ttl config.ValueLoader[time.Duration return nil, err } - go es.gcLoop() + rruntime.Go(es.gcLoop) return es, nil } diff --git a/enterprise/reporting/event_sampler/event_sampler.go b/enterprise/reporting/event_sampler/event_sampler.go index a91d49c685..8e3da3fd0e 100644 --- a/enterprise/reporting/event_sampler/event_sampler.go +++ b/enterprise/reporting/event_sampler/event_sampler.go @@ -1,7 +1,7 @@ package event_sampler import ( - "fmt" + "context" "time" "github.com/rudderlabs/rudder-go-kit/config" @@ -21,22 +21,21 @@ type EventSampler interface { } func NewEventSampler( + ctx context.Context, ttl config.ValueLoader[time.Duration], eventSamplerType config.ValueLoader[string], eventSamplingCardinality config.ValueLoader[int], conf *config.Config, log logger.Logger, -) (EventSampler, error) { - var es EventSampler - var err error - +) (es EventSampler, err error) { switch eventSamplerType.Load() { case BadgerTypeEventSampler: - es, err = NewBadgerEventSampler(BadgerEventSamplerPathName, ttl, conf, log) + es, err = NewBadgerEventSampler(ctx, BadgerEventSamplerPathName, ttl, conf, log) case InMemoryCacheTypeEventSampler: - es, err = NewInMemoryCacheEventSampler(ttl, eventSamplingCardinality) + es, err = NewInMemoryCacheEventSampler(ctx, ttl, eventSamplingCardinality) default: - err = fmt.Errorf("invalid event sampler type: %s", eventSamplerType.Load()) + log.Warnf("invalid event sampler type: %s. Using default badger event sampler", eventSamplerType.Load()) + es, err = NewBadgerEventSampler(ctx, BadgerEventSamplerPathName, ttl, conf, log) } if err != nil { diff --git a/enterprise/reporting/event_sampler/in_memory_cache_event_sampler.go b/enterprise/reporting/event_sampler/in_memory_cache_event_sampler.go index 9267b5455c..a8e413fc8e 100644 --- a/enterprise/reporting/event_sampler/in_memory_cache_event_sampler.go +++ b/enterprise/reporting/event_sampler/in_memory_cache_event_sampler.go @@ -1,7 +1,7 @@ package event_sampler import ( - "sync" + "context" "time" "github.com/rudderlabs/rudder-go-kit/cachettl" @@ -9,17 +9,21 @@ import ( ) type InMemoryCacheEventSampler struct { + ctx context.Context + cancel context.CancelFunc cache *cachettl.Cache[string, bool] - mu sync.Mutex ttl config.ValueLoader[time.Duration] limit config.ValueLoader[int] length int } -func NewInMemoryCacheEventSampler(ttl config.ValueLoader[time.Duration], limit config.ValueLoader[int]) (*InMemoryCacheEventSampler, error) { +func NewInMemoryCacheEventSampler(ctx context.Context, ttl config.ValueLoader[time.Duration], limit config.ValueLoader[int]) (*InMemoryCacheEventSampler, error) { c := cachettl.New[string, bool](cachettl.WithNoRefreshTTL) + ctx, cancel := context.WithCancel(ctx) es := &InMemoryCacheEventSampler{ + ctx: ctx, + cancel: cancel, cache: c, ttl: ttl, limit: limit, @@ -34,15 +38,11 @@ func NewInMemoryCacheEventSampler(ttl config.ValueLoader[time.Duration], limit c } func (es *InMemoryCacheEventSampler) Get(key string) (bool, error) { - es.mu.Lock() - defer es.mu.Unlock() value := es.cache.Get(key) return value, nil } func (es *InMemoryCacheEventSampler) Put(key string) error { - es.mu.Lock() - defer es.mu.Unlock() if es.length >= es.limit.Load() { return nil } @@ -53,4 +53,5 @@ func (es *InMemoryCacheEventSampler) Put(key string) error { } func (es *InMemoryCacheEventSampler) Close() { + es.cancel() } diff --git a/enterprise/reporting/reporting.go b/enterprise/reporting/reporting.go index f073b318d8..b8f5cf8d15 100644 --- a/enterprise/reporting/reporting.go +++ b/enterprise/reporting/reporting.go @@ -99,7 +99,7 @@ func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Log eventSamplingEnabled := conf.GetReloadableBoolVar(false, "Reporting.eventSampling.enabled") eventSamplingDuration := conf.GetReloadableDurationVar(60, time.Minute, "Reporting.eventSampling.durationInMinutes") eventSamplerType := conf.GetReloadableStringVar("badger", "Reporting.eventSampling.type") - eventSamplingCardinality := conf.GetReloadableIntVar(1000000, 1, "Reporting.eventSampling.cardinality") + eventSamplingCardinality := conf.GetReloadableIntVar(100000, 1, "Reporting.eventSampling.cardinality") // only send reports for wh actions sources if whActionsOnly is configured whActionsOnly := config.GetBool("REPORTING_WH_ACTIONS_ONLY", false) if whActionsOnly { @@ -108,7 +108,7 @@ func NewDefaultReporter(ctx context.Context, conf *config.Config, log logger.Log if eventSamplingEnabled.Load() { var err error - eventSampler, err = event_sampler.NewEventSampler(eventSamplingDuration, eventSamplerType, eventSamplingCardinality, conf, log) + eventSampler, err = event_sampler.NewEventSampler(ctx, eventSamplingDuration, eventSamplerType, eventSamplingCardinality, conf, log) if err != nil { panic(err) } From 8e51ecc6d3cc67aa5fcf04dfc42bc23509f63487 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Fri, 6 Dec 2024 16:25:05 +0530 Subject: [PATCH 43/44] fix: tests --- .../reporting/event_sampler/event_sampler_test.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/enterprise/reporting/event_sampler/event_sampler_test.go b/enterprise/reporting/event_sampler/event_sampler_test.go index e4b4d646bd..4d47f24123 100644 --- a/enterprise/reporting/event_sampler/event_sampler_test.go +++ b/enterprise/reporting/event_sampler/event_sampler_test.go @@ -1,6 +1,7 @@ package event_sampler import ( + "context" "testing" "time" @@ -13,6 +14,7 @@ import ( ) func TestBadger(t *testing.T) { + ctx := context.Background() conf := config.New() ttl := conf.GetReloadableDurationVar(3000, time.Millisecond, "Reporting.eventSampling.durationInMinutes") eventSamplerType := conf.GetReloadableStringVar("badger", "Reporting.eventSampling.type") @@ -21,7 +23,7 @@ func TestBadger(t *testing.T) { t.Run("should put and get keys", func(t *testing.T) { assert.Equal(t, 3000*time.Millisecond, ttl.Load()) - es, _ := NewEventSampler(ttl, eventSamplerType, eventSamplingCardinality, conf, log) + es, _ := NewEventSampler(ctx, ttl, eventSamplerType, eventSamplingCardinality, conf, log) _ = es.Put("key1") _ = es.Put("key2") _ = es.Put("key3") @@ -41,7 +43,7 @@ func TestBadger(t *testing.T) { conf.Set("Reporting.eventSampling.durationInMinutes", 100) assert.Equal(t, 100*time.Millisecond, ttl.Load()) - es, _ := NewEventSampler(ttl, eventSamplerType, eventSamplingCardinality, conf, log) + es, _ := NewEventSampler(ctx, ttl, eventSamplerType, eventSamplingCardinality, conf, log) defer es.Close() _ = es.Put("key1") @@ -54,6 +56,7 @@ func TestBadger(t *testing.T) { } func TestInMemoryCache(t *testing.T) { + ctx := context.Background() conf := config.New() eventSamplerType := conf.GetReloadableStringVar("in_memory_cache", "Reporting.eventSampling.type") eventSamplingCardinality := conf.GetReloadableIntVar(3, 1, "Reporting.eventSampling.cardinality") @@ -62,7 +65,7 @@ func TestInMemoryCache(t *testing.T) { t.Run("should put and get keys", func(t *testing.T) { assert.Equal(t, 3000*time.Millisecond, ttl.Load()) - es, _ := NewEventSampler(ttl, eventSamplerType, eventSamplingCardinality, conf, log) + es, _ := NewEventSampler(ctx, ttl, eventSamplerType, eventSamplingCardinality, conf, log) _ = es.Put("key1") _ = es.Put("key2") _ = es.Put("key3") @@ -80,7 +83,7 @@ func TestInMemoryCache(t *testing.T) { t.Run("should not get evicted keys", func(t *testing.T) { conf.Set("Reporting.eventSampling.durationInMinutes", 100) assert.Equal(t, 100*time.Millisecond, ttl.Load()) - es, _ := NewEventSampler(ttl, eventSamplerType, eventSamplingCardinality, conf, log) + es, _ := NewEventSampler(ctx, ttl, eventSamplerType, eventSamplingCardinality, conf, log) _ = es.Put("key1") require.Eventually(t, func() bool { @@ -92,7 +95,7 @@ func TestInMemoryCache(t *testing.T) { t.Run("should not add keys if length exceeds", func(t *testing.T) { conf.Set("Reporting.eventSampling.durationInMinutes", 3000) assert.Equal(t, 3000*time.Millisecond, ttl.Load()) - es, _ := NewEventSampler(ttl, eventSamplerType, eventSamplingCardinality, conf, log) + es, _ := NewEventSampler(ctx, ttl, eventSamplerType, eventSamplingCardinality, conf, log) _ = es.Put("key1") _ = es.Put("key2") _ = es.Put("key3") @@ -128,6 +131,7 @@ func BenchmarkEventSampler(b *testing.B) { }, } + ctx := context.Background() conf := config.New() ttl := conf.GetReloadableDurationVar(1, time.Minute, "Reporting.eventSampling.durationInMinutes") eventSamplerType := conf.GetReloadableStringVar("default", "Reporting.eventSampling.type") @@ -139,6 +143,7 @@ func BenchmarkEventSampler(b *testing.B) { conf.Set("Reporting.eventSampling.type", tc.eventSamplerType) eventSampler, err := NewEventSampler( + ctx, ttl, eventSamplerType, eventSamplingCardinality, From fa4f07444f8800e9486e97321d6c61211917d3b0 Mon Sep 17 00:00:00 2001 From: vamsikrishnakandi Date: Fri, 6 Dec 2024 16:50:21 +0530 Subject: [PATCH 44/44] chore: addressed review comments --- .../event_sampler/badger_event_sampler.go | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/enterprise/reporting/event_sampler/badger_event_sampler.go b/enterprise/reporting/event_sampler/badger_event_sampler.go index 89c2a23e52..ac0e1ba79b 100644 --- a/enterprise/reporting/event_sampler/badger_event_sampler.go +++ b/enterprise/reporting/event_sampler/badger_event_sampler.go @@ -22,6 +22,7 @@ type BadgerEventSampler struct { ttl config.ValueLoader[time.Duration] ctx context.Context cancel context.CancelFunc + wg sync.WaitGroup } func DefaultPath(pathName string) (string, error) { @@ -61,13 +62,18 @@ func NewBadgerEventSampler(ctx context.Context, pathName string, ttl config.Valu ttl: ttl, ctx: ctx, cancel: cancel, + wg: sync.WaitGroup{}, } if err != nil { return nil, err } - rruntime.Go(es.gcLoop) + es.wg.Add(1) + rruntime.Go(func() { + defer es.wg.Done() + es.gcLoop() + }) return es, nil } @@ -107,35 +113,31 @@ func (es *BadgerEventSampler) Put(key string) error { }) } -func (es *BadgerEventSampler) performGC() { - // One call would only result in removal of at max one log file. - // As an optimization, we can call it in a loop until it returns an error. - for { - select { - case <-es.ctx.Done(): - return - default: - if err := es.db.RunValueLogGC(0.5); err != nil { - return - } - } - } -} - func (es *BadgerEventSampler) gcLoop() { for { select { case <-es.ctx.Done(): + _ = es.db.RunValueLogGC(0.5) return - case <-time.After(5 * time.Minute): - es.performGC() + } + again: + if es.ctx.Err() != nil { + return + } + // One call would only result in removal of at max one log file. + // As an optimization, you could also immediately re-run it whenever it returns nil error + // (this is why `goto again` is used). + err := es.db.RunValueLogGC(0.5) + if err == nil { + goto again } } } func (es *BadgerEventSampler) Close() { es.cancel() + es.wg.Wait() if es.db != nil { _ = es.db.Close() }