-
Notifications
You must be signed in to change notification settings - Fork 336
Improve insight collector #1340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 14 commits
902e5f1
2d01d13
16ac4bf
709e19a
8abc72a
7404079
261b654
0e99357
26322b5
0ba2717
8bca2b1
06c1d3f
83958c0
b35358d
2b494d0
f8a5fa8
0c76835
3fedbfd
0c89dc2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ import ( | |
| "github.com/pipe-cd/pipe/pkg/admin" | ||
| "github.com/pipe-cd/pipe/pkg/app/ops/handler" | ||
| "github.com/pipe-cd/pipe/pkg/app/ops/insightcollector" | ||
| "github.com/pipe-cd/pipe/pkg/backoff" | ||
| "github.com/pipe-cd/pipe/pkg/cli" | ||
| "github.com/pipe-cd/pipe/pkg/datastore" | ||
| "github.com/pipe-cd/pipe/pkg/version" | ||
|
|
@@ -102,18 +103,28 @@ func (s *ops) run(ctx context.Context, t cli.Telemetry) error { | |
| collector := insightcollector.NewInsightCollector(ds, fs, t.Logger) | ||
| c := cron.New(cron.WithLocation(time.UTC)) | ||
| _, err := c.AddFunc(cfg.InsightCollector.Schedule, func() { | ||
| start := time.Now() | ||
| if err := collector.CollectProjectsInsight(ctx); err != nil { | ||
| t.Logger.Error("failed to run the project insight collector", zap.Error(err)) | ||
| } else { | ||
| t.Logger.Info("project insight collector successfully finished", zap.Duration("duration", time.Since(start))) | ||
| } | ||
|
|
||
| start = time.Now() | ||
| if err := collector.CollectApplicationInsight(ctx); err != nil { | ||
| t.Logger.Error("failed to run the application insight collector", zap.Error(err)) | ||
| } else { | ||
| t.Logger.Info("application insight collector successfully finished", zap.Duration("duration", time.Since(start))) | ||
| retry := backoff.NewRetry(cfg.InsightCollector.RetryTime, backoff.NewConstant(time.Duration(cfg.InsightCollector.RetryIntervalHour)*time.Hour)) | ||
| for retry.WaitNext(ctx) { | ||
| var failed bool | ||
| start := time.Now() | ||
| if err = collector.ProcessNewlyCompletedDeployments(ctx); err != nil { | ||
| t.Logger.Error("failed to process the insight collector with completedAt", zap.Error(err)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: |
||
| failed = true | ||
| } else { | ||
| t.Logger.Info("processing the insight collector with completedAt successfully finished", zap.Duration("duration", time.Since(start))) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: |
||
| } | ||
|
|
||
| start = time.Now() | ||
| if err = collector.ProcessNewlyCreatedDeployments(ctx); err != nil { | ||
| t.Logger.Error("failed to process the insight collector with createdAt", zap.Error(err)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: |
||
| failed = true | ||
| } else { | ||
| t.Logger.Info("processing the insight collector with createdAt successfully finished", zap.Duration("duration", time.Since(start))) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: |
||
| } | ||
|
|
||
| if !failed { | ||
| return | ||
| } | ||
| } | ||
| }) | ||
| if err != nil { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you skip calling this function when it was completed successfully while
ProcessNewlyCreatedDeploymentswas failed? I know that the insight data will not be affected but we can save some unnecessary DB calls by skip calling this one.