Skip to content

Print WPCLI output (both stdout/stderr) to wp-cron-runner stream#36

Open
walthowd wants to merge 1 commit intotrunkfrom
walt/add-wp-cli-output
Open

Print WPCLI output (both stdout/stderr) to wp-cron-runner stream#36
walthowd wants to merge 1 commit intotrunkfrom
walt/add-wp-cli-output

Conversation

@walthowd
Copy link
Copy Markdown

Re: @rinatkhaziev and @rebeccahum 's question - The wp-cron-runner invokes WP-CLI commands from within inside the container.

The virtual pty inside the golang program logs the output to a temporary file and then reads the log through the TCP stream back through vip-cli -- However the effective k8s/internal logging (and thus dashboard) logs are lost as WPCLI is not invoked through the php-fpm sock/container. That batch PHP-FPM container is the only container used for the client dashboard logs.

This commit will print the WP-CLI output (both stdout and stderr) to the log stream of the wp-cron-runner container and thus they will be available in ELK.

However they will still not be shipped to customers unless Parker is modified to consume the wp-cron-runner log stream from K8s. This log stream is already not designed to be customer facing so would need to be filtered (only look at lines beginning with WP-CLI output).

Or -- we update our documentation that errors from WP CLI commands are not included in logs/log streams.

@rinatkhaziev
Copy link
Copy Markdown

@walthowd I'm not sure we want stdout in the logs? Seems very noisy potentially.

@walthowd
Copy link
Copy Markdown
Author

Followed up in Slack, the virtual pty gets you both but we can decouple if needed.

Log stream doesn't not automatically go into ELK, we transform, filter and enrich with vector and could filter these out if needed.

That would let Parker (via GOOP) consume them straight from K8s API if needed.

But the more I think about this, I think just update the docs to say not supported for WP-CLI commands -- as ideally any PHP error there is send to the user via the VIP-CLI stream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants