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 logger API #1113

Merged
merged 9 commits into from
Mar 20, 2024
Merged

Add logger API #1113

merged 9 commits into from
Mar 20, 2024

Conversation

bgrgicak
Copy link
Collaborator

@bgrgicak bgrgicak commented Mar 18, 2024

What is this PR doing?

This PR adds a logger API endpoint to Playground.WordPress.net.

What problem is it solving?

It creates a standard place where users can send crash reports.

How is the problem addressed?

A PHP script accepts the log message, validates the format, and sends the log message to a Making WordPress Slack channel.

Testing Instructions

curl --location 'http://localhost:10013/logger.php' \
--form 'message="What happened?

Ut consequuntur esse sint sequi sapiente corrupti. Suscipit est facere voluptatem omnis earum necessitatibus molestiae. Exercitationem odio id ut alias voluptatum.

Dolor ut dolores labore hic at. Aspernatur provident voluptatem dignissimos et aut minima et aut. Eaque odio aut consequatur quam quidem.

Logs

[02-Jan-2024 08:08:06 UTC] PHP-WASM Fatal: null function or function signature mismatch Error: null function or function signature mismatch
    at #handleRequest (http://localhost:5400/@fs/Users/bero/Projects/wordpress-playground/packages/php-wasm/universal/src/lib/base-php.ts?t=1710835676492:518:24)
    at async WebPHP.run (http://localhost:5400/@fs/Users/bero/Projects/wordpress-playground/packages/php-wasm/universal/src/lib/base-php.ts?t=1710835676492:211:24)
    at async #dispatchToPHP (http://localhost:5400/@fs/Users/bero/Projects/wordpress-playground/packages/php-wasm/universal/src/lib/php-request-handler.ts?t=1710835676492:178:14)
    at async PHPRequestHandler.request (http://localhost:5400/@fs/Users/bero/Projects/wordpress-playground/packages/php-wasm/universal/src/lib/php-request-handler.ts?t=1710835676492:84:14)
    at async PHPBrowser.request (http://localhost:5400/@fs/Users/bero/Projects/wordpress-playground/packages/php-wasm/universal/src/lib/php-browser.ts:32:22)
[19-Mar-2024 08:08:06 UTC] PHP-WASM Fatal: null function or function signature mismatch Error: null function or function signature mismatch
    at #handleRequest (http://localhost:5400/@fs/Users/bero/Projects/wordpress-playground/packages/php-wasm/universal/src/lib/base-php.ts?t=1710835676492:518:24)
    at async WebPHP.run (http://localhost:5400/@fs/Users/bero/Projects/wordpress-playground/packages/php-wasm/universal/src/lib/base-php.ts?t=1710835676492:211:24)
    at async #dispatchToPHP (http://localhost:5400/@fs/Users/bero/Projects/wordpress-playground/packages/php-wasm/universal/src/lib/php-request-handler.ts?t=1710835676492:178:14)
    at async PHPRequestHandler.request (http://localhost:5400/@fs/Users/bero/Projects/wordpress-playground/packages/php-wasm/universal/src/lib/php-request-handler.ts?t=1710835676492:84:14)
    at async PHPBrowser.request (http://localhost:5400/@fs/Users/bero/Projects/wordpress-playground/packages/php-wasm/universal/src/lib/php-browser.ts:32:22)

Url

http://localhost:5400/website-server/?php=8.0&wp=latest&storage=none&networking=yes#%7B%20%22landingPage%22:%20%22/wp-admin/%22,%20%22features%22:%20%7B%20%22networking%22:%20true%20%7D,%20%22steps%22:%20%5B%20%7B%20%22step%22:%20%22login%22%20%7D,%20%7B%20%22step%22:%20%22writeFile%22,%20%22path%22:%20%22/wordpress/wp-content/mu-plugins/rewrite.php%22,%20%22data%22:%20%22%3C?php%20add_action(%20'\''shutdown'\'',%20function()%20%7B%20post_message_to_js('\''test'\'');%20%7D%20);%22%20%7D%20%5D%20%7D"'

@bgrgicak bgrgicak self-assigned this Mar 18, 2024
@bgrgicak bgrgicak marked this pull request as ready for review March 18, 2024 12:03
@bgrgicak bgrgicak marked this pull request as draft March 18, 2024 12:03
@bgrgicak bgrgicak marked this pull request as ready for review March 19, 2024 10:11
@bgrgicak bgrgicak requested a review from adamziel March 19, 2024 10:11
@adamziel
Copy link
Collaborator

This is cool, thank you @bgrgicak! The only thing I'd adjust before merging is disabling PHP error messages in case someone found a way to expose secrets by trigerring a warning or so.

@brandonpayton
Copy link
Member

brandonpayton commented Mar 19, 2024

Should we add any sort of rate-limiting for specific IPs to help avoid people abusing the logging endpoint and causing DoS due to hitting Slack API limits?

@brandonpayton
Copy link
Member

Should we add any sort of rate-limiting for specific IPs to help avoid people abusing the logging endpoint and causing DoS due to hitting Slack API limits?

Perhaps there is already some kind of limiting at the web server as well.

@adamziel
Copy link
Collaborator

Perhaps there is already some kind of limiting at the web server as well.

It's not designed to protect against that use-case so rate-limiting sounds like a good idea 👍 I wonder how is it implemented in other WP.org endpoints forwarding data to Slack.

@bgrgicak
Copy link
Collaborator Author

Should we add any sort of rate-limiting for specific IPs to help avoid people abusing the logging endpoint and causing DoS due to hitting Slack API limits?

Sounds like a good idea. I need to check first what we can use on the server for this. Usually storing IPs and counts in Memcache worked well for me.

@bgrgicak
Copy link
Collaborator Author

I took a quick look and it seems like the server doesn't have Memcache. The only alternative I can think of is storing counts in a file or db. It would slow down the request, but I don't expect a lot so we should be ok.

@adamziel do you have any suggestions? What does the server support and can we install things on it?

@adamziel
Copy link
Collaborator

adamziel commented Mar 20, 2024

cc @dd32 for comments – I wonder how WP.org services approach this problem.

@bgrgicak
Copy link
Collaborator Author

@adamziel @brandonpayton would it be ok for me to merge this as-is?
I would like to finish the modal this week and ensure it works with the logger API.

I would add the rate-limiting once we decide on how to do it.

Alternatively I could add a simple limiter to this PR and update it later if we find a better solution.

@adamziel
Copy link
Collaborator

Let's do that @bgrgicak and keep the discussion going – perhaps in a separate issue?

@adamziel adamziel merged commit 8aedfa8 into trunk Mar 20, 2024
5 checks passed
@adamziel adamziel deleted the add/logger-api branch March 20, 2024 12:10
@brandonpayton
Copy link
Member

would it be ok for me to merge this as-is?

Let's do that @bgrgicak and keep the discussion going – perhaps in a separate issue?

👍 That sounds good.

@dd32
Copy link
Member

dd32 commented Mar 21, 2024

I wonder how WP.org services approach this problem.

FWIW WordPress.org uses Memcache for it.

You could potentially use apcu but I don't know what's enabled/installed on the WordPress.net system.

@brandonpayton
Copy link
Member

Thanks, @dd32 🙇

@bgrgicak
Copy link
Collaborator Author

You could potentially use apcu but I don't know what's enabled/installed on the WordPress.net system.

It's not enabled on the server, but it seems like a good solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants