Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add verbosity flag #1429

Closed
carolynvs opened this issue Jan 28, 2021 · 5 comments · Fixed by #2258
Closed

Add verbosity flag #1429

carolynvs opened this issue Jan 28, 2021 · 5 comments · Fixed by #2258
Assignees
Labels
1 - 🍫 Eat chocolate _after_ emergency donuts breaking change 💥 Breaking changes to Porter's CLI, config or behavior enhancement New code incoming! hmm 🛑🤔 Needs more thinking time. Don't start on it yet, please.
Milestone

Comments

@carolynvs
Copy link
Member

carolynvs commented Jan 28, 2021

What design is being proposed?
Replace --debug and --debug-plugins and porter build --verbose with a single --verbosity|v flag.

#1353

🚨 This needs a PEP.

@carolynvs carolynvs added the enhancement New code incoming! label Jan 28, 2021
@carolynvs carolynvs added this to the 1.0 milestone Jan 28, 2021
@carolynvs carolynvs added this to Inbox in Porter and Mixins via automation Jan 28, 2021
@carolynvs carolynvs added the breaking change 💥 Breaking changes to Porter's CLI, config or behavior label May 10, 2021
@carolynvs
Copy link
Member Author

I have a branch with structured logging and tracing with opentracing that I think will work well for this issue. My plan is to switching to opentelemetry, and then submit a PR with just the plumbing for this, and a feature flag to toggle it. Then we can slowly convert existing log messages over to structured logs.

All logs are emitted to stderr, and only the command output goes to stdout. So even messages like "Building invocation image" go to stderr in the new scheme.

Instead of log levels with numbers, I'm experimenting with allowing to filter on associated key value pairs on the logs.

  • level: debug, info, warn, error
  • type:
    • stdout: it was printed to the console for the user, for example they requested -o json and it's the installation resource
    • task: building invocation image, installing bundle, etc. These are spans in our traces
    • command: any external commands executed, like calling out to jq or helm
    • protocol: communication with mixins/plugins/http/mongo
  • area: installer, runtime, plugin key, mixin key, etc
  • context: command, resolved bundle, correlation id (see a chain of everything executed from a command). These are all additional arbitrary key/value pairs stored on a trace span or emited to the logs.

This will let us see a ton of info in jaeger and not rely so much on print debugging. I'm having logs printed to the output in plaintext, optionally printed as the structured logs, writing the structured logs to a file in ~/.porter/logs, and also sending them to jaeger (if tracing is enabled).

I am still thinking through how to apply filters to the 3 different sinks: jaeger, structure log file, and console output. I think people will want to be able to tune the firehose, mostly to limit data usage, so we may end up with log.level, console.level and trace.level, with a global level that will set all 3 at once for simplicity. Because I can totally see tracing at debug level, just in case, but only showing errors to the console or some other combination.

@carolynvs carolynvs self-assigned this Oct 7, 2021
@carolynvs
Copy link
Member Author

We still need this in v1 for two reasons: backwards incompatible changes to output format and flags, and if anyone is using porter in production we really want the code fully instrumented so we can troubleshoot.

@carolynvs carolynvs added the 1 - 🍫 Eat chocolate _after_ emergency donuts label Nov 18, 2021
@carolynvs carolynvs removed their assignment Feb 19, 2022
@carolynvs carolynvs moved this from Inbox to Backlog in Porter and Mixins Feb 22, 2022
@carolynvs carolynvs added the hmm 🛑🤔 Needs more thinking time. Don't start on it yet, please. label Mar 4, 2022
@carolynvs
Copy link
Member Author

I'd like to have two configurable settings: a flag to control how verbose the console output is, and another to control how much is sent to the tracer/logger. That way I can run at a level that isn't overwhelming and if a command fails go look up the extra info from the log file or jaeger.

@carolynvs carolynvs self-assigned this Jun 16, 2022
@carolynvs
Copy link
Member Author

--verbosity will control the console output level. It would default to info when the flag is omitted. It can be set with PORTER_VERBOSITY or verbosity: debug in the config file.

Separately, the log-level configuration setting controls how much is output to the log file (if any):

# ~/.porter/config.yaml
logs:
  log-to-file: true # turn on the log file
  log-level: debug # dump all logs into the file

You can do a mix:

porter install -v error
# the config file has log-level: debug

Then the console would only show errors, and the log file would contain debug through error messages.


if you run porter install it will output info logs to the console
if you run porter install -v, that's CLI error. We will prompt for the the level
if you run porter install -v error, we only print errors to the console

@github-actions
Copy link

Closed by #2258.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - 🍫 Eat chocolate _after_ emergency donuts breaking change 💥 Breaking changes to Porter's CLI, config or behavior enhancement New code incoming! hmm 🛑🤔 Needs more thinking time. Don't start on it yet, please.
Projects
Development

Successfully merging a pull request may close this issue.

1 participant