Skip to content

Commit

Permalink
[improvement] improve span propagation to ensure Reconcile is always …
Browse files Browse the repository at this point in the history
…a top level span parent (#387)

* fix: improve span propagation

Signed-off-by: Mateusz Urbanek <[email protected]>

* feat: add extra info to span via default decorators

Signed-off-by: Mateusz Urbanek <[email protected]>

* test: add tests for helpers

Signed-off-by: Mateusz Urbanek <[email protected]>

* chore: add to placementgroups

Signed-off-by: Mateusz Urbanek <[email protected]>

---------

Signed-off-by: Mateusz Urbanek <[email protected]>
  • Loading branch information
shanduur authored Jul 29, 2024
1 parent 4e620d5 commit e83dc0c
Show file tree
Hide file tree
Showing 21 changed files with 956 additions and 348 deletions.
1 change: 1 addition & 0 deletions api/v1alpha1/webhook_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ var (
// defaultLinodeClient is an unauthenticated Linode client
defaultLinodeClient = linodeclient.NewLinodeClientWithTracing(
ptr.To(linodego.NewClient(&http.Client{Timeout: defaultClientTimeout})),
linodeclient.DefaultDecorator(),
)
)

Expand Down
1 change: 1 addition & 0 deletions api/v1alpha2/webhook_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ var (
// defaultLinodeClient is an unauthenticated Linode client
defaultLinodeClient = linodeclient.NewLinodeClientWithTracing(
ptr.To(linodego.NewClient(&http.Client{Timeout: defaultClientTimeout})),
linodeclient.DefaultDecorator(),
)
)

Expand Down
1 change: 1 addition & 0 deletions cloud/scope/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ func CreateLinodeClient(apiKey string, timeout time.Duration, opts ...Option) (L

return linodeclient.NewLinodeClientWithTracing(
&linodeClient,
linodeclient.DefaultDecorator(),
), nil
}

Expand Down
75 changes: 32 additions & 43 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import (
infrastructurev1alpha2 "github.com/linode/cluster-api-provider-linode/api/v1alpha2"
"github.com/linode/cluster-api-provider-linode/controller"
"github.com/linode/cluster-api-provider-linode/observability/tracing"
"github.com/linode/cluster-api-provider-linode/observability/wrappers/reconciler"
"github.com/linode/cluster-api-provider-linode/version"

_ "go.uber.org/automaxprocs"
Expand Down Expand Up @@ -164,64 +163,54 @@ func main() {
os.Exit(1)
}

if err = reconciler.NewReconcilerWithTracing(
&controller.LinodeClusterReconciler{
Client: mgr.GetClient(),
Recorder: mgr.GetEventRecorderFor("LinodeClusterReconciler"),
WatchFilterValue: clusterWatchFilter,
LinodeApiKey: linodeToken,
},
).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: linodeClusterConcurrency}); err != nil {
if err = (&controller.LinodeClusterReconciler{
Client: mgr.GetClient(),
Recorder: mgr.GetEventRecorderFor("LinodeClusterReconciler"),
WatchFilterValue: clusterWatchFilter,
LinodeApiKey: linodeToken,
}).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: linodeClusterConcurrency}); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "LinodeCluster")
os.Exit(1)
}

if err = reconciler.NewReconcilerWithTracing(
&controller.LinodeMachineReconciler{
Client: mgr.GetClient(),
Recorder: mgr.GetEventRecorderFor("LinodeMachineReconciler"),
WatchFilterValue: machineWatchFilter,
LinodeApiKey: linodeToken,
LinodeDNSAPIKey: linodeDNSToken,
},
).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: linodeMachineConcurrency}); err != nil {
if err = (&controller.LinodeMachineReconciler{
Client: mgr.GetClient(),
Recorder: mgr.GetEventRecorderFor("LinodeMachineReconciler"),
WatchFilterValue: machineWatchFilter,
LinodeApiKey: linodeToken,
LinodeDNSAPIKey: linodeDNSToken,
}).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: linodeMachineConcurrency}); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "LinodeMachine")
os.Exit(1)
}

if err = reconciler.NewReconcilerWithTracing(
&controller.LinodeVPCReconciler{
Client: mgr.GetClient(),
Recorder: mgr.GetEventRecorderFor("LinodeVPCReconciler"),
WatchFilterValue: clusterWatchFilter,
LinodeApiKey: linodeToken,
},
).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: linodeVPCConcurrency}); err != nil {
if err = (&controller.LinodeVPCReconciler{
Client: mgr.GetClient(),
Recorder: mgr.GetEventRecorderFor("LinodeVPCReconciler"),
WatchFilterValue: clusterWatchFilter,
LinodeApiKey: linodeToken,
}).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: linodeVPCConcurrency}); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "LinodeVPC")
os.Exit(1)
}

