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.

Sign up for GitHub

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

Jump to bottom

Several small fixes and enhancements #115

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Several small fixes and enhancements #115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0ffd5ec
added variable config file location in header.php (via constant / env…
lopezio Apr 17, 2018
1bb665a
added pre tag to error display for easier debugging
lopezio Apr 17, 2018
de5860f
added a preliminary composer.json
lopezio Apr 17, 2018
63e2e96
update config searching logic to find apache environment variables to…
lopezio Apr 17, 2018
b16bbb4
fix urls for situations in which xhprof is symlinked in the document …
lopezio Apr 17, 2018
04ddf70
header.php refactoring: remove both & and ? from URI before redirecti…
lopezio Apr 19, 2018
6cf0d4f
change example 'servername' in sample config. the database column is …
lopezio Apr 19, 2018
13654e3
update README to reflect some changes
lopezio Apr 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions README.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,22 +53,28 @@ Installation

* Install your favourite mix of PHP and web server
* Install MySQL server
* Clone the project to some folder
* Clone the project to some folder of your choice.
* Map the sub folder `xhprof_html` to be accessible over HTTP
* Move `xhprof_lib/config.sample.php` to `xhprof_lib/config.php`
* You can do it with an Alias directive or (if symlinks are enabled) by just symlinking `xhprof_html` to any location within your document root.
* Copy `xhprof_lib/config.sample.php` to `xhprof_lib/config.php`
* Alternatively, you can copy the config.php to any place you like, and then specify the location of the config file in your ENV, in a PHP constant, or via Apache / Nginx Env variable (see below)
* Edit `xhprof_lib/config.php`
* Update the SQL server configuration
* Update the URL of the service (should point to `xhprof_html` over HTTP)
* Update the `dot_binary` configuration - otherwise no call graphs!
* Update the `controlIPs` variable to enable access.
* Update the SQL server configuration
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why these lines are indented differently now.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it looks more intuitive to me to put all the steps pertaining to editing the configuration under it...

* Update the URL of the service (should point to `xhprof_html` over HTTP)
* Update the `dot_binary` configuration - otherwise no call graphs!
* Update the `controlIPs` variable to enable access.
* For a development machine you can set this to `false` to disable IP checks.
* Import the DB schema (it is just 1 table)
* See the SQL at [xhprof_runs.php](https://github.com/toomasr/xhprof/blob/master/xhprof_lib/utils/xhprof_runs.php#L109)
* Add a PHP configuration to enable the profiling
* If using Apache you can edit your virtual host configuration
* Add `php_admin_value auto_prepend_file "/path/to/xhprof/external/header.php"`
* If using Apache you can edit your virtual host configuration
* Add `php_admin_value auto_prepend_file "/path/to/xhprof/external/header.php"`
* (optional) Add `SetEnv XHPROF_CONFIG /absolute/path/to/config.php` to your apache config to set location of config file for that host
* (optional) If you include the header.php manually, you can define the location of the config file via define('XHPROF_CONFIG','/absolute/path/to/config.php');
* (optional) Within a shell script, you can export `XHPROF_CONFIG=/absolute/path/to/config.php` to specify location of config file
* Visit http://your-server/xhprof/xhprof_html/ and be amazed!
* To get profiler information showing up there visit your page with a `GET` variable `_profile=1`.
* To get profiler information showing up there visit your page with a `GET` variable `_profile=1`.
This will enable it (via cookie) until you disable it by adding the parameter `_profile=0` to any url (or removing the _profile cookie manually)
* For example `http://localhost/?_profile=1`

We Are Working On
Expand Down
32 changes: 32 additions & 0 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"name": "xhprof/xhgui",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The project is all in one: PHP extension and gui. For that reason using xhprof/xhgui name won't be correct.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, please propose one, I'll add it to the changeset. The question is: If these changes are going to be taken into this current project, I'm perfectly fine with preinheimer/xhprof. Or anything. If not, I'll have to stick to my fork...

"description": "XHGUI is a GUI for the XHProf PHP extension, using a database backend, and pretty graphs to make it easy to use and interpret.",
"keywords": ["php", "profiling", "xhprof", "gui"],
"authors": [
{
"name": "Changhao Jiang",
"role": "Creator"
},
{
"name": "Kannan Muthukkaruppan",
"role": "Creator"
},
{
"name": "Venkat Venkataramani",
"role": "Creator"
},
{
"name": "George Cabrera",
"role": "Additional Contributor - UI Enhancements"
},
{
"name": "Paul Saab",
"role": "Additional Contributor - FreeBSD port"
},
{
"name": "Lorenzo Perone",
"role": "Additional Contributor "
}
],
"license": "Apache"
}
114 changes: 86 additions & 28 deletions external/header.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,32 @@
$_SERVER['REQUEST_URI'] = $_SERVER['SCRIPT_NAME'];
}

