Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
shulin-sq committed Jun 13, 2024
1 parent 12e6131 commit 728309a
Show file tree
Hide file tree
Showing 18 changed files with 72 additions and 78 deletions.
2 changes: 1 addition & 1 deletion pkg/controllers/accesslogpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func RegisterAccessLogPolicyController(
}

func (r *accessLogPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
ctx = gwlog.StartReconcileTrace(ctx, r.log, "accesslogpolicy", req.Name)
ctx = gwlog.StartReconcileTrace(ctx, r.log, "accesslogpolicy", req.Name, req.Namespace)
defer func() {
gwlog.EndReconcileTrace(ctx, r.log)
}()
Expand Down
3 changes: 2 additions & 1 deletion pkg/controllers/gateway_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package controllers
import (
"context"
"fmt"

anv1alpha1 "github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1"
"github.com/aws/aws-application-networking-k8s/pkg/aws/services"
"github.com/aws/aws-application-networking-k8s/pkg/controllers/eventhandlers"
Expand Down Expand Up @@ -119,7 +120,7 @@ func RegisterGatewayController(
//+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways/finalizers,verbs=update

func (r *gatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
ctx = gwlog.StartReconcileTrace(ctx, r.log, "gateway", req.Name)
ctx = gwlog.StartReconcileTrace(ctx, r.log, "gateway", req.Name, req.Namespace)
defer func() {
gwlog.EndReconcileTrace(ctx, r.log)
}()
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/gatewayclass_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func RegisterGatewayClassController(log gwlog.Logger, mgr ctrl.Manager) error {
//+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gatewayclasses/finalizers,verbs=update

func (r *gatewayClassReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
ctx = gwlog.StartReconcileTrace(ctx, r.log, "gatewayclass", req.Name)
ctx = gwlog.StartReconcileTrace(ctx, r.log, "gatewayclass", req.Name, req.Namespace)
defer func() {
gwlog.EndReconcileTrace(ctx, r.log)
}()
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/iamauthpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func RegisterIAMAuthPolicyController(log gwlog.Logger, mgr ctrl.Manager, cloud p
//
// Policy Attachment Spec is defined in [GEP-713]: https://gateway-api.sigs.k8s.io/geps/gep-713/.
func (c *IAMAuthPolicyController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
ctx = gwlog.StartReconcileTrace(ctx, c.log, "iamauthpolicy", req.Name)
ctx = gwlog.StartReconcileTrace(ctx, c.log, "iamauthpolicy", req.Name, req.Namespace)
defer func() {
gwlog.EndReconcileTrace(ctx, c.log)
}()
Expand Down
8 changes: 1 addition & 7 deletions pkg/controllers/route_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,17 +149,11 @@ func RegisterAllRouteControllers(
}

func (r *routeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
ctx = gwlog.StartReconcileTrace(ctx, r.log, "route", req.Name)
ctx = gwlog.StartReconcileTrace(ctx, r.log, "route", req.Name, req.Namespace)
defer func() {
gwlog.EndReconcileTrace(ctx, r.log)
}()

r.log.Infow(ctx, "reconcile starting", gwlog.GetMetadata(ctx)...)

defer func() {
r.log.Infow(ctx, "reconcile completed", gwlog.GetMetadata(ctx)...)
}()

recErr := r.reconcile(ctx, req)
if recErr != nil {
r.log.Infow(ctx, "reconcile error", "name", req.Name, "message", recErr.Error())
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/service_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func RegisterServiceController(
//+kubebuilder:rbac:groups=core,resources=configmaps, verbs=create;delete;patch;update;get;list;watch

func (r *serviceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
ctx = gwlog.StartReconcileTrace(ctx, r.log, "service", req.Name)
ctx = gwlog.StartReconcileTrace(ctx, r.log, "service", req.Name, req.Namespace)
defer func() {
gwlog.EndReconcileTrace(ctx, r.log)
}()
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/serviceexport_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func RegisterServiceExportController(
//+kubebuilder:rbac:groups=application-networking.k8s.aws,resources=serviceexports/finalizers,verbs=update

func (r *serviceExportReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
ctx = gwlog.StartReconcileTrace(ctx, r.log, "serviceexport", req.Name)
ctx = gwlog.StartReconcileTrace(ctx, r.log, "serviceexport", req.Name, req.Namespace)
defer func() {
gwlog.EndReconcileTrace(ctx, r.log)
}()
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/serviceimport_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func RegisterServiceImportController(
//+kubebuilder:rbac:groups=application-networking.k8s.aws,resources=serviceimports/finalizers,verbs=update

func (r *serviceImportReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
ctx = gwlog.StartReconcileTrace(ctx, r.log, "serviceimport", req.Name)
ctx = gwlog.StartReconcileTrace(ctx, r.log, "serviceimport", req.Name, req.Namespace)
defer func() {
gwlog.EndReconcileTrace(ctx, r.log)
}()
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/targetgrouppolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func RegisterTargetGroupPolicyController(log gwlog.Logger, mgr ctrl.Manager) err
}

func (c *TargetGroupPolicyController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
ctx = gwlog.StartReconcileTrace(ctx, c.log, "targetgrouppolicy", req.Name)
ctx = gwlog.StartReconcileTrace(ctx, c.log, "targetgrouppolicy", req.Name, req.Namespace)
defer func() {
gwlog.EndReconcileTrace(ctx, c.log)
}()
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/vpcassociationpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func RegisterVpcAssociationPolicyController(log gwlog.Logger, cloud pkg_aws.Clou
}

func (c *vpcAssociationPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
ctx = gwlog.StartReconcileTrace(ctx, c.log, "vpcassociationpolicy", req.Name)
ctx = gwlog.StartReconcileTrace(ctx, c.log, "vpcassociationpolicy", req.Name, req.Namespace)
defer func() {
gwlog.EndReconcileTrace(ctx, c.log)
}()
Expand Down
4 changes: 2 additions & 2 deletions pkg/deploy/lattice/listener_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ func Test_defaultListenerManager_getLatticeListenerDefaultAction_HTTP_HTTPS_List
log: gwlog.FallbackLogger,
cloud: cloud,
}
got, err := d.getLatticeListenerDefaultAction(modelListener)
got, err := d.getLatticeListenerDefaultAction(context.TODO(), modelListener)
if tt.wantErr {
assert.Error(t, err)
} else {
Expand Down Expand Up @@ -643,7 +643,7 @@ func Test_ListenerManager_getLatticeListenerDefaultAction_TLS_PASSTHROUGH_Listen
log: gwlog.FallbackLogger,
cloud: cloud,
}
gotDefaultAction, err := d.getLatticeListenerDefaultAction(modelListener)
gotDefaultAction, err := d.getLatticeListenerDefaultAction(context.TODO(), modelListener)
if tt.wantErr {
assert.Error(t, err)
} else {
Expand Down
10 changes: 5 additions & 5 deletions pkg/deploy/lattice/target_group_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -458,24 +458,24 @@ func (s *defaultTargetGroupManager) findSvcExportTG(ctx context.Context, svcImpo
// ResolveRuleTgIds populates all target group ids in the rule's actions
func (s *defaultTargetGroupManager) ResolveRuleTgIds(ctx context.Context, ruleAction *model.RuleAction, stack core.Stack) error {
if len(ruleAction.TargetGroups) == 0 {
s.log.Debugf("no target groups to resolve for rule")
s.log.Debugf(ctx, "no target groups to resolve for rule")
return nil
}
for i, ruleActionTg := range ruleAction.TargetGroups {
if ruleActionTg.StackTargetGroupId == "" && ruleActionTg.SvcImportTG == nil && ruleActionTg.LatticeTgId == "" {
return errors.New("rule TG is missing a required target group identifier")
}
if ruleActionTg.LatticeTgId != "" {
s.log.Debugf("Rule TG %d already resolved %s", i, ruleActionTg.LatticeTgId)
s.log.Debugf(ctx, "Rule TG %d already resolved %s", i, ruleActionTg.LatticeTgId)
continue
}
if ruleActionTg.StackTargetGroupId != "" {
if ruleActionTg.StackTargetGroupId == model.InvalidBackendRefTgId {
s.log.Debugf("Rule TG has an invalid backendref, setting TG id to invalid")
s.log.Debugf(ctx, "Rule TG has an invalid backendref, setting TG id to invalid")
ruleActionTg.LatticeTgId = model.InvalidBackendRefTgId
continue
}
s.log.Debugf("Fetching TG %d from the stack (ID %s)", i, ruleActionTg.StackTargetGroupId)
s.log.Debugf(ctx, "Fetching TG %d from the stack (ID %s)", i, ruleActionTg.StackTargetGroupId)
stackTg := &model.TargetGroup{}
err := stack.GetResource(ruleActionTg.StackTargetGroupId, stackTg)
if err != nil {
Expand All @@ -487,7 +487,7 @@ func (s *defaultTargetGroupManager) ResolveRuleTgIds(ctx context.Context, ruleAc
ruleActionTg.LatticeTgId = stackTg.Status.Id
}
if ruleActionTg.SvcImportTG != nil {
s.log.Debugf("Getting target group for service import %s %s (%s, %s)",
s.log.Debugf(ctx, "Getting target group for service import %s %s (%s, %s)",
ruleActionTg.SvcImportTG.K8SServiceName, ruleActionTg.SvcImportTG.K8SServiceNamespace,
ruleActionTg.SvcImportTG.K8SClusterName, ruleActionTg.SvcImportTG.VpcId)
tgId, err := s.findSvcExportTG(ctx, *ruleActionTg.SvcImportTG)
Expand Down
2 changes: 1 addition & 1 deletion pkg/gateway/model_build_lattice_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (t *latticeServiceModelBuildTask) buildModel(ctx context.Context) error {
t.log.Debugf(ctx, "Building rules for %d listeners", len(modelListeners))
for _, modelListener := range modelListeners {
if modelListener.Spec.Protocol == vpclattice.ListenerProtocolTlsPassthrough {
t.log.Debugf("Skip building rules for TLS_PASSTHROUGH listener %s, since lattice TLS_PASSTHROUGH listener can only have listener defaultAction and without any other rule", modelListener.ID())
t.log.Debugf(ctx, "Skip building rules for TLS_PASSTHROUGH listener %s, since lattice TLS_PASSTHROUGH listener can only have listener defaultAction and without any other rule", modelListener.ID())
continue
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/gateway/model_build_listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (t *latticeServiceModelBuildTask) extractListenerInfo(
listenerPort := int(section.Port)
protocol := section.Protocol
if isTLSPassthroughGatewayListener(&section) {
t.log.Debugf("Found TLS passthrough section %v", section.TLS)
t.log.Debugf(ctx, "Found TLS passthrough section %v", section.TLS)
protocol = vpclattice.ListenerProtocolTlsPassthrough
}
return int64(listenerPort), string(protocol), nil
Expand Down
4 changes: 2 additions & 2 deletions pkg/utils/gwlog/actions.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package gwlog

const ReconcileStart = "[ACTION_RECONCILE_START]"
const ReconcileEnd = "[ACTION_RECONCILE_END]"
const ReconcileStart = "RECONCILE_START_MARKER"
const ReconcileEnd = "RECONCILE_END_MARKER"
52 changes: 26 additions & 26 deletions pkg/utils/gwlog/gwlog.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,97 +14,97 @@ type TracedLogger struct {
InnerLogger *zap.SugaredLogger
}

func (s *TracedLogger) Infoln(args ...interface{}) {
s.InnerLogger.Infoln(args...)
func (t *TracedLogger) Infoln(args ...interface{}) {
t.InnerLogger.Infoln(args...)
}

func (t *TracedLogger) Infow(ctx context.Context, msg string, keysAndValues ...interface{}) {
if GetTrace(ctx) != "" {
keysAndValues = append(keysAndValues, traceID, GetTrace(ctx))
if tr := GetTraceID(ctx); tr != "" {
keysAndValues = append(keysAndValues, traceID, tr)
}
t.InnerLogger.Infow(msg, keysAndValues...)
}

func (t *TracedLogger) Infof(ctx context.Context, template string, args ...interface{}) {
if GetTrace(ctx) != "" {
t.InnerLogger.Infow(fmt.Sprintf(template, args...), traceID, GetTrace(ctx))
if tr := GetTraceID(ctx); tr != "" {
t.InnerLogger.Infow(fmt.Sprintf(template, args...), traceID, tr)
return
}
t.InnerLogger.Infof(template, args...)
}

func (t *TracedLogger) Info(ctx context.Context, msg string) {
if GetTrace(ctx) != "" {
t.InnerLogger.Infow(msg, traceID, GetTrace(ctx))
if tr := GetTraceID(ctx); tr != "" {
t.InnerLogger.Infow(msg, traceID, tr)
return
}
t.InnerLogger.Info(msg)
}

func (t *TracedLogger) Errorw(ctx context.Context, msg string, keysAndValues ...interface{}) {
if GetTrace(ctx) != "" {
keysAndValues = append(keysAndValues, traceID, GetTrace(ctx))
if tr := GetTraceID(ctx); tr != "" {
keysAndValues = append(keysAndValues, traceID, tr)
}
t.InnerLogger.Errorw(msg, keysAndValues)
}

func (t *TracedLogger) Errorf(ctx context.Context, template string, args ...interface{}) {
if GetTrace(ctx) != "" {
t.InnerLogger.Errorw(fmt.Sprintf(template, args...), traceID, GetTrace(ctx))
if tr := GetTraceID(ctx); tr != "" {
t.InnerLogger.Errorw(fmt.Sprintf(template, args...), traceID, tr)
return
}
t.InnerLogger.Errorf(template, args...)
}

func (t *TracedLogger) Error(ctx context.Context, msg string) {
if GetTrace(ctx) != "" {
t.InnerLogger.Errorw(msg, traceID, GetTrace(ctx))
if tr := GetTraceID(ctx); tr != "" {
t.InnerLogger.Errorw(msg, traceID, tr)
return
}
t.InnerLogger.Error(msg)
}

func (t *TracedLogger) Debugw(ctx context.Context, msg string, keysAndValues ...interface{}) {
if GetTrace(ctx) != "" {
keysAndValues = append(keysAndValues, traceID, GetTrace(ctx))
if tr := GetTraceID(ctx); tr != "" {
keysAndValues = append(keysAndValues, traceID, tr)
}
t.InnerLogger.Debugw(msg, keysAndValues...)
}

func (t *TracedLogger) Debugf(ctx context.Context, template string, args ...interface{}) {
if GetTrace(ctx) != "" {
t.InnerLogger.Debugw(fmt.Sprintf(template, args...), traceID, GetTrace(ctx))
if tr := GetTraceID(ctx); tr != "" {
t.InnerLogger.Debugw(fmt.Sprintf(template, args...), traceID, tr)
return
}
t.InnerLogger.Debugf(template, args...)
}

func (t *TracedLogger) Debug(ctx context.Context, msg string) {
if GetTrace(ctx) != "" {
t.InnerLogger.Debugw(msg, traceID, GetTrace(ctx))
if tr := GetTraceID(ctx); tr != "" {
t.InnerLogger.Debugw(msg, traceID, tr)
return
}
t.InnerLogger.Debug(msg)
}

func (t *TracedLogger) Warnw(ctx context.Context, msg string, keysAndValues ...interface{}) {
if GetTrace(ctx) != "" {
keysAndValues = append(keysAndValues, traceID, GetTrace(ctx))
if tr := GetTraceID(ctx); tr != "" {
keysAndValues = append(keysAndValues, traceID, tr)
}
t.InnerLogger.Warnw(msg, keysAndValues...)
}

func (t *TracedLogger) Warnf(ctx context.Context, template string, args ...interface{}) {
if GetTrace(ctx) != "" {
t.InnerLogger.Warnw(fmt.Sprintf(template, args...), traceID, GetTrace(ctx))
if tr := GetTraceID(ctx); tr != "" {
t.InnerLogger.Warnw(fmt.Sprintf(template, args...), traceID, tr)
return
}
t.InnerLogger.Warnf(template, args...)
}

func (t *TracedLogger) Warn(ctx context.Context, msg string) {
if GetTrace(ctx) != "" {
t.InnerLogger.Warnw(msg, traceID, GetTrace(ctx))
if tr := GetTraceID(ctx); tr != "" {
t.InnerLogger.Warnw(msg, traceID, tr)
return
}
t.InnerLogger.Warn(msg)
Expand Down
Loading

0 comments on commit 728309a

Please sign in to comment.