if err = reconciler.NewReconcilerWithTracing(
&controller.LinodeObjectStorageBucketReconciler{
Client: mgr.GetClient(),
Logger: ctrl.Log.WithName("LinodeObjectStorageBucketReconciler"),
Recorder: mgr.GetEventRecorderFor("LinodeObjectStorageBucketReconciler"),
WatchFilterValue: objectStorageBucketWatchFilter,
LinodeApiKey: linodeToken,
},
).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: linodeObjectStorageBucketConcurrency}); err != nil {
if err = (&controller.LinodeObjectStorageBucketReconciler{
Client: mgr.GetClient(),
Logger: ctrl.Log.WithName("LinodeObjectStorageBucketReconciler"),
Recorder: mgr.GetEventRecorderFor("LinodeObjectStorageBucketReconciler"),
WatchFilterValue: objectStorageBucketWatchFilter,
LinodeApiKey: linodeToken,
}).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: linodeObjectStorageBucketConcurrency}); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "LinodeObjectStorageBucket")
os.Exit(1)
}

if err = reconciler.NewReconcilerWithTracing(
&controller.LinodePlacementGroupReconciler{
Client: mgr.GetClient(),
Recorder: mgr.GetEventRecorderFor("LinodePlacementGroupReconciler"),
WatchFilterValue: clusterWatchFilter,
LinodeApiKey: linodeToken,
},
).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: linodePlacementGroupConcurrency}); err != nil {
if err = (&controller.LinodePlacementGroupReconciler{
Client: mgr.GetClient(),
Recorder: mgr.GetEventRecorderFor("LinodePlacementGroupReconciler"),
WatchFilterValue: clusterWatchFilter,
LinodeApiKey: linodeToken,
}).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: linodePlacementGroupConcurrency}); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "LinodePlacementGroup")
os.Exit(1)
}
Expand Down
14 changes: 10 additions & 4 deletions controller/linodecluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ import (
infrav1alpha2 "github.com/linode/cluster-api-provider-linode/api/v1alpha2"
"github.com/linode/cluster-api-provider-linode/cloud/scope"
"github.com/linode/cluster-api-provider-linode/cloud/services"
wrappedruntimeclient "github.com/linode/cluster-api-provider-linode/observability/wrappers/runtimeclient"
wrappedruntimereconciler "github.com/linode/cluster-api-provider-linode/observability/wrappers/runtimereconciler"
"github.com/linode/cluster-api-provider-linode/util"
"github.com/linode/cluster-api-provider-linode/util/reconciler"
)
Expand All @@ -69,13 +71,13 @@ func (r *LinodeClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques

logger := ctrl.LoggerFrom(ctx).WithName("LinodeClusterReconciler").WithValues("name", req.NamespacedName.String())
linodeCluster := &infrav1alpha2.LinodeCluster{}
if err := r.Client.Get(ctx, req.NamespacedName, linodeCluster); err != nil {
if err := r.TracedClient().Get(ctx, req.NamespacedName, linodeCluster); err != nil {
logger.Info("Failed to fetch Linode cluster", "error", err.Error())

return ctrl.Result{}, client.IgnoreNotFound(err)
}

cluster, err := kutil.GetOwnerCluster(ctx, r.Client, linodeCluster.ObjectMeta)
cluster, err := kutil.GetOwnerCluster(ctx, r.TracedClient(), linodeCluster.ObjectMeta)
if err != nil {
logger.Info("Failed to get owner cluster", "error", err.Error())

Expand All @@ -91,7 +93,7 @@ func (r *LinodeClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
ctx,
r.LinodeApiKey,
scope.ClusterScopeParams{
Client: r.Client,
Client: r.TracedClient(),
Cluster: cluster,
LinodeCluster: linodeCluster,
},
Expand Down Expand Up @@ -300,10 +302,14 @@ func (r *LinodeClusterReconciler) SetupWithManager(mgr ctrl.Manager, options crc
kutil.ClusterToInfrastructureMapFunc(context.TODO(), infrav1alpha2.GroupVersion.WithKind("LinodeCluster"), mgr.GetClient(), &infrav1alpha2.LinodeCluster{}),
),
builder.WithPredicates(predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetLogger())),
).Complete(r)
).Complete(wrappedruntimereconciler.NewRuntimeReconcilerWithTracing(r, wrappedruntimereconciler.DefaultDecorator()))
if err != nil {
return fmt.Errorf("failed to build controller: %w", err)
}

return nil
}

