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 xdebug support. #31

Closed
wants to merge 1 commit into from
Closed

Conversation

micaherne
Copy link

@micaherne micaherne commented Aug 16, 2018

Here's a patch for xdebug support (#17). It installs the module but leaves it disabled. The module will be enabled if there is a non-empty XDEBUG_CONFIG environment variable.

I thought it would be better to use the existing XDEBUG_CONFIG environment variable rather than introduce a new one, as realistically you're likely to want to configure xdebug if you enable it. In the event that someone wants it to run with all the default settings - which will probably be rare - you can just pass one of the default settings to enable it (e.g. XDEBUG_CONFIG="default_enable=1")

@scara
Copy link
Contributor

scara commented Aug 16, 2018

Hi @micaherne,
TNX for providing a proposal based on XDEBUG_CONFIG 👍.
Find below my per review:

  1. INI_FILE should be IMHO renamed into XDEBUG_INI_FILE. I'm not sure if we could always load the extension, given everything disabled, without any perf hit (http://web.performancerasta.com/php%E2%80%99s-xdebug-tracing-overhead-in-production/): this would eventually remove the need for a custom docker entrypoint
  2. It would be helpful to add more to the Xdebug ini file to highlight some key points of its configuration, even the profiling (https://docs.moodle.org/dev/Profiling_PHP) and the coverage. Everything should be disabled by default
  3. You should document the new option into the README here and in the one of https://github.com/moodlehq/moodle-docker i.e. export XDEBUG_CONFIG and its Windows counterpart SET XDEBUG_CONFIG, since you can override any default setting using that ENV var before starting the composition, given that you properly pass that var to the container. In the PR to https://github.com/moodlehq/moodle-docker you should both change README and the composition (up to you to add an optional VOLUME to expose the profiling data too).

TIA,
Matteo

@micaherne micaherne force-pushed the xdebug-support branch 2 times, most recently from 3fe3715 to 48201fe Compare August 17, 2018 07:57
@micaherne
Copy link
Author

Hi Matteo, thanks for the quick peer review, it's very much appreciated.

  1. I've renamed the variable as suggested. I totally agree that it would be good if we could simply load Xdebug with all features disabled, but we'd need to be certain there would be no performance hit. It might be better to leave that for the future.

  2. I think I'm missing the point of this, so please bear with me! I wasn't anticipating anyone would see, or interact in any way, with the Xdebug ini file, so I'm not sure what you mean by "highlight some key points of its configuration". Can you describe the scenario you're thinking of?

  3. I've updated the README with a short section about enabling Xdebug. I'm not sure when a user would set the environment variable in the shell - I was expecting it would either be passed as a parameter to the run command, or in the environment section if you were using docker-compose. If I'm missing a scenario can you let me know and I'll add it to the README.

I'll see about getting a pull request into the moodle-docker project too.

Thanks again, Michael.

@scara
Copy link
Contributor

scara commented Aug 19, 2018

Hi @micaherne,
apologies for my late reply.
TNX for taking my points!
About point (2), I mean something like the lines below:

$ diff -u moodle-php-entrypoint moodle-php-entrypoint.new
--- moodle-php-entrypoint       2018-08-19 16:12:32.512788500 +0200
+++ moodle-php-entrypoint.new   2018-08-19 16:15:16.003172500 +0200
@@ -5,7 +5,25 @@
     rm -f $XDEBUG_INI_FILE
 else
     if [ ! -e $XDEBUG_INI_FILE ]; then
+        # Enable the extension.
         echo "zend_extension=$(find /usr/local/lib/php/extensions/ -name xdebug.so)" > $XDEBUG_INI_FILE
+        # Configure the main settings, keeping features disabled.
+        cat <<EOF >> $XDEBUG_INI_FILE
+# Code coverage.
+xdebug.coverage_enable = 0
+# Stack trace.
+xdebug.default_enable = 0
+# Remote debug.
+xdebug.remote_enable = 0
+xdebug.remote_host = localhost
+xdebug.remote_port = 9000
+xdebug.remote_autostart = 1
+# Profiling: https://docs.moodle.org/dev/Profiling_PHP#Quick_summary
+xdebug.profiler_enable = 0
+xdebug.profiler_enable_trigger = 0
+xdebug.profiler_output_dir = /tmp
+xdebug.profiler_output_name = cachegrind.out.%R.%u
+EOF
     fi
 fi

This will IMHO help people when deciding what they should provide via XDEBUG_CONFIG: kind of quick help if they'll look at the github repo files - maybe, it would be better something more into the README -; otherwise, how do you expect people to use that env var compared to the different use cases when xdebug would be helpful (e.g. debug) o required (e.g. phpunit coverage)?

TIA,
Matteo

@micaherne
Copy link
Author

Thanks again, Matteo! I've added your suggested patch.

I see what you're getting at now. The way I was looking at it was that people who want to use Xdebug already know what they're doing, and they can work out what settings they need to set from the documentation, but I guess it's useful to signpost some of the more-used config settings for people who are just getting started with Xdebug too.

Cheers, Michael

@Kathrin84
Copy link

Kathrin84 commented Jan 8, 2019

Hi,

thank you very much for this patch!

Recently, I've tried to get XDebug working with this patch together with Moodle Docker and PHPStorm. However, this patch as is unfortunately did not work out for me and it took me quite a while to find the solution.

My solution to get this working (I'm operating on a Mac):

  • Change the setting xdebug.remote_host to "host.docker.internal":
    xdebug.remote_host = host.docker.internal
  • Do NOT SET the environment variable: remote_connect_back=1

I don't know if this also works on Windows? If it does, it would be great to change the patch one more to address this fix. And moreover, it would be really appreciated if this could then be exported to the Docker Hub! (Currently I needed to clone this repo locally and build the image in my Moodle Docker container manually.)

Futhermore, I've never used this code mentioned in the readme:
$ docker run --name web0 -p 8080:80 -v $PWD:/var/www/html -e XDEBUG_CONFIG="remote_enable=1 remote_connect_back=1" moodlehq/moodle-php-apache:7.1

Best, Kathrin

@caperneoignis
Copy link

@Kathrin84 That should also work with windows because you are essientally setting up xdebug to work over a remote connection. The ideal way is to leave xdebug.remote_host as localhost and then connect using ssh proxying as listed here but not sure how that would work in a dockerized setup since the host machine can't reference the container by name. But figured someone may find the information helpful if running docker on a remote machine and want's to connect using phpstorm.

@scara
Copy link
Contributor

scara commented Jan 29, 2019

Hello Everyone,
host.docker.internal is a nice feature "limited" to both Docker for Mac and Docker for Windows.
In the past, before 18.03, aliasing the network interface was the trick for Mac e.g. https://www.arroyolabs.com/2016/10/docker-xdebug/ .

It would be nice to provide these examples in the README too since the configuration of xdebug actually depends on how the user plays w/ her/his dev env e.g. locally on the same machine hosting the IDE (i.e. it depends on the OS) or in a different remote machine (i.e. it depends on the network setup).

The key point is that both the machine hosting the IDE and the machine hosting docker running this container should connect each other, the first to consume web requests served by the container over HTTP and the latter to connect back to the machine hosting the IDE to "remotely" debug the application via the xdebug extension.
xdebug.remote_connect_back is a sensible setting since it implies to work with visibility for both networks (the one of the machine hosting the IDE and the other of the machine hosting the container) which cannot always be true when working w/ moodle-docker in a different host (unless, guessing, you change the network setup in the yml file to be a host one): it's worth noting that if enabled, the xdebug.remote_host setting is ignored.

TNX @Kathrin84 for raising your issue w/ Mac 👍 .
At the time of the first review I didn't catch it as well as eventually on (Docker for) Windows.

So we need to improve the patch to highlight the settings for "common" local envs:

  1. Linux: xdebug.remote_host=localhost or xdebug.remote_connect_back=1, aliasing, <whatever compatible w/ the docker host network setup>
  2. Docker for Mac: xdebug.remote_host=host.docker.internal
  3. Docker for Windows: xdebug.remote_host=host.docker.internal

HTH,
Matteo

@dmonllao
Copy link
Contributor

dmonllao commented Apr 8, 2019

Looking forward to have xdebug support in moodle-docker. Thanks all.

@stronk7
Copy link
Member

stronk7 commented Aug 6, 2023

I'm closing this now, because as of #173, xdebug is now installed (disabled by default) in all the >= php74 images. Plus readme tells how it should be enabled on container creation.

Ciao :-)

@stronk7 stronk7 closed this Aug 6, 2023
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.

7 participants