trace sdk.go resource manager methods (#98)

Updates the sdk.go-specific templates to take advantage of the new
ACK runtime logger that is embedded in all Context structs passed from
the ACK runtime v0.2.3 release and after.

In working these log traces into the various sdkXXX methods, I took this
opportunity to clean up some of the ugly template conditionals in these methods
that checked to see whether an Output shape was being referenced as a
variable after calling the aws-sdk-go API calls and setting the
associated variable name to `_` in order to prevent `unused variable
declared` compilation failures.

The template code for handling this used to look like this:

```
{{ $createCode := GoCodeSetCreateOutput .CRD "resp" "ko" 1 false }}
	{{ if not ( Empty $createCode ) }}resp{{ else }}_{{ end }}, respErr := rm.sdkapi.{{ .CRD.Ops.Create.ExportedName }}WithContext(ctx, input)
	rm.metrics.RecordAPICall("CREATE", "{{ .CRD.Ops.Create.ExportedName }}", respErr)
	if respErr != nil {
		return nil, respErr
	}
	// Merge in the information we read from the API call above to the copy of
	// the original Kubernetes object we passed to the function
	ko := r.ko.DeepCopy()
{{ $createCode }}
```

but now is simpler and easier to read:

```
	var resp {{ .CRD.GetOutputShapeGoType .CRD.Ops.Create }}
	resp, err = rm.sdkapi.{{ .CRD.Ops.Create.ExportedName }}WithContext(ctx, input)
	rm.metrics.RecordAPICall("CREATE", "{{ .CRD.Ops.Create.ExportedName }}", err)
	if err != nil {
		return nil, err
	}
	// Merge in the information we read from the API call above to the copy of
	// the original Kubernetes object we passed to the function
	ko := desired.ko.DeepCopy()
{{ GoCodeSetCreateOutput .CRD "resp" "ko" 1 false }}
```

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Author: jaypipes

go.mod

@@ -3,7 +3,7 @@ module github.com/aws-controllers-k8s/code-generator
 go 1.14
 
 require (
-	github.com/aws-controllers-k8s/runtime v0.2.0
+	github.com/aws-controllers-k8s/runtime v0.2.3
 	github.com/aws/aws-sdk-go v1.37.4
 	github.com/dlclark/regexp2 v1.4.0
 	// pin to v0.1.1 due to release problem with v0.1.2

go.sum

@@ -61,8 +61,8 @@ github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmV
 github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8=
 github.com/asaskevich/govalidator v0.0.0-20180720115003-f9ffefc3facf/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY=
 github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY=
-github.com/aws-controllers-k8s/runtime v0.2.0 h1:gd0Kq8xGelgkZoNjr8yZbHfpvPA1R+wfMCi1lT4H8x4=
-github.com/aws-controllers-k8s/runtime v0.2.0/go.mod h1:xA2F18PJerBHaqrS4de1lpP7skeSMeStkmh+3x5sWvw=
+github.com/aws-controllers-k8s/runtime v0.2.3 h1:pDDSXOJj5QLlC9OcgnGujeocQEg5U1oqQw3kUSDefLU=
+github.com/aws-controllers-k8s/runtime v0.2.3/go.mod h1:xA2F18PJerBHaqrS4de1lpP7skeSMeStkmh+3x5sWvw=
 github.com/aws/aws-sdk-go v1.37.4 h1:tWxrpMK/oRSXVnjUzhGeCWLR00fW0WF4V4sycYPPrJ8=
 github.com/aws/aws-sdk-go v1.37.4/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro=
 github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=

pkg/generate/ack/runtime_test.go

@@ -14,6 +14,7 @@
 package ack
 
 import (
+	"context"
 	"testing"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -23,6 +24,7 @@ import (
 
 	ackv1alpha1 "github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1"
 	ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare"
+	ackrtlog "github.com/aws-controllers-k8s/runtime/pkg/runtime/log"
 	acktypes "github.com/aws-controllers-k8s/runtime/pkg/types"
 )
 
@@ -82,4 +84,12 @@ func TestRuntimeDependency(t *testing.T) {
 
 	require.Implements((*acktypes.AWSResourceIdentifiers)(nil), new(fakeIdentifiers))
 	require.Implements((*acktypes.AWSResourceDescriptor)(nil), new(fakeDescriptor))
+
+	// ACK runtime 0.2.3 introduced a new logger that is now passed into the
+	// Context and retrievable using the `pkg/runtime/log.FromContext`
+	// function.  This function returns NoopLogger if no such logger is found
+	// in the context, but this check here is mostly to ensure that the new
+	// function used in ACK runtime 0.2.3 and templates in code-generator
+	// consuming 0.2.3 are properly pinned.
+	require.Implements((*acktypes.Logger)(nil), ackrtlog.FromContext(context.TODO()))
 }

pkg/generate/mq_test.go

@@ -51,3 +51,17 @@ func TestMQ_Broker(t *testing.T) {
 	require.True(found)
 	assert.Equal("*ackv1alpha1.SecretKeyReference", passAttr.GoType)
 }
+
+func TestMQ_GetOutputShapeGoType(t *testing.T) {
+	assert := assert.New(t)
+	require := require.New(t)
+
+	g := testutil.NewGeneratorForService(t, "mq")
+
+	crd := testutil.GetCRDByName(t, g, "Broker")
+	require.NotNil(crd)
+
+	exp := "*svcsdkapi.CreateBrokerResponse"
+	otype := crd.GetOutputShapeGoType(crd.Ops.Create)
+	assert.Equal(exp, otype)
+}

pkg/model/crd.go

@@ -365,6 +365,19 @@ func (r *CRD) SetOutputCustomMethodName(
 	return &resGenConfig.SetOutputCustomMethodName
 }
 
+// GetOutputShapeGoType returns the Go type of the supplied operation's Output
+// shape, renamed to use the standardized svcsdkapi alias.
+func (r *CRD) GetOutputShapeGoType(
+	op *awssdkmodel.Operation,
+) string {
+	if op == nil {
+		panic("called GetOutputShapeGoType on nil operation.")
+	}
+	orig := op.OutputRef.GoType()
+	// orig will contain "*<OutputShape>" with no package specifier
+	return "*svcsdkapi." + orig[1:]
+}
+
 // GetOutputWrapperFieldPath returns the JSON-Path of the output wrapper field
 // as *string for a given operation, if specified in generator config.
 func (r *CRD) GetOutputWrapperFieldPath(

templates/pkg/resource/manager.go.tpl

@@ -5,16 +5,16 @@ package {{ .CRD.Names.Snake }}
 import (
 	"context"
 	"fmt"
-	ackerr "github.com/aws-controllers-k8s/runtime/pkg/errors"
-	corev1 "k8s.io/api/core/v1"
 
 	ackv1alpha1 "github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1"
 	ackcfg "github.com/aws-controllers-k8s/runtime/pkg/config"
 	ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare"
+	ackerr "github.com/aws-controllers-k8s/runtime/pkg/errors"
 	ackmetrics "github.com/aws-controllers-k8s/runtime/pkg/metrics"
 	acktypes "github.com/aws-controllers-k8s/runtime/pkg/types"
 	"github.com/aws/aws-sdk-go/aws/session"
 	"github.com/go-logr/logr"
+	corev1 "k8s.io/api/core/v1"
 
 	svcsdk "github.com/aws/aws-sdk-go/service/{{ .ServiceIDClean }}"
 	svcsdkapi "github.com/aws/aws-sdk-go/service/{{ .ServiceIDClean }}/{{ .ServiceIDClean }}iface"

templates/pkg/resource/sdk.go.tpl

@@ -9,12 +9,14 @@ import (
 	ackv1alpha1 "github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1"
 	ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare"
 	ackerr "github.com/aws-controllers-k8s/runtime/pkg/errors"
+	ackrtlog "github.com/aws-controllers-k8s/runtime/pkg/runtime/log"
 	"github.com/aws/aws-sdk-go/aws"
 	svcsdk "github.com/aws/aws-sdk-go/service/{{ .ServiceIDClean }}"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	svcapitypes "github.com/aws-controllers-k8s/{{.ServiceIDClean }}-controller/apis/{{ .APIVersion }}"
+	svcsdkapi "github.com/aws/aws-sdk-go/service/{{ .ServiceIDClean }}"
 )
 
 // Hack to avoid import errors during build...
@@ -26,6 +28,7 @@ var (
 	_ = &svcapitypes.{{ .CRD.Names.Camel }}{}
 	_ = ackv1alpha1.AWSAccountID("")
 	_ = &ackerr.NotFound
+	_ = svcsdkapi.New
 )
 
 // sdkFind returns SDK-specific information about a supplied resource
@@ -40,52 +43,57 @@ var (
 {{- end }}
 
 // sdkCreate creates the supplied resource in the backend AWS service API and
-// returns a new resource with any fields in the Status field filled in
+// returns a copy of the resource with resource fields (in both Spec and
+// Status) filled in with values from the CREATE API operation's Output shape.
 func (rm *resourceManager) sdkCreate(
 	ctx context.Context,
-	r *resource,
-) (*resource, error) {
+	desired *resource,
+) (created *resource, err error) {
+	rlog := ackrtlog.FromContext(ctx)
+	exit := rlog.Trace("rm.sdkCreate")
+	defer exit(err)
+
 {{- if $hookCode := Hook .CRD "sdk_create_pre_build_request" }}
 {{ $hookCode }}
 {{- end }}
-{{- $customMethod := .CRD.GetCustomImplementation .CRD.Ops.Create -}}
-{{- if $customMethod }}
-	customResp, customRespErr := rm.{{ $customMethod }}(ctx, r)
-	if customResp != nil || customRespErr != nil {
-		return customResp, customRespErr
+{{- if $customMethod := .CRD.GetCustomImplementation .CRD.Ops.Create -}}
+	created, err = rm.{{ $customMethod }}(ctx, desired)
+	if created != nil || err != nil {
+		return created, err
 	}
 {{- end }}
-	input, err := rm.newCreateRequestPayload(ctx, r)
+	input, err := rm.newCreateRequestPayload(ctx, desired)
 	if err != nil {
 		return nil, err
 	}
 {{- if $hookCode := Hook .CRD "sdk_create_post_build_request" }}
 {{ $hookCode }} 
-{{- end }}	
-{{ $createCode := GoCodeSetCreateOutput .CRD "resp" "ko" 1 false }}
-	{{ if not ( Empty $createCode ) }}resp{{ else }}_{{ end }}, respErr := rm.sdkapi.{{ .CRD.Ops.Create.ExportedName }}WithContext(ctx, input)
+{{- end }}
+
+	var resp {{ .CRD.GetOutputShapeGoType .CRD.Ops.Create }}
+	resp, err = rm.sdkapi.{{ .CRD.Ops.Create.ExportedName }}WithContext(ctx, input)
 {{- if $hookCode := Hook .CRD "sdk_create_post_request" }}
 {{ $hookCode }}
 {{- end }}
-	rm.metrics.RecordAPICall("CREATE", "{{ .CRD.Ops.Create.ExportedName }}", respErr)
-	if respErr != nil {
-		return nil, respErr
+	rm.metrics.RecordAPICall("CREATE", "{{ .CRD.Ops.Create.ExportedName }}", err)
+	if err != nil {
+		return nil, err
 	}
 	// Merge in the information we read from the API call above to the copy of
 	// the original Kubernetes object we passed to the function
-	ko := r.ko.DeepCopy()
+	ko := desired.ko.DeepCopy()
 {{- if $hookCode := Hook .CRD "sdk_create_pre_set_output" }}
 {{ $hookCode }}
 {{- end }}
-{{ $createCode }}
+{{ GoCodeSetCreateOutput .CRD "resp" "ko" 1 false }}
 	rm.setStatusDefaults(ko)
-	{{ if $setOutputCustomMethodName := .CRD.SetOutputCustomMethodName .CRD.Ops.Create }}
-		// custom set output from response
-		ko, err = rm.{{ $setOutputCustomMethodName }}(ctx, r, resp, ko)
-		if err != nil {
-			return nil, err
-		}
-	{{ end }}
+{{- if $setOutputCustomMethodName := .CRD.SetOutputCustomMethodName .CRD.Ops.Create }}
+	// custom set output from response
+	ko, err = rm.{{ $setOutputCustomMethodName }}(ctx, desired, resp, ko)
+	if err != nil {
+		return nil, err
+	}
+{{- end }}
 {{- if $hookCode := Hook .CRD "sdk_create_post_set_output" }}
 {{ $hookCode }}
 {{- end }}
@@ -95,8 +103,8 @@ func (rm *resourceManager) sdkCreate(
 // newCreateRequestPayload returns an SDK-specific struct for the HTTP request
 // payload of the Create API call for the resource
 func (rm *resourceManager) newCreateRequestPayload(
-    ctx context.Context,
-    r *resource,
+	ctx context.Context,
+	r *resource,
 ) (*svcsdk.{{ .CRD.Ops.Create.InputRef.Shape.ShapeName }}, error) {
 	res := &svcsdk.{{ .CRD.Ops.Create.InputRef.Shape.ShapeName }}{}
 {{ GoCodeSetCreateInput .CRD "r.ko" "res" 1 }}
@@ -119,31 +127,33 @@ func (rm *resourceManager) newCreateRequestPayload(
 func (rm *resourceManager) sdkDelete(
 	ctx context.Context,
 	r *resource,
-) error {
+) (err error) {
+	rlog := ackrtlog.FromContext(ctx)
+	exit := rlog.Trace("rm.sdkDelete")
+	defer exit(err)
+
 {{- if .CRD.Ops.Delete }}
 {{- if $hookCode := Hook .CRD "sdk_delete_pre_build_request" }}
 {{ $hookCode }}
 {{- end }}
-{{ $customMethod := .CRD.GetCustomImplementation .CRD.Ops.Delete }}
-{{ if $customMethod }}
-	customRespErr := rm.{{ $customMethod }}(ctx, r)
-	if customRespErr != nil {
-		return customRespErr
+{{- if $customMethod := .CRD.GetCustomImplementation .CRD.Ops.Delete }}
+	if err = rm.{{ $customMethod }}(ctx, r); err != nil {
+		return err
 	}
-{{ end }}
+{{- end }}
 	input, err := rm.newDeleteRequestPayload(r)
 	if err != nil {
 		return err
 	}
 {{- if $hookCode := Hook .CRD "sdk_delete_post_build_request" }}
 {{ $hookCode }}
 {{- end }}
-	_, respErr := rm.sdkapi.{{ .CRD.Ops.Delete.Name }}WithContext(ctx, input)
-	rm.metrics.RecordAPICall("DELETE", "{{ .CRD.Ops.Delete.Name }}", respErr)
+	_, err = rm.sdkapi.{{ .CRD.Ops.Delete.Name }}WithContext(ctx, input)
+	rm.metrics.RecordAPICall("DELETE", "{{ .CRD.Ops.Delete.Name }}", err)
 {{- if $hookCode := Hook .CRD "sdk_delete_post_request" }}
 {{ $hookCode }}
 {{- end }}
-	return respErr
+	return err
 {{- else }}
 	// TODO(jaypipes): Figure this out...
 	return nil
@@ -335,4 +345,4 @@ func (rm *resourceManager) handleImmutableFieldsChangedCondition(
 
 	return &resource{ko}
 }
-{{- end }}
+{{- end }}

templates/pkg/resource/sdk_find_get_attributes.go.tpl

@@ -2,7 +2,11 @@
 func (rm *resourceManager) sdkFind(
 	ctx context.Context,
 	r *resource,
-) (*resource, error) {
+) (latest *resource, err error) {
+	rlog := ackrtlog.FromContext(ctx)
+	exit := rlog.Trace("rm.sdkFind")
+	defer exit(err)
+
 {{- if $hookCode := Hook .CRD "sdk_get_attributes_pre_build_request" }}
 {{ $hookCode }}
 {{- end }}
@@ -20,23 +24,23 @@ func (rm *resourceManager) sdkFind(
 {{- if $hookCode := Hook .CRD "sdk_get_attributes_post_build_request" }}
 {{ $hookCode }}
 {{- end }}
-{{ $setCode := GoCodeGetAttributesSetOutput .CRD "resp" "ko" 1 }}
-	{{ if not ( Empty $setCode ) }}resp{{ else }}_{{ end }}, respErr := rm.sdkapi.{{ .CRD.Ops.GetAttributes.ExportedName }}WithContext(ctx, input)
+	var resp {{ .CRD.GetOutputShapeGoType .CRD.Ops.GetAttributes }}
+	resp, err = rm.sdkapi.{{ .CRD.Ops.GetAttributes.ExportedName }}WithContext(ctx, input)
 {{- if $hookCode := Hook .CRD "sdk_get_attributes_post_request" }}
 {{ $hookCode }}
 {{- end }}
-	rm.metrics.RecordAPICall("GET_ATTRIBUTES", "{{ .CRD.Ops.GetAttributes.ExportedName }}", respErr)
-	if respErr != nil {
-		if awsErr, ok := ackerr.AWSError(respErr); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessageCheck .CRD 404 }}{
+	rm.metrics.RecordAPICall("GET_ATTRIBUTES", "{{ .CRD.Ops.GetAttributes.ExportedName }}", err)
+	if err != nil {
+		if awsErr, ok := ackerr.AWSError(err); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessageCheck .CRD 404 }}{
 			return nil, ackerr.NotFound
 		}
-		return nil, respErr
+		return nil, err
 	}
 
 	// Merge in the information we read from the API call above to the copy of
 	// the original Kubernetes object we passed to the function
 	ko := r.ko.DeepCopy()
-{{ $setCode }}
+{{ GoCodeGetAttributesSetOutput .CRD "resp" "ko" 1 }}
 {{- if $hookCode := Hook .CRD "sdk_get_attributes_pre_set_output" }}
 {{ $hookCode }}
 {{- end }}

templates/pkg/resource/sdk_find_read_many.go.tpl

@@ -2,7 +2,11 @@
 func (rm *resourceManager) sdkFind(
 	ctx context.Context,
 	r *resource,
-) (*resource, error) {
+) (latest *resource, err error) {
+	rlog := ackrtlog.FromContext(ctx)
+	exit := rlog.Trace("rm.sdkFind")
+	defer exit(err)
+
 {{- if $hookCode := Hook .CRD "sdk_read_many_pre_build_request" }}
 {{ $hookCode }}
 {{- end }}
@@ -13,17 +17,17 @@ func (rm *resourceManager) sdkFind(
 {{- if $hookCode := Hook .CRD "sdk_read_many_post_build_request" }}
 {{ $hookCode }}
 {{- end }}
-{{ $setCode := GoCodeSetReadManyOutput .CRD "resp" "ko" 1 true }}
-	{{ if not ( Empty $setCode ) }}resp{{ else }}_{{ end }}, respErr := rm.sdkapi.{{ .CRD.Ops.ReadMany.ExportedName }}WithContext(ctx, input)
+	var resp {{ .CRD.GetOutputShapeGoType .CRD.Ops.ReadMany }}
+	resp, err = rm.sdkapi.{{ .CRD.Ops.ReadMany.ExportedName }}WithContext(ctx, input)
 {{- if $hookCode := Hook .CRD "sdk_read_many_post_request" }}
 {{ $hookCode }}
 {{- end }}
-	rm.metrics.RecordAPICall("READ_MANY", "{{ .CRD.Ops.ReadMany.ExportedName }}", respErr)
-	if respErr != nil {
-		if awsErr, ok := ackerr.AWSError(respErr); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessageCheck .CRD 404 }}{
+	rm.metrics.RecordAPICall("READ_MANY", "{{ .CRD.Ops.ReadMany.ExportedName }}", err)
+	if err != nil {
+		if awsErr, ok := ackerr.AWSError(err); ok && awsErr.Code() == "{{ ResourceExceptionCode .CRD 404 }}" {{ GoCodeSetExceptionMessageCheck .CRD 404 }}{
 			return nil, ackerr.NotFound
 		}
-		return nil, respErr
+		return nil, err
 	}
 
 	// Merge in the information we read from the API call above to the copy of
@@ -32,15 +36,15 @@ func (rm *resourceManager) sdkFind(
 {{- if $hookCode := Hook .CRD "sdk_read_many_pre_set_output" }}
 {{ $hookCode }}
 {{- end }}
-{{ $setCode }}
+{{ GoCodeSetReadManyOutput .CRD "resp" "ko" 1 true }}
 	rm.setStatusDefaults(ko)
-{{ if $setOutputCustomMethodName := .CRD.SetOutputCustomMethodName .CRD.Ops.ReadMany }}
+{{- if $setOutputCustomMethodName := .CRD.SetOutputCustomMethodName .CRD.Ops.ReadMany }}
 	// custom set output from response
 	ko, err = rm.{{ $setOutputCustomMethodName }}(ctx, r, resp, ko)
 	if err != nil {
 		return nil, err
 	}
-{{ end }}
+{{- end }}
 {{- if $hookCode := Hook .CRD "sdk_read_many_post_set_output" }}
 {{ $hookCode }}
 {{- end }}