Added debug logs for render command #5328
Conversation
|
Added logs in just a few places around pulling and starting functions. I am not that familiar with the render code so feel free to reccomend other places where its useful. Ill go through it as well myself. |
Signed-off-by: lsviben <sviben.lovro@gmail.com>
lsviben
force-pushed
the
render-debug-logs
branch
from
dc61ae0
to
1f35023
Compare
| // Run our Functions. | ||
| conns := map[string]*grpc.ClientConn{} | ||
| for _, fn := range in.Functions { | ||
| runtime, err := GetRuntime(fn) | ||
| if err != nil { | ||
| return Outputs{}, errors.Wrapf(err, "cannot get runtime for Function %q", fn.GetName()) | ||
| } | ||
| rctx, err := runtime.Start(ctx) | ||
| rctx, err := runtime.Start(ctx, logger) |
There was a problem hiding this comment.
Would it make sense to pass the logger in GetRuntime? So it would be a field of the runtime struct rather than an argument to the Start method?
| @@ -49,7 +50,7 @@ const ( | |||
| // A Runtime runs a Function. | |||
| type Runtime interface { | |||
| // Start the Function. | |||
| Start(ctx context.Context) (RuntimeContext, error) | |||
| Start(ctx context.Context, logger logging.Logger) (RuntimeContext, error) | |||
There was a problem hiding this comment.
| Start(ctx context.Context, logger logging.Logger) (RuntimeContext, error) | |
| Start(ctx context.Context, l logging.Logger) (RuntimeContext, error) |
Nit. Or log if l is too short, but "logger logging logger" stutters a lot. 😄 In other contexts I can live with it because the logger variable tends to live for a lot of lines (you can't always easily see what it is from its type). Maybe log everywhere would be a good compromise though? 🤔
|
@lsviben I'm going to merge this so that it's in the first RC for 1.15, feel free to address the nits above in a follow up PR. |
|
Backport failed for |
Description of your changes
Adds debug logs to
crossplane beta render.For
renderthe output is expected to be a valid yaml file, so we should be carefull where and when to log. Actually we dont need to do anything special as the CLI logger is already active only when--verboseflag is provider, plus it logs by default to stderr, so users can filter the command output from the logs if needed.How it looks like:
crossplane beta render ...- no changescrossplane beta render ... --verbose:crossplane beta render ... --verbose 2>crossplane beta render ... --verbose 1>Fixes #4809
I have:
make reviewableto ensure this PR is ready for review.[] Added or updated unit tests.[] Added or updated e2e tests.[] Linked a PR or a docs tracking issue to document this change.[] Addedbackport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.