ROX-15242 - Reduce fleetshard and fleet-manager log noise #785

SimonBaeumer · 2023-02-01T09:43:39Z

Description

Clean up logs. Added logger to only log if the given integer value had changed.

Checklist (Definition of Done)

Unit and integration tests added
Added test description under Test manual
Documentation added if necessary (i.e. changes to dev setup, test execution, ...)
CI and all relevant tests are passing
Add the ticket number to the PR title if available, i.e. ROX-12345: ...
Discussed security and business related topics privately. Will move any security and business related topics that arise to private communication channel.

Test manual

TODO: Add manual testing efforts

# To run tests locally run:
make db/teardown db/setup db/migrate
make ocm/setup OCM_OFFLINE_TOKEN=<ocm-offline-token> OCM_ENV=development
make verify lint binary test test/integration

openshift-ci · 2023-02-01T09:43:44Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

fleetshard/pkg/runtime/runtime.go

vladbologa · 2023-02-22T13:38:51Z

fleetshard/pkg/central/reconciler/reconciler.go

@@ -104,6 +104,8 @@ func (r *CentralReconciler) Reconcile(ctx context.Context, remoteCentral private
 		return nil, errors.Wrapf(err, "checking if central changed")
 	}

+	glog.Infof("Start reconcile central %s/%s", remoteCentral.Metadata.Namespace, remoteCentral.Metadata.Name)
+


I think logging here will still result in many log messages, should be after line 113 to have effect.

And one more thing here, is it ok to log remoteCentral.Metadata.Name? It's customer data, and I remember that when I wanted to tag the DB instances with the name, we decided not to do it.

Not sure about if we should remove the name, but adding the ID would be good since this is most important to filter for logs of a specific tenant.

The namespace already contains the Id, but it would be better to replace Namespace with Id.

Convinced. I'll create a follow-up PR to this to keep the changes small.

Structured logging would be great now :)

Follow up PR: #856

I think logging here will still result in many log messages, should be after line 113 to have effect.

Looks like my initial comment was forgotten because of the subsequent namespace / ID discussion.

I'd still move this log to after line 113, so that we avoid logging "Starting reconcile..." for every tenant, every 5-10 seconds. WDYT?

vladbologa · 2023-02-22T13:39:24Z

fleetshard/pkg/runtime/runtime.go

@@ -4,6 +4,7 @@ package runtime
 import (
 	"context"
 	"fmt"
+	"github.com/stackrox/acs-fleet-manager/pkg/logger"


please move to the section below

+1, usually the formatter should fix that. Don't know why it doesn't.

vladbologa · 2023-02-22T16:26:16Z

fleetshard/pkg/runtime/runtime.go

@@ -119,7 +122,8 @@ func (r *Runtime) Start() error {
 		}

 		// Start for each Central its own reconciler which can be triggered by sending a central to the receive channel.
-		glog.Infof("Received %d centrals", len(list.Items))
+		reconciledCentralCountCache = int32(len(list.Items))
+		logger.InfoChangedInt32(&reconciledCentralCountCache, "Received central count changed: received %d centrals")


This doesn't seem to work as expected, here are some logs from my local machine:

I0222 16:18:02.130817 1 runtime.go:126] Received central count changed: received %d centrals0 I0222 16:19:32.236458 1 runtime.go:126] Received central count changed: received %d centrals0 I0222 16:19:32.236553 1 reconciler.go:107] Start reconcile central rhacs-cfr40h6og40g01q0bb50/test-central-1 I0222 16:19:32.277767 1 reconciler.go:285] Creating central rhacs-cfr40h6og40g01q0bb50/test-central-1 I0222 16:19:32.298116 1 reconciler.go:289] Central rhacs-cfr40h6og40g01q0bb50/test-central-1 created I0222 16:19:42.247972 1 runtime.go:126] Received central count changed: received %d centrals0 I0222 16:19:42.248036 1 reconciler.go:107] Start reconcile central rhacs-cfr40h6og40g01q0bb50/test-central-1 I0222 16:19:42.267968 1 reconciler.go:291] Update central rhacs-cfr40h6og40g01q0bb50/test-central-1 I0222 16:19:52.259769 1 runtime.go:126] Received central count changed: received %d centrals0 I0222 16:19:52.259844 1 reconciler.go:107] Start reconcile central rhacs-cfr40h6og40g01q0bb50/test-central-1 I0222 16:19:52.285547 1 reconciler.go:291] Update central rhacs-cfr40h6og40g01q0bb50/test-central-1 I0222 16:20:02.266983 1 runtime.go:126] Received central count changed: received %d centrals0 I0222 16:20:02.267147 1 reconciler.go:107] Start reconcile central rhacs-cfr40h6og40g01q0bb50/test-central-1 I0222 16:20:02.296243 1 reconciler.go:291] Update central rhacs-cfr40h6og40g01q0bb50/test-central-1 I0222 16:20:12.277558 1 runtime.go:126] Received central count changed: received %d centrals0 I0222 16:20:12.277622 1 reconciler.go:107] Start reconcile central rhacs-cfr40h6og40g01q0bb50/test-central-1 I0222 16:20:12.294873 1 reconciler.go:291] Update central rhacs-cfr40h6og40g01q0bb50/test-central-1 I0222 16:20:22.290589 1 runtime.go:126] Received central count changed: received %d centrals0

