newrelic RUM #603

Closed
szepeviktor opened this Issue Jan 24, 2014 · 36 comments

Comments

Projects
None yet
10 participants

Please consider turning off RUM in xsl and xml files:

https://docs.newrelic.com/docs/php/the-php-api#api-rum-disable

if (extension_loaded('newrelic')) {}

Thank you!

Contributor

jrfnl commented Jan 26, 2014

Why ? What's this supposed to do and what is it breaking ?

newrelic does not look to be an official php extension.

New Relic monitoring is very popular. https://newrelic.com/
They have their own PHP ext. https://docs.newrelic.com/docs/releases/php

New Relic's realtime user monitoring inserts two script elements in every document.

  • to set start time
  • to report the measurement to New Relic

e.g. http://xn--eskvi-videk-zeb3ip0d.hu/
Look for "NREUM" in the page's source.

The Yoast SEO plugin is also very popular. It generates xsl and xml files.
In case of these two files RUM should be disabled.

if (extension_loaded('newrelic')) {
    newrelic_disable_autorum();
}

Thank you!

Contributor

jrfnl commented Feb 14, 2014

Just checking: you say two RUM files should be disabled. Does the one command in your sample code do this or is more needed to get this fixed ?

I mean Real User Monitoring should be disabled when generating xsl and xml files.
This code should stop newrelic daemon from injecting js code:

if (extension_loaded('newrelic')) {
    newrelic_disable_autorum();
}

@szepeviktor @jrfnl Adding the above code to xml-sitemap-xsl.php allows the sitemap to be generated but preserves New Relic monitoring for the rest of WP SEO.

@szepeviktor Great comment.

The temporarily fix is:

1. add this to .htaccess
php_flag newrelic.browser_monitoring.auto_instrument off
2. insert this after </title>
<?php if (extension_loaded('newrelic') && !is_user_logged_in()) { echo newrelic_get_browser_timing_header(); } ?>
3. insert this just before </body>
<?php if (extension_loaded('newrelic') && !is_user_logged_in()) { echo newrelic_get_browser_timing_footer(); } ?>

@szepeviktor Your suggested fix isn't universal enough because not everyone uses .htaccess. Nginx doesn't use .htaccess, for example, and there are plenty of WP SEO users running Nginx. Second, it would also alter what New Relic monitors in a way that some New Relic users may not want by not running New Relic for users that aren't logged in. Since the only conflict that I know between New Relic and WP SEO involves the sitemap, a simpler fix is to use your original code here #603 (comment) and add it to the top of xml-sitemap-xsl.php.

We've tested that on 1.4.24 and 1.5 and it works perfectly by not allowing the XSL to get mangled.

You could use php ini_set() to set php flags and freely leave out && !is_user_logged_in()
I just do not use

  • ClickTale
  • Google Analytics
  • NewRelic etc.
    on editors and admins. You could filter users by their roles.
Owner

jdevalk commented Feb 22, 2014

I'd prefer actually pushing the fix into our plugin, saves a lot of support for people who don't know what's hitting them...

jdevalk reopened this Feb 22, 2014

Agree 100%.

Just put the code below at the top of xml-sitemap-xsl.php:

if (extension_loaded('newrelic')) {
    newrelic_disable_autorum();
}

We use New Relic and can confirm that works on 1.4.24 and 1.5 beta.

