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

Use code templates #30

Merged
merged 5 commits into from
May 6, 2020

Conversation

miguelcobain
Copy link
Contributor

@miguelcobain miguelcobain commented Jun 5, 2019

The PR includes #29
Check this commit if you want to see only the new changes that aren't already on that PR: 2d59fc0

What this does is to separate the new relic code scripts from the index.js file.
We now place them in a separate new-relic-templates folder and there we can use {{expressions}} to replace values.

I also updates the classic and spa templates with the latest. I grabbed them from the new relic dashboard.
By the way, what is exactly the "classic" code? I used the "pro" version for the classic. Maybe new relic changed names a bit. This is how I see it right now:

Screen Shot 2019-06-05 at 23 16 50

So now we have 3 different types of codes: Lite, Pro and Pro + SPA.

Should we update our settings a bit to allow the user to choose one of those 3?

This PR also wraps the new-relic code using fastboot-transform package. So this also closes #23 and #19.

@miguelcobain miguelcobain force-pushed the use-code-templates branch 2 times, most recently from cdca0b4 to 4613b3c Compare June 5, 2019 22:58
@miguelcobain miguelcobain force-pushed the use-code-templates branch 2 times, most recently from 026d0c0 to 41c82d0 Compare June 5, 2019 23:05
@miguelcobain
Copy link
Contributor Author

@sir-dunxalot any feedback?
I think this is a great feature!

@GCheung55
Copy link

@sir-dunxalot @miguelcobain I'm also interested in having this merged/released. New Relic informed me that I have an NR code being loaded.

@gilest
Copy link

gilest commented Sep 25, 2019

Possible candidate for Adoption ?

@GCheung55
Copy link

@sir-dunxalot Just following up. Anything we can help with to move this along?

@les2
Copy link

les2 commented Apr 16, 2020

Note: The SPA snippet needs to optionally handle distributed tracing config.

We use a custom "solution" (if you wanna call my hack that) for newrelic integration that handles a couple of edge cases not covered by community addons.

I would not mind helping / sharing learnings so that we could switch to a community addon.

These are a few of the things we handle in our in-house addon:

  1. Distributed tracing support
  2. Release identification (with CSP support)
  3. Service for setting customAttributes, e.g., authenticated principal username / id, tenant id and name, etc

Distributed tracing support

The SPA snippet you download from the the URL doesn't include distributed tracing config yet (worked with NR support on this). To enable it, you much need to pre-prepend the config as shown below:

      const newRelicContent = fs.readFileSync(`${ __dirname }/my-templates/new-relic/new-relic.js`, 'utf8');
      const prefixes = [
        ';window.NREUM||(NREUM={});NREUM.init={distributed_tracing:{enabled:true}}'
        // ^^^^^ this prefix is needed when distributed tracing is enabled
      ];
      const suffixes = [
      `;NREUM.loader_config={accountID:"<account ID>",trustKey:"<trust ID>",agentID:${ JSON.stringify(newRelicConfig.applicationId) },licenseKey:"<licence key>",applicationID:${ JSON.stringify(newRelicConfig.applicationId) }}`,
      `;NREUM.info={beacon:"bam.nr-data.net",errorBeacon:"bam.nr-data.net",licenseKey:"<license key>",applicationID: ${ JSON.stringify(newRelicConfig.applicationId) }, sa:1}`,
      ];
      const nrScript = `\n${prefixes.join('')}${newRelicContent}${suffixes.join('')}\n`;
      // ^^^^^ this is script we inject in head

Release identification (with CSP support)

Supporting newrelic.addRelease( to identify version id of release to newrelic.
This also requires deployment step to create version using NR API. We also take care of adding hash of this script to CSP (with tokens replaced).

Our implementation passes in token values via environment variables since version number is not contained in source code but is determined by CI automatically. In other words:

NEW_RELIC_RELEASE_NAME='foo' NEW_RELIC_RELEASE_ID='bar' ember build --environment production
<script>
(function(newrelic, releaseName, releaseId) {
if (newrelic && newrelic.addRelease) {
  newrelic.addRelease(releaseName, releaseId);
}
  })(window.NREUM, "{{NEW_RELIC_RELEASE_NAME}}", "{{NEW_RELIC_RELEASE_ID}}");
</script>

Sorry for rambling.

@sir-dunxalot sir-dunxalot merged commit 4cadccd into sir-dunxalot:master May 6, 2020
@sir-dunxalot
Copy link
Owner

Thanks @miguelcobain! I've merged, updated additional deps locally, and will release a new version soon.

Frankly, I no longer use New Relic on any projects (and barely use Ember these days) so I hope to find a co-maintainer for this repo. Let me know if you're interested.

@les2 I believe your comments are best suited to a new issue or pull request. I do not know the demand for distributed tracing. I will happily receive a PR with test coverage. Regarding release identification, is that something this addon should support? Maybe your example is best added as instructions in the README.

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.

5 participants