vladbologa

Removing my request changes to not block this PR while on PTO.

Removing my review to not block this PR while on PTO.

SimonBaeumer · 2023-02-27T15:25:54Z

/retest

johannes94

LGTM

openshift-ci · 2023-03-01T14:57:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johannes94, SimonBaeumer

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [SimonBaeumer,johannes94]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2023-03-01T15:10:57Z

New changes are detected. LGTM label has been removed.

SimonBaeumer · 2023-03-29T12:45:10Z

/retest

b4a141d Add log caches to reduce log noise 35ce2dd WIP 65aedae WIP 1a853d1 reduce logs 15e8fa3 WIP fd32e77 update cluster logs 4dd5724 WIP d3f3a68 WIP ab6c110 WIP 6084ed8 fix log message 8eed701 stylez

openshift-ci bot added the do-not-merge/work-in-progress label Feb 1, 2023

SimonBaeumer temporarily deployed to development February 1, 2023 09:43 — with GitHub Actions Inactive

openshift-ci bot added the approved label Feb 1, 2023

SimonBaeumer temporarily deployed to development February 1, 2023 09:44 — with GitHub Actions Inactive

SimonBaeumer force-pushed the sb/reduce-fleetshard-logs branch from 14be5d2 to 22920c5 Compare February 1, 2023 14:12

SimonBaeumer temporarily deployed to development February 1, 2023 14:12 — with GitHub Actions Inactive

SimonBaeumer temporarily deployed to development February 2, 2023 08:35 — with GitHub Actions Inactive

SimonBaeumer temporarily deployed to development February 2, 2023 09:30 — with GitHub Actions Inactive

vladbologa reviewed Feb 15, 2023

View reviewed changes

fleetshard/pkg/runtime/runtime.go Outdated Show resolved Hide resolved

SimonBaeumer force-pushed the sb/reduce-fleetshard-logs branch from 083b0cb to 15e8fa3 Compare February 22, 2023 13:17

SimonBaeumer temporarily deployed to development February 22, 2023 13:17 — with GitHub Actions Inactive

SimonBaeumer temporarily deployed to development February 22, 2023 13:20 — with GitHub Actions Inactive

SimonBaeumer temporarily deployed to development February 22, 2023 13:21 — with GitHub Actions Inactive

SimonBaeumer temporarily deployed to development February 22, 2023 13:23 — with GitHub Actions Inactive

SimonBaeumer temporarily deployed to development February 22, 2023 13:27 — with GitHub Actions Inactive

SimonBaeumer requested a review from johannes94 February 22, 2023 13:29

SimonBaeumer marked this pull request as ready for review February 22, 2023 13:29

openshift-ci bot removed the do-not-merge/work-in-progress label Feb 22, 2023

SimonBaeumer temporarily deployed to development February 22, 2023 13:29 — with GitHub Actions Inactive

vladbologa previously requested changes Feb 22, 2023

View reviewed changes

openshift-ci bot assigned vladbologa Feb 22, 2023

vladbologa reviewed Feb 22, 2023

View reviewed changes

vladbologa reviewed Feb 23, 2023

View reviewed changes

vladbologa removed their assignment Feb 23, 2023

