Skip to content

Commit

Permalink
chore: Refactor operator logs (apache#4586)
Browse files Browse the repository at this point in the history
- Reduce amount of informational log events by using Debug level on events like "Reconciling ...", "Invoking action ..."
- Use Debug level for each "Reconciling ..." log event
- Use new "Initializing ..." Info level log event when a resource is initializing (used in metrics tests to verify metrics)
- Log error details on state transition to Phase "Error" for resources Integration, Build, KameletBinding and Pipe
  • Loading branch information
christophd committed Jul 20, 2023
1 parent 2d0935d commit 42791ff
Show file tree
Hide file tree
Showing 20 changed files with 83 additions and 27 deletions.
8 changes: 4 additions & 4 deletions e2e/commonwithcustominstall/operator_metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func TestMetrics(t *testing.T) {
WithNewTestNamespace(t, func(ns string) {
name := "java"
operatorID := "camel-k-metrics"
Expect(KamelInstallWithID(operatorID, ns).Execute()).To(Succeed())
Expect(KamelInstallWithID(operatorID, ns, "--log-level", "debug").Execute()).To(Succeed())
Expect(KamelRunWithID(operatorID, ns, "files/Java.java",
"-t", "prometheus.enabled=true",
"-t", "prometheus.pod-monitor=false",
Expand Down Expand Up @@ -104,7 +104,7 @@ func TestMetrics(t *testing.T) {
err = NewLogWalker(&logs).
AddStep(MatchFields(IgnoreExtras, Fields{
"LoggerName": Equal("camel-k.controller.build"),
"Message": Equal("state transition"),
"Message": Equal("State transition"),
"PhaseFrom": Equal(string(v1.BuildPhaseScheduling)),
"PhaseTo": Equal(string(v1.BuildPhasePending)),
"RequestName": Equal(build.Name),
Expand All @@ -115,7 +115,7 @@ func TestMetrics(t *testing.T) {
}), LogEntryNoop).
AddStep(MatchFields(IgnoreExtras, Fields{
"LoggerName": Equal("camel-k.controller.build"),
"Message": Equal("state transition"),
"Message": Equal("State transition"),
"PhaseFrom": Equal(string(v1.BuildPhaseRunning)),
"PhaseTo": Equal(string(v1.BuildPhaseSucceeded)),
"RequestName": Equal(build.Name),
Expand Down Expand Up @@ -422,7 +422,7 @@ func TestMetrics(t *testing.T) {
err = NewLogWalker(&logs).
AddStep(MatchFields(IgnoreExtras, Fields{
"LoggerName": Equal("camel-k.controller.build"),
"Message": Equal("state transition"),
"Message": Equal("State transition"),
"PhaseFrom": Equal(string(v1.BuildPhaseScheduling)),
"PhaseTo": Equal(string(v1.BuildPhasePending)),
"RequestName": Equal(build.Name),
Expand Down
19 changes: 16 additions & 3 deletions pkg/controller/build/build_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ type reconcileBuild struct {
// Result.Requeue is true, otherwise upon completion it will remove the work from the queue.
func (r *reconcileBuild) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) {
rlog := Log.WithValues("request-namespace", request.Namespace, "request-name", request.Name)
rlog.Info("Reconciling Build")
rlog.Debug("Reconciling Build")

// Make sure the operator is allowed to act on namespace
if ok, err := platform.IsOperatorAllowedOnNamespace(ctx, r.client, request.Namespace); err != nil {
Expand Down Expand Up @@ -176,7 +176,7 @@ func (r *reconcileBuild) Reconcile(ctx context.Context, request reconcile.Reques
a.InjectRecorder(r.recorder)

if a.CanHandle(target) {
targetLog.Infof("Invoking action %s", a.Name())
targetLog.Debugf("Invoking action %s", a.Name())

newTarget, err := a.Handle(ctx, target)
if err != nil {
Expand All @@ -192,10 +192,23 @@ func (r *reconcileBuild) Reconcile(ctx context.Context, request reconcile.Reques

if newTarget.Status.Phase != instance.Status.Phase {
targetLog.Info(
"state transition",
"State transition",
"phase-from", instance.Status.Phase,
"phase-to", newTarget.Status.Phase,
)

if newTarget.Status.Phase == v1.BuildPhaseError || newTarget.Status.Phase == v1.BuildPhaseFailed {
reason := string(newTarget.Status.Phase)

if newTarget.Status.Failure != nil {
reason = newTarget.Status.Failure.Reason
}

targetLog.Info(
"Build error",
"reason", reason,
"error-message", newTarget.Status.Error)
}
}

target = newTarget
Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/build/initialize_pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ func (action *initializePodAction) CanHandle(build *v1.Build) bool {

// Handle handles the builds.
func (action *initializePodAction) Handle(ctx context.Context, build *v1.Build) (*v1.Build, error) {
action.L.Info("Initializing Build")

if err := deleteBuilderPod(ctx, action.client, build); err != nil {
return nil, fmt.Errorf("cannot delete build pod: %w", err)
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/build/initialize_routine.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ func (action *initializeRoutineAction) CanHandle(build *v1.Build) bool {

// Handle handles the builds.
func (action *initializeRoutineAction) Handle(ctx context.Context, build *v1.Build) (*v1.Build, error) {
action.L.Info("Initializing Build")

build.Status.Phase = v1.BuildPhaseScheduling

return build, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/build/monitor_routine.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func (action *monitorRoutineAction) updateBuildStatus(ctx context.Context, build
return err
}
if target.Status.Phase != build.Status.Phase {
action.L.Info("state transition", "phase-from", build.Status.Phase, "phase-to", target.Status.Phase)
action.L.Info("State transition", "phase-from", build.Status.Phase, "phase-to", target.Status.Phase)
}
event.NotifyBuildUpdated(ctx, action.client, action.recorder, build, target)
build.Status = target.Status
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/build/schedule.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func (action *scheduleAction) patchBuildStatus(ctx context.Context, build *v1.Bu
}

if target.Status.Phase != build.Status.Phase {
action.L.Info("state transition", "phase-from", build.Status.Phase, "phase-to", target.Status.Phase)
action.L.Info("State transition", "phase-from", build.Status.Phase, "phase-to", target.Status.Phase)
}
event.NotifyBuildUpdated(ctx, action.client, action.recorder, build, target)

Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/catalog/catalog_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ type reconcileCatalog struct {
// Result.Requeue is true, otherwise upon completion it will remove the work from the queue.
func (r *reconcileCatalog) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) {
rlog := Log.WithValues("request-namespace", request.Namespace, "request-name", request.Name)
rlog.Info("Reconciling CamelCatalog")
rlog.Debug("Reconciling CamelCatalog")

// Make sure the operator is allowed to act on namespace
if ok, err := platform.IsOperatorAllowedOnNamespace(ctx, r.client, request.Namespace); err != nil {
Expand Down Expand Up @@ -185,7 +185,7 @@ func (r *reconcileCatalog) Reconcile(ctx context.Context, request reconcile.Requ

if targetPhase != phaseFrom {
targetLog.Info(
"state transition",
"State transition",
"phase-from", phaseFrom,
"phase-to", target.Status.Phase,
)
Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/catalog/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ func (action *initializeAction) CanHandle(catalog *v1.CamelCatalog) bool {
}

func (action *initializeAction) Handle(ctx context.Context, catalog *v1.CamelCatalog) (*v1.CamelCatalog, error) {
action.L.Info("Initializing CamelCatalog")

platform, err := platformutil.GetOrFindLocal(ctx, action.client, catalog.Namespace)

if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/integration/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ func (action *initializeAction) CanHandle(integration *v1.Integration) bool {

// Handle handles the integrations.
func (action *initializeAction) Handle(ctx context.Context, integration *v1.Integration) (*v1.Integration, error) {
action.L.Info("Initializing Integration")

if _, err := trait.Apply(ctx, action.client, integration, nil); err != nil {
integration.Status.Phase = v1.IntegrationPhaseError
integration.SetReadyCondition(corev1.ConditionFalse,
Expand Down
15 changes: 12 additions & 3 deletions pkg/controller/integration/integration_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ type reconcileIntegration struct {
// Result.Requeue is true, otherwise upon completion it will remove the work from the queue.
func (r *reconcileIntegration) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) {
rlog := Log.WithValues("request-namespace", request.Namespace, "request-name", request.Name)
rlog.Info("Reconciling Integration")
rlog.Debug("Reconciling Integration")

// Make sure the operator is allowed to act on namespace
if ok, err := platform.IsOperatorAllowedOnNamespace(ctx, r.client, request.Namespace); err != nil {
Expand Down Expand Up @@ -354,7 +354,7 @@ func (r *reconcileIntegration) Reconcile(ctx context.Context, request reconcile.
a.InjectLogger(targetLog)

if a.CanHandle(target) {
targetLog.Infof("Invoking action %s", a.Name())
targetLog.Debugf("Invoking action %s", a.Name())

newTarget, err := a.Handle(ctx, target)
if err != nil {
Expand Down Expand Up @@ -398,10 +398,19 @@ func (r *reconcileIntegration) update(ctx context.Context, base *v1.Integration,

if target.Status.Phase != base.Status.Phase {
log.Info(
"state transition",
"State transition",
"phase-from", base.Status.Phase,
"phase-to", target.Status.Phase,
)

if target.Status.Phase == v1.IntegrationPhaseError {
if cond := target.Status.GetCondition(v1.IntegrationConditionReady); cond != nil && cond.Status == corev1.ConditionFalse {
log.Info(
"Integration error",
"reason", cond.GetReason(),
"error-message", cond.GetMessage())
}
}
}

return nil
Expand Down
1 change: 1 addition & 0 deletions pkg/controller/integrationkit/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ func (action *initializeAction) Handle(ctx context.Context, kit *v1.IntegrationK
return nil, err
}

action.L.Info("Initializing IntegrationKit")
kit.Status.Version = defaults.Version

if kit.Spec.Image == "" {
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/integrationkit/integrationkit_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ type reconcileIntegrationKit struct {
// Result.Requeue is true, otherwise upon completion it will remove the work from the queue.
func (r *reconcileIntegrationKit) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) {
rlog := Log.WithValues("request-namespace", request.Namespace, "request-name", request.Name)
rlog.Info("Reconciling IntegrationKit")
rlog.Debug("Reconciling IntegrationKit")

// Make sure the operator is allowed to act on namespace
if ok, err := platform.IsOperatorAllowedOnNamespace(ctx, r.client, request.Namespace); err != nil {
Expand Down Expand Up @@ -291,7 +291,7 @@ func (r *reconcileIntegrationKit) Reconcile(ctx context.Context, request reconci

if targetPhase != instance.Status.Phase {
targetLog.Info(
"state transition",
"State transition",
"phase-from", instance.Status.Phase,
"phase-to", targetPhase,
)
Expand Down
1 change: 1 addition & 0 deletions pkg/controller/integrationplatform/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func (action *initializeAction) Handle(ctx context.Context, platform *v1.Integra
return nil, nil
}

action.L.Info("Initializing IntegrationPlatform")
if err = platformutil.ConfigureDefaults(ctx, action.client, platform, true); err != nil {
return nil, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ type reconcileIntegrationPlatform struct {
// Result.Requeue is true, otherwise upon completion it will remove the work from the queue.
func (r *reconcileIntegrationPlatform) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) {
rlog := Log.WithValues("request-namespace", request.Namespace, "request-name", request.Name)
rlog.Info("Reconciling IntegrationPlatform")
rlog.Debug("Reconciling IntegrationPlatform")

// Make sure the operator is allowed to act on namespace
if ok, err := platform.IsOperatorAllowedOnNamespace(ctx, r.client, request.Namespace); err != nil {
Expand Down Expand Up @@ -193,7 +193,7 @@ func (r *reconcileIntegrationPlatform) Reconcile(ctx context.Context, request re

if targetPhase != phaseFrom {
targetLog.Info(
"state transition",
"State transition",
"phase-from", phaseFrom,
"phase-to", target.Status.Phase,
)
Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/kamelet/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,7 @@ func (action *initializeAction) CanHandle(kamelet *v1.Kamelet) bool {
}

func (action *initializeAction) Handle(ctx context.Context, kamelet *v1.Kamelet) (*v1.Kamelet, error) {
action.L.Info("Initializing Kamelet")

return kameletutils.Initialize(kamelet)
}
6 changes: 3 additions & 3 deletions pkg/controller/kamelet/kamelet_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ type reconcileKamelet struct {
// Result.Requeue is true, otherwise upon completion it will remove the work from the queue.
func (r *reconcileKamelet) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) {
rlog := Log.WithValues("request-namespace", request.Namespace, "request-name", request.Name)
rlog.Info("Reconciling Kamelet")
rlog.Debug("Reconciling Kamelet")

// Make sure the operator is allowed to act on namespace
if ok, err := platform.IsOperatorAllowedOnNamespace(ctx, r.client, request.Namespace); err != nil {
Expand Down Expand Up @@ -164,7 +164,7 @@ func (r *reconcileKamelet) Reconcile(ctx context.Context, request reconcile.Requ
continue
}

targetLog.Infof("Invoking action %s", a.Name())
targetLog.Debugf("Invoking action %s", a.Name())

phaseFrom := target.Status.Phase

Expand All @@ -186,7 +186,7 @@ func (r *reconcileKamelet) Reconcile(ctx context.Context, request reconcile.Requ

if targetPhase != phaseFrom {
targetLog.Info(
"state transition",
"State transition",
"phase-from", phaseFrom,
"phase-to", target.Status.Phase,
)
Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/kameletbinding/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ func (action *initializeAction) CanHandle(binding *v1alpha1.KameletBinding) bool
}

func (action *initializeAction) Handle(ctx context.Context, binding *v1alpha1.KameletBinding) (*v1alpha1.KameletBinding, error) {
action.L.Info("Initializing KameletBinding")

it, err := CreateIntegrationFor(ctx, action.client, binding)
if err != nil {
binding.Status.Phase = v1alpha1.KameletBindingPhaseError
Expand Down
15 changes: 12 additions & 3 deletions pkg/controller/kameletbinding/kameletbinding_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ type ReconcileKameletBinding struct {
// Result.Requeue is true, otherwise upon completion it will remove the work from the queue.
func (r *ReconcileKameletBinding) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) {
rlog := Log.WithValues("request-namespace", request.Namespace, "request-name", request.Name)
rlog.Info("Reconciling KameletBinding")
rlog.Debug("Reconciling KameletBinding")

// Make sure the operator is allowed to act on namespace
if ok, err := platform.IsOperatorAllowedOnNamespace(ctx, r.client, request.Namespace); err != nil {
Expand Down Expand Up @@ -190,7 +190,7 @@ func (r *ReconcileKameletBinding) Reconcile(ctx context.Context, request reconci
a.InjectLogger(targetLog)

if a.CanHandle(target) {
targetLog.Infof("Invoking action %s", a.Name())
targetLog.Debugf("Invoking action %s", a.Name())

target, err = a.Handle(ctx, target)
if err != nil {
Expand Down Expand Up @@ -229,10 +229,19 @@ func (r *ReconcileKameletBinding) update(ctx context.Context, base *v1alpha1.Kam

if target.Status.Phase != base.Status.Phase {
log.Info(
"state transition",
"State transition",
"phase-from", base.Status.Phase,
"phase-to", target.Status.Phase,
)

if target.Status.Phase == v1alpha1.KameletBindingPhaseError {
if cond := target.Status.GetCondition(v1alpha1.KameletBindingIntegrationConditionError); cond != nil {
log.Info(
"Integration error",
"reason", cond.GetReason(),
"error-message", cond.GetMessage())
}
}
}

return nil
Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/pipe/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ func (action *initializeAction) CanHandle(binding *v1.Pipe) bool {
}

func (action *initializeAction) Handle(ctx context.Context, binding *v1.Pipe) (*v1.Pipe, error) {
action.L.Info("Initializing Pipe")

it, err := CreateIntegrationFor(ctx, action.client, binding)
if err != nil {
binding.Status.Phase = v1.PipePhaseError
Expand Down
15 changes: 12 additions & 3 deletions pkg/controller/pipe/pipe_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ type ReconcilePipe struct {
// Result.Requeue is true, otherwise upon completion it will remove the work from the queue.
func (r *ReconcilePipe) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) {
rlog := Log.WithValues("request-namespace", request.Namespace, "request-name", request.Name)
rlog.Info("Reconciling Pipe")
rlog.Debug("Reconciling Pipe")

// Make sure the operator is allowed to act on namespace
if ok, err := platform.IsOperatorAllowedOnNamespace(ctx, r.client, request.Namespace); err != nil {
Expand Down Expand Up @@ -190,7 +190,7 @@ func (r *ReconcilePipe) Reconcile(ctx context.Context, request reconcile.Request
a.InjectLogger(targetLog)

if a.CanHandle(target) {
targetLog.Infof("Invoking action %s", a.Name())
targetLog.Debugf("Invoking action %s", a.Name())

target, err = a.Handle(ctx, target)
if err != nil {
Expand Down Expand Up @@ -229,10 +229,19 @@ func (r *ReconcilePipe) update(ctx context.Context, base *v1.Pipe, target *v1.Pi

if target.Status.Phase != base.Status.Phase {
log.Info(
"state transition",
"State transition",
"phase-from", base.Status.Phase,
"phase-to", target.Status.Phase,
)

if target.Status.Phase == v1.PipePhaseError {
if cond := target.Status.GetCondition(v1.PipeIntegrationConditionError); cond != nil {
log.Info(
"Integration error",
"reason", cond.GetReason(),
"error-message", cond.GetMessage())
}
}
}

return nil
Expand Down

0 comments on commit 42791ff

Please sign in to comment.