include(dirname(__FILE__) . '/../xhprof_lib/config.php');
// Search for config in different places - adding constant and env
if(defined('XHPROF_CONFIG') && is_file(XHPROF_CONFIG)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as for changes in xhprof_html/index.php file.

require_once XHPROF_CONFIG;
}
else {
$XHPROF_CONFIG = getenv('XHPROF_CONFIG');
if ( ! empty($XHPROF_CONFIG) && is_file($XHPROF_CONFIG)) {
require_once $XHPROF_CONFIG;
} elseif ( ! empty($_SERVER['XHPROF_CONFIG']) && is_file($_SERVER['XHPROF_CONFIG'])) {
require_once $_SERVER['XHPROF_CONFIG'];
} else {
require_once(XHPROF_LIB_ROOT . "/config.php");
}
}

function getExtensionName()
{
if (extension_loaded('tideways_xhprof'))
{
return 'tideways_xhprof';
}
if (extension_loaded('tideways'))
{
return 'tideways';
}elseif(extension_loaded('xhprof')) {
}
elseif(extension_loaded('xhprof')) {
return 'xhprof';
}
return false;
Expand Down Expand Up @@ -51,32 +69,72 @@ public static function __callstatic($name, $arguments)
}

// Only users from authorized IP addresses may control Profiling
if ($controlIPs === false || in_array($_SERVER['REMOTE_ADDR'], $controlIPs) || PHP_SAPI == 'cli')
{
/* Backwards Compatibility getparam check*/
if (!isset($_xhprof['getparam']))
{
$_xhprof['getparam'] = '_profile';
}

if (isset($_GET[$_xhprof['getparam']]))
{
//Give them a cookie to hold status, and redirect back to the same page
setcookie('_profile', $_GET[$_xhprof['getparam']]);
$newURI = str_replace(array($_xhprof['getparam'].'=1',$_xhprof['getparam'].'=0'), '', $_SERVER['REQUEST_URI']);
header("Location: $newURI");
exit;
}

if (isset($_COOKIE['_profile']) && $_COOKIE['_profile']
|| PHP_SAPI == 'cli' && ( (isset($_SERVER[$envVarName]) && $_SERVER[$envVarName])
|| (isset($_ENV[$envVarName]) && $_ENV[$envVarName])))
{
$_xhprof['display'] = true;
$_xhprof['doprofile'] = true;
$_xhprof['type'] = 1;
}
unset($envVarName);
if ($controlIPs === false || in_array($_SERVER['REMOTE_ADDR'], $controlIPs) || PHP_SAPI == 'cli') {

/* Backwards Compatibility getparam check*/
if ( ! isset($_xhprof['getparam'])) {
$_xhprof['getparam'] = '_profile';
}

if ( ! isset($_xhprof['displayparam'])) {
$_xhprof['displayparam'] = '_display';
}

$handleRuntimeToggle = function($key, $uri, $cookieName = null) {
if (isset($_GET[ $key ])) {
if (null === $cookieName) {
$cookieName = $key;
}
// Give them a cookie to hold status, and redirect back to the same page
if ($_GET[ $key ] === "1") {
setcookie($cookieName, $_GET[ $key ]);
} elseif ($_GET[ $key ] === "0") {
setcookie($cookieName, null, - 1);
unset($_COOKIE[ $cookieName ]);
}

$cleanURI = str_replace(array(
'&' . $key . '=1',
'&' . $key . '=0',
'?' . $key . '=1',
'?' . $key . '=0',
), '', $uri);

return [ true, $cleanURI ];
} else {
return [ false, $uri ];
}
};

$toggleParams = [ $_xhprof['getparam'], $_xhprof['displayparam'] ];

$currentURI = $_SERVER['REQUEST_URI'];
$changes = false;
foreach($toggleParams as $toggleParam) {
list($changed, $currentURI) = $handleRuntimeToggle($toggleParam, $currentURI);
$changes = ($changes or $changed);
}

if (isset($_COOKIE[ $_xhprof['getparam'] ]) && $_COOKIE[ $_xhprof['displayparam'] ]
|| PHP_SAPI == 'cli' && ((isset($_SERVER[ $envVarName ]) && $_SERVER[ $envVarName ])
|| (isset($_ENV[ $envVarName ]) && $_ENV[ $envVarName ]))) {
$_xhprof['doprofile'] = true;
$_xhprof['type'] = 1;
}

if (isset($_COOKIE[ $_xhprof['displayparam'] ]) && $_COOKIE[ $_xhprof['displayparam'] ]
|| PHP_SAPI == 'cli' && ((isset($_SERVER[ $envVarName ]) && $_SERVER[ $envVarName ])
|| (isset($_ENV[ $envVarName ]) && $_ENV[ $envVarName ]))) {
$_xhprof['display'] = true;
}

if (true === $changes) {
header('HTTP/1.1 302 Found');
header("Location: $currentURI");
die("Redirecting you to " . $currentURI);
}

unset($envVarName);
}


Expand Down
17 changes: 16 additions & 1 deletion xhprof_html/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,22 @@
if (!defined('XHPROF_LIB_ROOT')) {
define('XHPROF_LIB_ROOT', dirname(dirname(__FILE__)) . '/xhprof_lib');
}
require_once (XHPROF_LIB_ROOT . "/config.php");

// Search for config in different places - adding constant and env
if(defined('XHPROF_CONFIG') && is_file(XHPROF_CONFIG)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for checking in all these places (also prefixing environment variables with ENV_ sounds like too much). Won't just 2 places do:

  1. do the getenv('XHPROF_CONFIG_PATH')
  2. if non-empty and file exists, then use it
  3. otherwise use XHPROF_LIB_ROOT . "/config.php"
  4. require_once resulting file path

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getenv() only finds the env variables on command line, but (at least in my environment) not those set in the apache config (SetEnv...). Both cases might be useful, depending on command line / webserver usage. Agree on ENV_, adding to change list..

require_once XHPROF_CONFIG;
}
else {
$XHPROF_CONFIG = getenv('XHPROF_CONFIG');
if ( ! empty($XHPROF_CONFIG) && is_file($XHPROF_CONFIG)) {
require_once $XHPROF_CONFIG;
} elseif ( ! empty($_SERVER['XHPROF_CONFIG']) && is_file($_SERVER['XHPROF_CONFIG'])) {
require_once $_SERVER['XHPROF_CONFIG'];
} else {
require_once(XHPROF_LIB_ROOT . "/config.php");
}
}

include_once XHPROF_LIB_ROOT . '/display/xhprof.php';
include (XHPROF_LIB_ROOT . "/utils/common.php");

Expand Down
3 changes: 2 additions & 1 deletion xhprof_lib/config.sample.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
$_xhprof['dbpass'] = 'password';
$_xhprof['dbname'] = 'xhprof';
$_xhprof['dbadapter'] = 'Pdo';
$_xhprof['servername'] = 'myserver';
$_xhprof['servername'] = 's01';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this?

Copy link
Author

@lopezio lopezio Apr 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because the table structure has a CHAR(3) in the current version, and when I first copied these settings, they ended up breaking all INSERTs because "myserver" is 8 chars long. I thought it would be more intuitive to have an example with 3 chars, so no need to change table structure, but it doesn't fail on the first try.
There's another PR in the pipeline that I saw, that would rather change the table structure, but I thought that this change is less "destructive" for current users...

$_xhprof['namespace'] = 'myapp';
$_xhprof['url'] = 'http://url/to/xhprof/xhprof_html';
$_xhprof['getparam'] = "_profile";
$_xhprof['displayparam'] = "_display";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Is this a new parameter or an old one, that wasn't mentioned.
  2. Would it work correctly for people with config without this parameter?
  3. If this is a new parameter, then why it's needed?

Copy link
Author

@lopezio lopezio Apr 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. A new one, but actually referring to an already present functionality ($_xhprof['display'])
  2. Yes, as with "getparam", a default is set when it is not found (see header.php, line 79).
  3. I thought it would be a nice thing to be able to switch the "display" functionality (which mainly displays a link to the xhprof page for the current run) on/off at runtime, via a parameter - exactly like for the _profile toggle. I like to have the link, but some pages just won't work with it, paricularly JSON output or templates generated for frontend frameworks.
    I added the parameter to be "consistent" with the current way of handling the other runtime functionality, which is the "getparam"/profile.


/*
* MySQL/MySQLi/PDO ONLY
Expand Down
4 changes: 2 additions & 2 deletions xhprof_lib/display/xhprof.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@
* Our coding convention disallows relative paths in hrefs.
* Get the base URL path from the SCRIPT_NAME.
*/
$base_path = rtrim(dirname($_SERVER['SCRIPT_NAME']), "/");

$base_path = $_xhprof['url'];

/**
* Generate references to required stylesheets & javascript.
Expand Down Expand Up @@ -791,6 +790,7 @@ function print_flat_data($url_params, $title, $flat_data, $sort, $run1, $run2, $
global $sortable_columns;
global $vwbar;
global $base_path;
global $_xhprof;

$size = count($flat_data);
if (!$limit) { // no limit
Expand Down
2 changes: 1 addition & 1 deletion xhprof_lib/templates/profTable.phtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

<tr>
<td style="background-color: <?php echo $color; ?>;"><span style="display: none"><?php echo $color; ?></span>&nbsp;</td>
<td><a href="index.php?run=<?php echo $run1; ?>&amp;symbol=<?php echo urlencode($element['fn']); ?>"><?php echo htmlentities($element['fn'], ENT_QUOTES, 'UTF-8'); ?></a></td>
<td><a href="<?php echo $_xhprof['url']; ?>/index.php?run=<?php echo $run1; ?>&amp;symbol=<?php echo urlencode($element['fn']); ?>"><?php echo htmlentities($element['fn'], ENT_QUOTES, 'UTF-8'); ?></a></td>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which scenarios without this change something breaks? I have xhprof installed in a sub-folder of DocumentRoot and all works perfectly fine already.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, this is one of the situations where not having the whole xhprof code in the document root breaks it (but, for example, only Alias as suggested in the original version, or a symlink of xhprof_html). Putting in this not only fixes xhprof for that case, but potentially also enables the use of a dedicated virtual host for it (although I haven't tested the latter yet), which might be very useful when comparing, for example, the results of different development hosts.

<td><?php echo isset($element['ct'])?$element['ct']:''; ?></td>
<td><?php echo isset($element['wt'])?$element['wt']:''; ?></td>
<td><?php echo isset($element['cpu'])?$element['cpu']:''; ?></td>
Expand Down
2 changes: 1 addition & 1 deletion xhprof_lib/utils/xhprof_runs.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ public function save_run($xhprof_data, $type, $run_id = null, $xhprof_details =
global $_xhprof;
if ($_xhprof['display'] === true)
{
echo "Failed to insert: $query <br>\n";
echo "Failed to insert: <pre>$query</pre><br>\n";
}
return -1;
}
Expand Down