SimonBaeumer temporarily deployed to development February 27, 2023 14:58 — with GitHub Actions Inactive

SimonBaeumer changed the title ~~Reduce fleetshard and fleet-manager log noise~~ ROX-15242 - Reduce fleetshard and fleet-manager log noise Feb 27, 2023

SimonBaeumer temporarily deployed to development February 27, 2023 15:35 — with GitHub Actions Inactive

johannes94 approved these changes Mar 1, 2023

View reviewed changes

openshift-ci bot assigned johannes94 Mar 1, 2023

openshift-ci bot added the lgtm label Mar 1, 2023

SimonBaeumer enabled auto-merge (squash) March 1, 2023 15:09

SimonBaeumer disabled auto-merge March 1, 2023 15:09

SimonBaeumer force-pushed the sb/reduce-fleetshard-logs branch from 8eed701 to d9860c0 Compare March 1, 2023 15:10

openshift-ci bot removed the lgtm label Mar 1, 2023

SimonBaeumer temporarily deployed to development March 1, 2023 15:10 — with GitHub Actions Inactive

SimonBaeumer force-pushed the sb/reduce-fleetshard-logs branch from d9860c0 to e9fdfb2 Compare March 29, 2023 08:34

SimonBaeumer temporarily deployed to development March 29, 2023 08:34 — with GitHub Actions Inactive

openshift-merge-robot added the needs-rebase label Apr 21, 2023

SimonBaeumer added 2 commits April 24, 2023 10:39

X-Smart-Branch-Parent: main

d4e7ca6

X-Smart-Squash: Squashed 11 commits:

2b18159

b4a141d Add log caches to reduce log noise 35ce2dd WIP 65aedae WIP 1a853d1 reduce logs 15e8fa3 WIP fd32e77 update cluster logs 4dd5724 WIP d3f3a68 WIP ab6c110 WIP 6084ed8 fix log message 8eed701 stylez

SimonBaeumer force-pushed the sb/reduce-fleetshard-logs branch from e9fdfb2 to 2b18159 Compare April 24, 2023 08:40

SimonBaeumer temporarily deployed to development April 24, 2023 08:40 — with GitHub Actions Inactive

openshift-merge-robot removed the needs-rebase label Apr 24, 2023

SimonBaeumer merged commit 0b6676d into main Apr 25, 2023

SimonBaeumer deleted the sb/reduce-fleetshard-logs branch April 25, 2023 11:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ROX-15242 - Reduce fleetshard and fleet-manager log noise #785

ROX-15242 - Reduce fleetshard and fleet-manager log noise #785

SimonBaeumer commented Feb 1, 2023 •

edited

Loading

openshift-ci bot commented Feb 1, 2023

vladbologa Feb 22, 2023

vladbologa Feb 22, 2023

johannes94 Feb 22, 2023

vladbologa Feb 22, 2023

SimonBaeumer Feb 27, 2023

SimonBaeumer Feb 27, 2023

SimonBaeumer Feb 27, 2023

vladbologa Mar 9, 2023 •

edited

Loading

vladbologa Feb 22, 2023

johannes94 Feb 22, 2023

vladbologa Feb 22, 2023

SimonBaeumer Feb 27, 2023

vladbologa left a comment •

edited

Loading

SimonBaeumer commented Feb 27, 2023

johannes94 left a comment

openshift-ci bot commented Mar 1, 2023

openshift-ci bot commented Mar 1, 2023

SimonBaeumer commented Mar 29, 2023

ROX-15242 - Reduce fleetshard and fleet-manager log noise #785

ROX-15242 - Reduce fleetshard and fleet-manager log noise #785

Conversation

SimonBaeumer commented Feb 1, 2023 • edited Loading

Description

Checklist (Definition of Done)

Test manual

openshift-ci bot commented Feb 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vladbologa Mar 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vladbologa left a comment • edited Loading

Choose a reason for hiding this comment

SimonBaeumer commented Feb 27, 2023

johannes94 left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Mar 1, 2023

openshift-ci bot commented Mar 1, 2023

SimonBaeumer commented Mar 29, 2023

SimonBaeumer commented Feb 1, 2023 •

edited

Loading

vladbologa Mar 9, 2023 •

edited

Loading

vladbologa left a comment •

edited

Loading