func (r *LinodeClusterReconciler) TracedClient() client.Client {
return wrappedruntimeclient.NewRuntimeClientWithTracing(r.Client, wrappedruntimereconciler.DefaultDecorator())
}
20 changes: 15 additions & 5 deletions controller/linodemachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ import (
infrav1alpha2 "github.com/linode/cluster-api-provider-linode/api/v1alpha2"
"github.com/linode/cluster-api-provider-linode/cloud/scope"
"github.com/linode/cluster-api-provider-linode/cloud/services"
wrappedruntimeclient "github.com/linode/cluster-api-provider-linode/observability/wrappers/runtimeclient"
wrappedruntimereconciler "github.com/linode/cluster-api-provider-linode/observability/wrappers/runtimereconciler"
"github.com/linode/cluster-api-provider-linode/util"
"github.com/linode/cluster-api-provider-linode/util/reconciler"
)
Expand Down Expand Up @@ -118,7 +120,7 @@ func (r *LinodeMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques
log := ctrl.LoggerFrom(ctx).WithName("LinodeMachineReconciler").WithValues("name", req.NamespacedName.String())

linodeMachine := &infrav1alpha2.LinodeMachine{}
if err := r.Client.Get(ctx, req.NamespacedName, linodeMachine); err != nil {
if err := r.TracedClient().Get(ctx, req.NamespacedName, linodeMachine); err != nil {
if err = client.IgnoreNotFound(err); err != nil {
log.Error(err, "Failed to fetch LinodeMachine")
}
Expand All @@ -142,7 +144,7 @@ func (r *LinodeMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques
r.LinodeApiKey,
r.LinodeDNSAPIKey,
scope.MachineScopeParams{
Client: r.Client,
Client: r.TracedClient(),
Cluster: cluster,
Machine: machine,
LinodeCluster: &infrav1alpha2.LinodeCluster{},
Expand Down Expand Up @@ -221,7 +223,7 @@ func (r *LinodeMachineReconciler) reconcile(
Name: machineScope.Cluster.Spec.InfrastructureRef.Name,
}

if err := r.Client.Get(ctx, linodeClusterKey, machineScope.LinodeCluster); err != nil {
if err := r.Get(ctx, linodeClusterKey, machineScope.LinodeCluster); err != nil {
if err = client.IgnoreNotFound(err); err != nil {
return ctrl.Result{}, fmt.Errorf("get linodecluster %q: %w", linodeClusterKey, err)
}
Expand Down Expand Up @@ -740,7 +742,11 @@ func (r *LinodeMachineReconciler) reconcileDelete(

// SetupWithManager sets up the controller with the Manager.
func (r *LinodeMachineReconciler) SetupWithManager(mgr ctrl.Manager, options crcontroller.Options) error {
linodeMachineMapper, err := kutil.ClusterToTypedObjectsMapper(r.Client, &infrav1alpha2.LinodeMachineList{}, mgr.GetScheme())
linodeMachineMapper, err := kutil.ClusterToTypedObjectsMapper(
r.TracedClient(),
&infrav1alpha2.LinodeMachineList{},
mgr.GetScheme(),
)
if err != nil {
return fmt.Errorf("failed to create mapper for LinodeMachines: %w", err)
}
Expand All @@ -762,10 +768,14 @@ func (r *LinodeMachineReconciler) SetupWithManager(mgr ctrl.Manager, options crc
builder.WithPredicates(predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetLogger())),
).
WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetLogger(), r.WatchFilterValue)).
Complete(r)
Complete(wrappedruntimereconciler.NewRuntimeReconcilerWithTracing(r, wrappedruntimereconciler.DefaultDecorator()))
if err != nil {
return fmt.Errorf("failed to build controller: %w", err)
}

return nil
}

func (r *LinodeMachineReconciler) TracedClient() client.Client {
return wrappedruntimeclient.NewRuntimeClientWithTracing(r.Client, wrappedruntimeclient.DefaultDecorator())
}
8 changes: 4 additions & 4 deletions controller/linodemachine_controller_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (r *LinodeMachineReconciler) buildInstanceAddrs(ctx context.Context, machin
}

func (r *LinodeMachineReconciler) getOwnerMachine(ctx context.Context, linodeMachine infrav1alpha2.LinodeMachine, log logr.Logger) (*clusterv1.Machine, error) {
machine, err := kutil.GetOwnerMachine(ctx, r.Client, linodeMachine.ObjectMeta)
machine, err := kutil.GetOwnerMachine(ctx, r.TracedClient(), linodeMachine.ObjectMeta)
if err != nil {
if err = client.IgnoreNotFound(err); err != nil {
log.Error(err, "Failed to fetch owner machine")
Expand Down Expand Up @@ -212,7 +212,7 @@ func (r *LinodeMachineReconciler) getOwnerMachine(ctx context.Context, linodeMac
}

func (r *LinodeMachineReconciler) getClusterFromMetadata(ctx context.Context, machine clusterv1.Machine, log logr.Logger) (*clusterv1.Cluster, error) {
cluster, err := kutil.GetClusterFromMetadata(ctx, r.Client, machine.ObjectMeta)
cluster, err := kutil.GetClusterFromMetadata(ctx, r.TracedClient(), machine.ObjectMeta)
if err != nil {
if err = client.IgnoreNotFound(err); err != nil {
log.Error(err, "Failed to fetch cluster by label")
Expand Down Expand Up @@ -254,7 +254,7 @@ func (r *LinodeMachineReconciler) linodeClusterToLinodeMachines(logger logr.Logg
return nil
}

cluster, err := kutil.GetOwnerCluster(ctx, r.Client, linodeCluster.ObjectMeta)
cluster, err := kutil.GetOwnerCluster(ctx, r.TracedClient(), linodeCluster.ObjectMeta)
switch {
case apierrors.IsNotFound(err) || cluster == nil:
logger.Info("Cluster for LinodeCluster not found, skipping mapping")
Expand All @@ -281,7 +281,7 @@ func (r *LinodeMachineReconciler) requestsForCluster(ctx context.Context, namesp
labels := map[string]string{clusterv1.ClusterNameLabel: name}

machineList := clusterv1.MachineList{}
if err := r.Client.List(ctx, &machineList, client.InNamespace(namespace), client.MatchingLabels(labels)); err != nil {
if err := r.TracedClient().List(ctx, &machineList, client.InNamespace(namespace), client.MatchingLabels(labels)); err != nil {
return nil, err
}

Expand Down
18 changes: 14 additions & 4 deletions controller/linodeobjectstoragebucket_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ import (
infrav1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1"
"github.com/linode/cluster-api-provider-linode/cloud/scope"
"github.com/linode/cluster-api-provider-linode/cloud/services"
wrappedruntimeclient "github.com/linode/cluster-api-provider-linode/observability/wrappers/runtimeclient"
wrappedruntimereconciler "github.com/linode/cluster-api-provider-linode/observability/wrappers/runtimereconciler"
"github.com/linode/cluster-api-provider-linode/util"
"github.com/linode/cluster-api-provider-linode/util/reconciler"
)
Expand Down Expand Up @@ -81,7 +83,7 @@ func (r *LinodeObjectStorageBucketReconciler) Reconcile(ctx context.Context, req
logger := r.Logger.WithValues("name", req.NamespacedName.String())

objectStorageBucket := &infrav1alpha1.LinodeObjectStorageBucket{}
if err := r.Client.Get(ctx, req.NamespacedName, objectStorageBucket); err != nil {
if err := r.TracedClient().Get(ctx, req.NamespacedName, objectStorageBucket); err != nil {
if err = client.IgnoreNotFound(err); err != nil {
logger.Error(err, "Failed to fetch LinodeObjectStorageBucket", "name", req.NamespacedName.String())
}
Expand All @@ -93,7 +95,7 @@ func (r *LinodeObjectStorageBucketReconciler) Reconcile(ctx context.Context, req
ctx,
r.LinodeApiKey,
scope.ObjectStorageBucketScopeParams{
Client: r.Client,
Client: r.TracedClient(),
Bucket: objectStorageBucket,
Logger: &logger,
},
Expand Down Expand Up @@ -259,7 +261,11 @@ func (r *LinodeObjectStorageBucketReconciler) reconcileDelete(ctx context.Contex

// SetupWithManager sets up the controller with the Manager.
func (r *LinodeObjectStorageBucketReconciler) SetupWithManager(mgr ctrl.Manager, options crcontroller.Options) error {
linodeObjectStorageBucketMapper, err := kutil.ClusterToTypedObjectsMapper(r.Client, &infrav1alpha1.LinodeObjectStorageBucketList{}, mgr.GetScheme())
linodeObjectStorageBucketMapper, err := kutil.ClusterToTypedObjectsMapper(
r.TracedClient(),
&infrav1alpha1.LinodeObjectStorageBucketList{},
mgr.GetScheme(),
)
if err != nil {
return fmt.Errorf("failed to create mapper for LinodeObjectStorageBuckets: %w", err)
}
Expand All @@ -276,10 +282,14 @@ func (r *LinodeObjectStorageBucketReconciler) SetupWithManager(mgr ctrl.Manager,
&clusterv1.Cluster{},
handler.EnqueueRequestsFromMapFunc(linodeObjectStorageBucketMapper),
builder.WithPredicates(predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetLogger())),
).Complete(r)
).Complete(wrappedruntimereconciler.NewRuntimeReconcilerWithTracing(r, wrappedruntimereconciler.DefaultDecorator()))
if err != nil {
return fmt.Errorf("failed to build controller: %w", err)
}

return nil
}

func (r *LinodeObjectStorageBucketReconciler) TracedClient() client.Client {
return wrappedruntimeclient.NewRuntimeClientWithTracing(r.Client, wrappedruntimeclient.DefaultDecorator())
}
18 changes: 14 additions & 4 deletions controller/linodeplacementgroup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ import (

infrav1alpha2 "github.com/linode/cluster-api-provider-linode/api/v1alpha2"
"github.com/linode/cluster-api-provider-linode/cloud/scope"
wrappedruntimeclient "github.com/linode/cluster-api-provider-linode/observability/wrappers/runtimeclient"
wrappedruntimereconciler "github.com/linode/cluster-api-provider-linode/observability/wrappers/runtimereconciler"
"github.com/linode/cluster-api-provider-linode/util"
"github.com/linode/cluster-api-provider-linode/util/reconciler"
)
Expand Down Expand Up @@ -76,7 +78,7 @@ func (r *LinodePlacementGroupReconciler) Reconcile(ctx context.Context, req ctrl
log := ctrl.LoggerFrom(ctx).WithName("LinodePlacementGroupReconciler").WithValues("name", req.NamespacedName.String())

linodeplacementgroup := &infrav1alpha2.LinodePlacementGroup{}
if err := r.Client.Get(ctx, req.NamespacedName, linodeplacementgroup); err != nil {
if err := r.TracedClient().Get(ctx, req.NamespacedName, linodeplacementgroup); err != nil {
if err = client.IgnoreNotFound(err); err != nil {
log.Error(err, "Failed to fetch LinodePlacementGroup")
}
Expand All @@ -88,7 +90,7 @@ func (r *LinodePlacementGroupReconciler) Reconcile(ctx context.Context, req ctrl
ctx,
r.LinodeApiKey,
scope.PlacementGroupScopeParams{
Client: r.Client,
Client: r.TracedClient(),
LinodePlacementGroup: linodeplacementgroup,
},
)
Expand Down Expand Up @@ -291,7 +293,11 @@ func (r *LinodePlacementGroupReconciler) reconcileDelete(ctx context.Context, lo
//
//nolint:dupl // this is same as Placement Group, worth making generic later.
func (r *LinodePlacementGroupReconciler) SetupWithManager(mgr ctrl.Manager, options crcontroller.Options) error {
linodePlacementGroupMapper, err := kutil.ClusterToTypedObjectsMapper(r.Client, &infrav1alpha2.LinodePlacementGroupList{}, mgr.GetScheme())
linodePlacementGroupMapper, err := kutil.ClusterToTypedObjectsMapper(
r.TracedClient(),
&infrav1alpha2.LinodePlacementGroupList{},
mgr.GetScheme(),
)
if err != nil {
return fmt.Errorf("failed to create mapper for LinodePlacementGroups: %w", err)
}
Expand All @@ -312,10 +318,14 @@ func (r *LinodePlacementGroupReconciler) SetupWithManager(mgr ctrl.Manager, opti
&clusterv1.Cluster{},
handler.EnqueueRequestsFromMapFunc(linodePlacementGroupMapper),
builder.WithPredicates(predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetLogger())),
).Complete(r)
).Complete(wrappedruntimereconciler.NewRuntimeReconcilerWithTracing(r, wrappedruntimereconciler.DefaultDecorator()))
if err != nil {
return fmt.Errorf("failed to build controller: %w", err)
}

return nil
}

func (r *LinodePlacementGroupReconciler) TracedClient() client.Client {
return wrappedruntimeclient.NewRuntimeClientWithTracing(r.Client, wrappedruntimeclient.DefaultDecorator())
}
Loading

0 comments on commit e83dc0c

Please sign in to comment.