Add prewebhook and cronitor telemetry integration #1099
Conversation
dkron/grpc.go
Outdated
| @@ -237,7 +237,7 @@ func (grpcs *GRPCServer) ExecutionDone(ctx context.Context, execDoneReq *proto.E | |||
| } | |||
|
|
|||
| // Send notification | |||
| if err := Notification(grpcs.agent.config, execution, exg, job).Send(grpcs.logger); err != nil { | |||
| if err := NewNotifier(grpcs.agent.config, execution, exg, job, grpcs.logger).End(); err != nil { | |||
There was a problem hiding this comment.
.Start() and .End() initially confused me; I thought End() was supposed to be called on the same notification object as Start(). But it isn't, they're not related.
I suggest renaming these to something like SendPreNotifications and SendPostNotifications.
Or maybe, instead of one constructor-method, have two methods taking appropriate arguments (saves the nil arg to be passed when sending the prehooks) that initialize and immediately send:
if err:= SendPostNotifications(grpcs.agent.config, execution, exg, job, grpcs.logger); err != nil { etc
There was a problem hiding this comment.
Yeah, makes much more sense, I tried to maintain the legacy interface but wasn't a good idea, refactored to these recommendations.
Prewebhook configuration will behave the same as Webhook but will be run before running the job.
We're integrating with Cronitor service for advanced monitoring.