Skip to content

NREUM should not be undefined when creating window.NewrelicTiming#2

Open
cesarandreu wants to merge 2 commits intouken:masterfrom
cesarandreu:master
Open

NREUM should not be undefined when creating window.NewrelicTiming#2
cesarandreu wants to merge 2 commits intouken:masterfrom
cesarandreu:master

Conversation

@cesarandreu
Copy link
Contributor

Now when window.NewrelicTiming is defined, if NREUM is undefined it will default to being an empty object. This means your newrelic scripts can load at a later point and you'll be able to call NREUM.inlineHit once it's finally loaded. Otherwise, since it's inside a closure, it will only bind to undefined.

@pitr
Copy link
Contributor

pitr commented Feb 19, 2014

The only problem I see is what if newrelic changes their script. Right now they simply initialize NREUM to {} if it's not set. What if they initialize NREUM earlier (perhaps as part of injected code). See https://js-agent.newrelic.com/nr-100.js

@nrn
Copy link

nrn commented Feb 19, 2014

There are no plans to clobber the NREUM object in our new instrumentation, we need the flexibility to have that object created at different times depending on injection order, so everything checks to see if it's been defined and uses the existing object if it is there. In most cases this should happen after NREUM has been defined by our instrumentation, but before inlineHit is added.

@cesarandreu
Copy link
Contributor Author

The problem I'm currently facing is that I have Rails serving up one static html file (index.html) for all non-api routes, and the newrelic_rpm gem is inserting the new relic js at the end of the file, so newrelic-timing was not working for me.

@pitr
Copy link
Contributor

pitr commented Feb 19, 2014

perhaps instead of resolving reference to window.NREUM and passing it at load time, we can access NREUM on window when we actually need it. @cesarandreu what do you think?

@pitr
Copy link
Contributor

pitr commented Feb 19, 2014

@nrn any plans at newrelic to include functionality of this gem?

@nrn
Copy link

nrn commented Feb 19, 2014

Angular instrumentation is definitely on the road map, but what exactly that looks like or when is still up in the air.

@cesarandreu
Copy link
Contributor Author

@pitr that would also work fine. I can write a PR for that if you want, probably either tonight or tomorrow.

@pitr
Copy link
Contributor

pitr commented Feb 19, 2014

@cesarandreu that would be great

@diwu1989
Copy link

I second that, right now I have to pay attention to place files in the right order.

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.

4 participants