Eliminate use of Sprintf in log messages

[davewalter]

Is there a related GitHub Issue?

#1929

What is this change about?

This PR eliminates the use of Sprintf in log messages. It moves injected values to named values instead.

Does this PR introduce a breaking change?

No.

Acceptance Steps

Deploy and run tests. Check the logs and confirm that the intended data is still being recorded.

[danail-branekov]

There are a few more occurrences of Sprintf when logging:

api/handlers/build.go:          return nil, apierrors.LogAndReturn(logger, apierrors.ForbiddenAsNotFound(err), fmt.Sprintf("Failed to fetch %s from Kubernetes", repositories.BuildResourceType), "guid", buildGUID)
api/handlers/deployment.go:             return nil, apierrors.LogAndReturn(logger, apierrors.NewRollingDeployNotSupportedError(err), fmt.Sprintf("Runner '%s' does not support rolling deploys", h.runnerName))
api/handlers/service_binding.go:                return nil, apierrors.LogAndReturn(logger, err, fmt.Sprintf("failed to get %s", repositories.AppResourceType))
api/handlers/service_binding.go:                return nil, apierrors.LogAndReturn(logger, err, fmt.Sprintf("failed to get %s", repositories.ServiceInstanceResourceType))
api/handlers/service_binding.go:                return nil, apierrors.LogAndReturn(logger, err, fmt.Sprintf("failed to list %s", repositories.ServiceBindingResourceType))
api/handlers/service_binding.go:                        return nil, apierrors.LogAndReturn(logger, err, fmt.Sprintf("failed to list %s", repositories.AppResourceType))
controllers/controllers/workloads/cfspace_controller.go:        log.V(1).Info(fmt.Sprintf("finalizing CFSpace for %fs", duration.Seconds()))
tests/e2e/helpers/fail_handler.go:      logHeader := fmt.Sprintf(
tests/e2e/helpers/fail_handler.go:              logHeader = fmt.Sprintf(
tools/k8s/reconcile.go:         log.Info(fmt.Sprintf("unable to fetch %T", obj), "reason", err)

[davewalter]

Thanks for taking a look at this @danail-branekov.

A lot of the occurrences you mentioned are simply using a constant string value in an error message, which I think is just a case of trying to keep the code dry. I have replaced the use of fmt.Sprintf with string concatenation to hopefully make this clearer, but I don't think that they should be separate named values as that would make the log message look something like:

{ "severity": "INFO", ..., "message": "failed to fetch object", "objectType": "App" }

instead of just:

{ "severity": "INFO", ..., "message": "failed to fetch App" }

I deliberately ignored the use of Sprintf in the e2e test fail handler, and I'm not sure what to do about the one in the reconcile helper that fetches the type of an object without resorting to the reflect package, which seems worse to me.