Skip to content
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

logger: make consistent flexible levels and contextual logging #1268

Open
wants to merge 7 commits into
base: incubation
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 16 additions & 29 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -222,26 +222,16 @@ This will ensure that the doc for the apis are updated accordingly.

### Enabled logging

#### Controller level
Global logger configuration can be changed with a command line switch `--log-mode <mode>`
for example from CSV. Valid values for `<mode>`: "" (as default) || prod || production || devel || development.

Logger on all controllers can only be changed from CSV with parameters: --log-mode devel
valid value: "" (as default) || prod || production || devel || development
Verbosity level is INFO.
To fine tune zap backend [standard operator sdk zap switches](https://sdk.operatorframework.io/docs/building-operators/golang/references/logging/)
can be used.

This mainly impacts logging for operator pod startup, generating common resource, monitoring deployment.

| --log-mode value | mapping Log level | Comments |
| ---------------- | ------------------- | -------------- |
| devel | debug / 0 | lowest level |
| "" | info / 1 | default option |
| default | info / 1 | default option |
| prod | error / 2 | highest level |

#### Component level

Logger on components can be changed by DSCI devFlags during runtime.
By default, if not set .spec.devFlags.logmode, it uses INFO level
Modification applies to all components, not only these "Managed" ones.
Update DSCI CR with .spec.devFlags.logmode, see example :
Log level can be changed by DSCI devFlags during runtime by setting
.spec.devFlags.logLevel. It accepts the same values as `--zap-log-level`
command line switch. See example :

```console
apiVersion: dscinitialization.opendatahub.io/v1
Expand All @@ -250,20 +240,17 @@ metadata:
name: default-dsci
spec:
devFlags:
logmode: development
logLevel: debug
...
```

Avaiable value for logmode is "devel", "development", "prod", "production".
The first two work the same set to DEBUG level; the later two work the same as using ERROR level.

| .spec.devFlags.logmode | stacktrace level | verbosity | Output | Comments |
| ---------------------- | ---------------- | --------- | -------- | -------------- |
| devel | WARN | INFO | Console | lowest level, using epoch time |
| development | WARN | INFO | Console | same as devel |
| "" | ERROR | INFO | JSON | default option |
| prod | ERROR | INFO | JSON | highest level, using human readable timestamp |
| production | ERROR | INFO | JSON | same as prod |
| logmode | stacktrace level | verbosity | Output | Comments |
|-------------|------------------|-----------|---------|-----------------------------------------------|
| devel | WARN | INFO | Console | lowest level, using epoch time |
| development | WARN | INFO | Console | same as devel |
| "" | ERROR | INFO | JSON | default option |
| prod | ERROR | INFO | JSON | highest level, using human readable timestamp |
| production | ERROR | INFO | JSON | same as prod |

### Example DSCInitialization

Expand Down
3 changes: 3 additions & 0 deletions apis/dscinitialization/v1/dscinitialization_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ type DevFlags struct {
// +kubebuilder:validation:Enum=devel;development;prod;production;default
// +kubebuilder:default="production"
LogMode string `json:"logmode,omitempty"`
// Override Zap log level. Can be "debug", "info", "error" or a number (more verbose).
// +optional
LogLevel string `json:"logLevel,omitempty"`
}

type TrustedCABundleSpec struct {
Expand Down
3 changes: 1 addition & 2 deletions components/codeflare/codeflare.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"fmt"
"path/filepath"

"github.com/go-logr/logr"
operatorv1 "github.com/openshift/api/operator/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -74,11 +73,11 @@ func (c *CodeFlare) GetComponentName() string {

func (c *CodeFlare) ReconcileComponent(ctx context.Context,
cli client.Client,
l logr.Logger,
owner metav1.Object,
dscispec *dsciv1.DSCInitializationSpec,
platform cluster.Platform,
_ bool) error {
l := logf.FromContext(ctx)
enabled := c.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

Expand Down
2 changes: 1 addition & 1 deletion components/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ type ManifestsConfig struct {

type ComponentInterface interface {
Init(ctx context.Context, platform cluster.Platform) error
ReconcileComponent(ctx context.Context, cli client.Client, logger logr.Logger,
ReconcileComponent(ctx context.Context, cli client.Client,
owner metav1.Object, DSCISpec *dsciv1.DSCInitializationSpec, platform cluster.Platform, currentComponentStatus bool) error
Cleanup(ctx context.Context, cli client.Client, owner metav1.Object, DSCISpec *dsciv1.DSCInitializationSpec) error
GetComponentName() string
Expand Down
2 changes: 1 addition & 1 deletion components/dashboard/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@ func (d *Dashboard) GetComponentName() string {

func (d *Dashboard) ReconcileComponent(ctx context.Context,
cli client.Client,
l logr.Logger,
owner metav1.Object,
dscispec *dsciv1.DSCInitializationSpec,
platform cluster.Platform,
currentComponentExist bool,
) error {
entryPath := DefaultPath
l := logf.FromContext(ctx)
enabled := d.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

Expand Down
3 changes: 1 addition & 2 deletions components/datasciencepipelines/datasciencepipelines.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"fmt"
"path/filepath"

"github.com/go-logr/logr"
operatorv1 "github.com/openshift/api/operator/v1"
conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -95,12 +94,12 @@ func (d *DataSciencePipelines) GetComponentName() string {

func (d *DataSciencePipelines) ReconcileComponent(ctx context.Context,
cli client.Client,
l logr.Logger,
owner metav1.Object,
dscispec *dsciv1.DSCInitializationSpec,
platform cluster.Platform,
_ bool,
) error {
l := logf.FromContext(ctx)
enabled := d.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

Expand Down
4 changes: 2 additions & 2 deletions components/kserve/kserve.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"path/filepath"
"strings"

"github.com/go-logr/logr"
operatorv1 "github.com/openshift/api/operator/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -112,7 +111,8 @@ func (k *Kserve) GetComponentName() string {
}

func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client,
l logr.Logger, owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
l := logf.FromContext(ctx)
enabled := k.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

Expand Down
5 changes: 3 additions & 2 deletions components/kserve/servicemesh_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (

operatorv1 "github.com/openshift/api/operator/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"

dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
Expand Down Expand Up @@ -37,6 +37,7 @@ func (k *Kserve) removeServiceMeshConfigurations(ctx context.Context, cli client
}

func (k *Kserve) defineServiceMeshFeatures(ctx context.Context, cli client.Client, dscispec *dsciv1.DSCInitializationSpec) feature.FeaturesProvider {
log := logf.FromContext(ctx)
return func(registry feature.FeaturesRegistry) error {
authorinoInstalled, err := cluster.SubscriptionExists(ctx, cli, "authorino-operator")
if err != nil {
Expand Down Expand Up @@ -68,7 +69,7 @@ func (k *Kserve) defineServiceMeshFeatures(ctx context.Context, cli client.Clien
return kserveExtAuthzErr
}
} else {
ctrl.Log.Info("WARN: Authorino operator is not installed on the cluster, skipping authorization capability")
log.Info("WARN: Authorino operator is not installed on the cluster, skipping authorization capability")
}

return nil
Expand Down
4 changes: 2 additions & 2 deletions components/kueue/kueue.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"fmt"
"path/filepath"

"github.com/go-logr/logr"
operatorv1 "github.com/openshift/api/operator/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -68,8 +67,9 @@ func (k *Kueue) GetComponentName() string {
return ComponentName
}

func (k *Kueue) ReconcileComponent(ctx context.Context, cli client.Client, l logr.Logger,
func (k *Kueue) ReconcileComponent(ctx context.Context, cli client.Client,
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
l := logf.FromContext(ctx)
enabled := k.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed
if enabled {
Expand Down
3 changes: 1 addition & 2 deletions components/modelmeshserving/modelmeshserving.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"path/filepath"
"strings"

"github.com/go-logr/logr"
operatorv1 "github.com/openshift/api/operator/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -103,12 +102,12 @@ func (m *ModelMeshServing) GetComponentName() string {

func (m *ModelMeshServing) ReconcileComponent(ctx context.Context,
cli client.Client,
l logr.Logger,
owner metav1.Object,
dscispec *dsciv1.DSCInitializationSpec,
platform cluster.Platform,
_ bool,
) error {
l := logf.FromContext(ctx)
enabled := m.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

Expand Down
4 changes: 2 additions & 2 deletions components/modelregistry/modelregistry.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"strings"
"text/template"

"github.com/go-logr/logr"
operatorv1 "github.com/openshift/api/operator/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -97,8 +96,9 @@ func (m *ModelRegistry) GetComponentName() string {
return ComponentName
}

func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Client, l logr.Logger,
func (m *ModelRegistry) ReconcileComponent(ctx context.Context, cli client.Client,
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
l := logf.FromContext(ctx)
enabled := m.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

Expand Down
4 changes: 2 additions & 2 deletions components/ray/ray.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"fmt"
"path/filepath"

"github.com/go-logr/logr"
operatorv1 "github.com/openshift/api/operator/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -69,8 +68,9 @@ func (r *Ray) GetComponentName() string {
return ComponentName
}

func (r *Ray) ReconcileComponent(ctx context.Context, cli client.Client, l logr.Logger,
func (r *Ray) ReconcileComponent(ctx context.Context, cli client.Client,
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
l := logf.FromContext(ctx)
enabled := r.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

Expand Down
4 changes: 2 additions & 2 deletions components/trainingoperator/trainingoperator.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"fmt"
"path/filepath"

"github.com/go-logr/logr"
operatorv1 "github.com/openshift/api/operator/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -70,8 +69,9 @@ func (r *TrainingOperator) GetComponentName() string {
return ComponentName
}

func (r *TrainingOperator) ReconcileComponent(ctx context.Context, cli client.Client, l logr.Logger,
func (r *TrainingOperator) ReconcileComponent(ctx context.Context, cli client.Client,
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
l := logf.FromContext(ctx)
enabled := r.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed

Expand Down
4 changes: 2 additions & 2 deletions components/trustyai/trustyai.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"fmt"
"path/filepath"

"github.com/go-logr/logr"
operatorv1 "github.com/openshift/api/operator/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -79,8 +78,9 @@ func (t *TrustyAI) GetComponentName() string {
return ComponentName
}

func (t *TrustyAI) ReconcileComponent(ctx context.Context, cli client.Client, l logr.Logger,
func (t *TrustyAI) ReconcileComponent(ctx context.Context, cli client.Client,
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
l := logf.FromContext(ctx)
enabled := t.GetManagementState() == operatorv1.Managed
monitoringEnabled := dscispec.Monitoring.ManagementState == operatorv1.Managed
entryPath := DefaultPath
Expand Down
4 changes: 2 additions & 2 deletions components/workbenches/workbenches.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"path/filepath"
"strings"

"github.com/go-logr/logr"
operatorv1 "github.com/openshift/api/operator/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -111,8 +110,9 @@ func (w *Workbenches) GetComponentName() string {
return ComponentName
}

func (w *Workbenches) ReconcileComponent(ctx context.Context, cli client.Client, l logr.Logger,
func (w *Workbenches) ReconcileComponent(ctx context.Context, cli client.Client,
owner metav1.Object, dscispec *dsciv1.DSCInitializationSpec, platform cluster.Platform, _ bool) error {
l := logf.FromContext(ctx)
// Set default notebooks namespace
// Create rhods-notebooks namespace in managed platforms
enabled := w.GetManagementState() == operatorv1.Managed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ spec:
Internal development useful field to test customizations.
This is not recommended to be used in production environment.
properties:
logLevel:
description: Override Zap log level. Can be "debug", "info", "error"
or a number (more verbose).
type: string
logmode:
default: production
enum:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"context"
"reflect"

"github.com/go-logr/logr"
operatorv1 "github.com/openshift/api/operator/v1"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
Expand All @@ -16,6 +15,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

Expand All @@ -28,13 +28,11 @@ import (
type CertConfigmapGeneratorReconciler struct {
Client client.Client
Scheme *runtime.Scheme
Log logr.Logger
}

// SetupWithManager sets up the controller with the Manager.
func (r *CertConfigmapGeneratorReconciler) SetupWithManager(mgr ctrl.Manager) error {
log := r.Log
log.Info("Adding controller for Configmap Generation.")
func (r *CertConfigmapGeneratorReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
logf.FromContext(ctx).Info("Adding controller for Configmap Generation.")
return ctrl.NewControllerManagedBy(mgr).
Named("cert-configmap-generator-controller").
Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(r.watchTrustedCABundleConfigMapResource), builder.WithPredicates(ConfigMapChangedPredicate)).
Expand All @@ -45,7 +43,7 @@ func (r *CertConfigmapGeneratorReconciler) SetupWithManager(mgr ctrl.Manager) er
// Reconcile will generate new configmap, odh-trusted-ca-bundle, that includes cluster-wide trusted-ca bundle and custom
// ca bundle in every new namespace created.
func (r *CertConfigmapGeneratorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
log := r.Log
log := logf.FromContext(ctx)
// Request includes namespace that is newly created or where odh-trusted-ca-bundle configmap is updated.
log.Info("Reconciling CertConfigMapGenerator.", " Request.Namespace", req.NamespacedName)
// Get namespace instance
Expand Down Expand Up @@ -109,8 +107,8 @@ func (r *CertConfigmapGeneratorReconciler) watchNamespaceResource(_ context.Cont
return nil
}

func (r *CertConfigmapGeneratorReconciler) watchTrustedCABundleConfigMapResource(_ context.Context, a client.Object) []reconcile.Request {
log := r.Log
func (r *CertConfigmapGeneratorReconciler) watchTrustedCABundleConfigMapResource(ctx context.Context, a client.Object) []reconcile.Request {
log := logf.FromContext(ctx)
if a.GetName() == trustedcabundle.CAConfigMapName {
log.Info("Cert configmap has been updated, start reconcile")
return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: a.GetName(), Namespace: a.GetNamespace()}}}
Expand Down
Loading