hm, I manually put the code in xml-sitemap-xsl.php, but the javascript snippet is still there and produces that error :(

Running on Multisite with Wordpress SEO Version 1.4.25 and W3 Total Cache

We're running 1.4.25 and manually applied this fix and everything is fine. A few things to check:

  • make sure you put the code at the top of the file, just after the few lines of comments at the top but after the opening tag
  • since you are running W3TC, you have to clear all of your caches
  • if that still doesn't work, try deactivating W3TC temporarily to see if that isolates the problem to a caching problem
  • and if that doesn't work open a support ticket with New Relic and ask them why that code snippet isn't working on your account
Contributor

jrfnl commented Feb 27, 2014

@ACiDC0re The code from @szepeviktor original suggestion will be included in WPSEO v1.5 so hopefully that'll help.

If you like you can test v1.5 already. You can download the branch from here. Do make a good backup of your settings/database before testing and don't test in a live/production environment.

hm, I disabled W3 Total Cache and it works, but with W3TC enabled it doesn't :/ Even after emptying the caches :(

Owner

jdevalk commented Feb 27, 2014

Did you hard refresh your browser cache too?—

On Thu, Feb 27, 2014 at 4:17 PM, ACiDC0re notifications@github.com
wrote:

hm, I disabled W3 Total Cache and it works, but with W3TC enabled it doesn't :/ Even after emptying the caches :(

Reply to this email directly or view it on GitHub:
#603 (comment)

yes, btw, I'm running nginx.... puh, I tried everything in the settings of W3TC, but only disabling W3TC disables the javascripts from newrelic ... I have no idea why :(

And the weired thing is if I directly go to the xsl file (wp-content/plugins/wordpress-seo/css/xml-sitemap.xsl there is no newrelic javascripts :/

I enabled page cache debugging and it says caching disabled in the source code of the main-sitemap.xsl file??

Owner

jdevalk commented Feb 27, 2014

Did you clear the cache correctly from W3TC?

hm? think so.

so, i disabled every cache + CDN and it worked. And enabling only minify breaks it again. But if I then disable every setting under minify tab doesn't "repair" it again, only disabling minify completely.

Owner

jdevalk commented Feb 27, 2014

If you re-enable minify, did you then also empty the minify cache?

yup, and it shows the error again

this is my w3tc config: https://www.dropbox.com/s/fozycncr9qa5xza/w3tc-settings-2.php

//edit: If I restore the default settings and enable only NewRelic and BrowserCache, the NewRelic Javascript is in the .xsl file. If I then disable the browser Cache, the error is gone and so is the javascript from NewRelic.

This solution is only for the newrelic daemon/apache mod. Not the W3TC php solution.

To be clear, we're running Nginx, not Apache, and the solution quoted by me works fine on Nginx. I think @ACiDC0re's issue is more connected to W3TC, not Apache vs Nginx. We're also running an object cache, memcached, plus Nginx's page caching, and everything works fine with the fix added as discussed above.

@wpperform Do you have a newrelic daemon or do you use W3TC/New Relic monitoring?

@ACiDC0re You can contact me viktor@szepe.net
I'll fix your site.

I see you have correct xml but main-sitemap.xsl (.*-sitemap.xsl) is not excluded from New Relic.

@szepeviktor We have the daemon. We don't use W3TC.

@jdevalk In versions 1.4.21 and earlier, this code was not necessary. Whatever changes were made to WP SEO from .22 onward (including up to the .5 beta) require this modification to stop New Relic from monitoring. After .24 came out, we tried .24 but didn't identify the problem, so reverted back to .21 and sitemaps worked fine. That means it is not strictly a New Relic issue, since it appears that changes to WP SEO triggered the need for this. We posted another issue here as those versions came out about non-working sitemaps (example: #607) but it never got any traction. It was not until @szepeviktor connected this to New Relic that we established the fix worked for .22 through the .5 beta. While you've committed this for 1.5, perhaps you can identify what changed between .21 and .22 here and avoid adding this altogether.

Owner

jdevalk commented Feb 28, 2014

What changed is that we generate the XSL through WP instead of having a hard coded file, which allows for more flexibility and a URL path off the root instead of 5 dirs deep. The latter gave issues with security plugins, the former allows us to make the XSL more flexible in the future, so I'm not really willing to revert.—
Joost de Valk
Founder & CEO Yoast.com

On Fri, Feb 28, 2014 at 4:25 PM, wpperform notifications@github.com
wrote:

@szepeviktor We have the daemon. We don't use W3TC.

@jdevalk In versions 1.4.21 and earlier, this code was not necessary. Whatever changes were made to WP SEO from .22 onward (including up to the .5 beta) require this modification to stop New Relic from monitoring. After .24 came out, we tried .24 but didn't identify the problem, so reverted back to .21 and sitemaps worked fine. That means it is not strictly a New Relic issue, since it appears that changes to WP SEO triggered the need for this. We posted another issue here as those versions came out about non-working sitemaps (example: #607) but it never got any traction. It was not until @szepeviktor connected this to New Relic that we established the fix worked for .22 through the .5 beta. While you've committed this for 1.5, perhaps you can identify what changed between .21 and .22 here and avoid adding this altogether.

Reply to this email directly or view it on GitHub:
#603 (comment)

Understandable. Generating the XSL through WP means PHP, and that’s where New Relic kicks in; hence the requirement for the fix you added to 1.5. We’re glad you added it and appreciate your doing so, but now that we know the cause, it’s a non issue for us, but it appears there still could be issues for those who use New Relic through W3TC.

From: Joost de Valk [mailto:notifications@github.com]
Sent: Friday, February 28, 2014 10:34 AM
To: Yoast/wordpress-seo
Cc: wpperform
Subject: Re: [wordpress-seo] newrelic RUM (#603)

What changed is that we generate the XSL through WP instead of having a hard coded file, which allows for more flexibility and a URL path off the root instead of 5 dirs deep. The latter gave issues with security plugins, the former allows us to make the XSL more flexible in the future, so I'm not really willing to revert.—
Joost de Valk
Founder & CEO Yoast.com

On Fri, Feb 28, 2014 at 4:25 PM, wpperform <notifications@github.com mailto:notifications@github.com >
wrote:

@szepeviktor We have the daemon. We don't use W3TC.

@jdevalk In versions 1.4.21 and earlier, this code was not necessary. Whatever changes were made to WP SEO from .22 onward (including up to the .5 beta) require this modification to stop New Relic from monitoring. After .24 came out, we tried .24 but didn't identify the problem, so reverted back to .21 and sitemaps worked fine. That means it is not strictly a New Relic issue, since it appears that changes to WP SEO triggered the need for this. We posted another issue here as those versions came out about non-working sitemaps (example: #607) but it never got any traction. It was not until @szepeviktor connected this to New Relic that we established the fix worked for .22 through the .5 beta. While you've committed this for 1.5, perhaps you can identify what changed between .21 and .22 here and avoid adding this altogether.

Reply to this email directly or view it on GitHub:
#603 (comment)


Reply to this email directly or view it on GitHub #603 (comment) . https://github.com/notifications/beacon/2600727__eyJzY29wZSI6Ik5ld3NpZXM6QmVhY29uIiwiZXhwaXJlcyI6MTcwOTEzNDQxOCwiZGF0YSI6eyJpZCI6MjQxOTQzMTh9fQ==--06ff48ec64dc683daef8db95502f80ea984dbb37.gif

Thanks for handling this! New Relic is incredibly useful for sites looking to optimize performance, but it does assume that every page is serving HTML. I was about to submit a PR for this, but happy to see it's already done.

calevans commented Mar 6, 2014

Just pulled 1.5 down and put it on one of my blogs that has New Relic. I can confirm that it works.

Thanks everyone for getting this fixed.

Cheers!
=C=

jdevalk closed this in afa1d9c Mar 11, 2014

zombeej commented Mar 11, 2014

Hi, just a bit of follow up here. Thanks much for the fix. I discovered that RUM was causing the broken XSL file and reported it to NewRelic about a week or so ago. I've informed them of this fix and they informed me that they're actually working on making sure they don't inject the RUM JavaScript into XSL/XML files.

From NewRelic support:

"Thanks for the follow-up. It's great that they have already implemented a solution. We'd definitely like to avoid causing the issue as well, so I'll follow up with our senior support engineers and the development team and see if we can understand what the issue is on our side, since we shouldn't be injecting into XML/XSL."

yazeed commented Mar 26, 2014

Still bugged with nginx, using latest version.

XML Parsing Error: not well-formed
Location: http://*.com/main-sitemap.xsl
Line Number 10, Column 277: <script type="text/javascript">window.NREUM||(NREUM={}),__nr_require=function a(b,c,d){function e(f){if(!c[f]){var g=c[f]={exports:{}};b[f][0].call(g.exports,function(a){var c=b[f][1][a];return e(c?c:a)},g,g.exports,a,b,c,d)}return c[f].exports}for(var f=0;f<d.length;f++)e(d[f]);return e}({"4O2Y62":[function(a,b){function c(a,b){var c=d[a];return c?c.apply(this,b):(e[a]||(e[a]=[]),void e[a].push(b))}var d={},e={};b.exports=c,c.queues=e,c.handlers=d},{}],handle:[function(a,b){b.exports=a("4O2Y62")},{}],YLUGVp:[function(a,b){function c(){var a=m.info=NREUM.info;if(a&&a.agent&&a.licenseKey&&a.applicationID){m.proto="https"===l.split(":")[0]||a.sslForHttp?"https://":"http://",g("mark",["onload",f()]);var b=i.createElement("script");b.src=m.proto+a.agent,i.body.appendChild(b)}}function d(){"complete"===i.readyState&&e()}function e(){g("mark",["domContent",f()])}function f(){return(new Date).getTime()}var g=a("handle"),h=window,i=h.document,j="addEventListener",k="attachEvent",l=(""+location).split("?")[0],m=b.exports={offset:f(),origin:l,features:[]};i[j]?(ij,hj):(ik,hk),g("mark",["firstbyte",f()])},{handle:"4O2Y62"}],loader:[function(a,b){b.exports=a("YLUGVp")},{}]},{},["YLUGVp"]);</script>

Its W3TC. If I turn off the Newrelic monitoring in W3TC the sitemap works fine. But then the RUM is also turned off and I see no results on Newrelic's page. But the newrelic daemon (newrelic-daemon + php agent) is running on my server.

I do not use the deamon's injection nor the W3TC module, but

if (extension_loaded('newrelic') && !is_user_logged_in()) { echo newrelic_get_browser_timing_header(); }

if (extension_loaded('newrelic') && !is_user_logged_in()) { echo newrelic_get_browser_timing_footer(); }

in header and footer respectively.

yazeed commented Sep 9, 2014

Any update here?

This seems to have been sitting for a while, and the W3TC conflict still exists - so FYI, here's a quick solution that can be implemented in Wordpress-SEO: define('DONOTAUTORUM', TRUE);

In xml-sitemap-xsl.php:

// This is to prevent issues with New Relics stupid auto injection of code. It's ugly but I don't want
// to deal with support requests for other people's wrong code...
if ( extension_loaded( 'newrelic' ) && function_exists( 'newrelic_disable_autorum' ) ) {
        define('DONOTAUTORUM', TRUE);
        newrelic_disable_autorum();
}

@jacobischwartz jacobischwartz added a commit to jacobischwartz/wordpress-seo that referenced this issue May 27, 2017

@jacobischwartz jacobischwartz Resolve conflict with W3TC plugin and New Relic extension
When using the New Relic extension for the popular W3 Total Cache plugin, javascript is injected into the XML stylesheet, causing the stylesheet to break. 

This is already resolved if not using W3TC's NR extension (just using NR's PHP daemon, for example) but this one change makes the solution work for W3TC users too.

See Yoast#603
Also see usage of the DONOTAUTORUM constant in the w3-total-cache plugin.
66ce538
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment