Is it just me? I have the xhprof_disable_admin_paths variable's checkbox checked, and yet XHProf still generates a profile log file and still shows "XHProf output" at the bottom of every screen, admin included. I have hardcoded a return FALSE in xhprof_is_enabled(), and that does silence the thing.

CommentFileSizeAuthor
#8 interdiff-2631144.txt421 bytesandypost
#4 2631144-administrative-paths-admin-menu-4.patch1.46 KBthePanz
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

texas-bronius created an issue. See original summary.

Elijah Lynn’s picture

Elijah Lynn
Location Portland, Oregon
CreditAttribution: Elijah Lynn at NBCUniversal commented

Same here. Doing a quick debug it looks like it may be honoring it on the first request. But then admin_menu does an XHR and it get's generated for that request. This is because it checks arg(0) == 'admin...

if (arg(0) == 'admin' && variable_get('xhprof_disable_admin_paths', TRUE)) {
  $enabled = FALSE;
}

This maybe needs to be updated to have a checkbox to also ignore admin menu requests too?

texas-bronius’s picture

texas-bronius CreditAttribution: texas-bronius commented

I see what you're saying and can corroborate your theory: When I disabled the admin_menu module, the superfluous XHProf link at the bottom of the generated page no longer shows on /admin pages.

thePanz’s picture

thePanz CreditAttribution: thePanz at Liip for FREITAG lab. AG commented
Title: XHProf doesn't seem to be ignoring administrative paths as preference is set in admin settings » XHProf doesn't seem to be ignoring administrative paths if admin_menu is enabled
Status: Active » Needs review
FileSize
2631144-administrative-paths-admin-menu-4.patch1.46 KB

Patch attached, new setting (enabled by default) to avoid profiling admin_menu XHR calls.

Elijah Lynn’s picture

Elijah Lynn
Location Portland, Oregon
CreditAttribution: Elijah Lynn at NBCUniversal commented

Quick review and it looks good @thePanz! Didn't test yet so I won't RTBC yet but just wanted to leave that feedback.

jstoller’s picture

jstoller
he/him
CreditAttribution: jstoller as a volunteer and at California Science Center commented
Status: Needs review » Reviewed & tested by the community

Works great! Thanks!

osopolar’s picture

osopolar
Primary language German
Location 🇩🇪 GER 🌐
CreditAttribution: osopolar commented

Works for me too. Thanks thePanz.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
FileSize
interdiff-2631144.txt421 bytes

On commit I've added uninstall of variable after #2892127: Configuration not deleted on uninstall

  • andypost committed 856456d on 7.x-1.x authored by thePanz 
    Issue #2631144 by thePanz, andypost: XHProf doesn't seem to be ignoring...
andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.