Add new relic gem #11256
Add new relic gem #11256
Conversation
rioug
force-pushed
the
10997-add-new-relic-gem
branch
2 times, most recently
from
c494954
to
4867b13
Compare
Gemfile
Outdated
| @@ -139,6 +139,7 @@ gem "faraday" | |||
| gem "private_address_check" | |||
|
|
|||
| group :production, :staging do | |||
| gem 'newrelic_rpm' | |||
There was a problem hiding this comment.
Would it hurt to have it in all environments?
I find it increases the chance to catch compatibility bugs early.
There was a problem hiding this comment.
It don't think it would. I fixed up the configuration so we don't run the agent by default in Dev. And I realised I forgot to actually test if the configuration was working, which I have now done.
rioug
force-pushed
the
10997-add-new-relic-gem
branch
2 times, most recently
from
f016de7
to
ce533ef
Compare
|
I don't actually know how env variable are set up on each instance, maybe someone more knowledgeable can update the testing note ? |
config/newrelic.yml
Outdated
|
|
||
| # To disable the agent regardless of other settings, uncomment the following: | ||
|
|
||
| # agent_enabled: false |
There was a problem hiding this comment.
I think we probably want to disable the agent by default if there's no API key present (in development for example). I think this file allows interpolation with erb.
Maybe here we can do something like:
agent_enabled: <%= ENV["NEW_RELIC_LICENSE_KEY"].present? %>There was a problem hiding this comment.
the new relic config doesn't allow interpolation any more. I couldn't find a way to disable the agent if the license key is not present, but I updated the config to disable the agent in dev, test and staging.
There was a problem hiding this comment.
That's a problem. Not every production instance will have a new relic key. Do we set it to false everywhere and then use environment variables to control it instead?
There was a problem hiding this comment.
Yes I think that would be a better solution. Do we need to update any documentation around this ?
There was a problem hiding this comment.
Do we need to update any documentation around this ?
Yes.
- The ofn-install repository has a template for the secrets which needs entries with comments.
- The wiki describes optional services, how to set them up etc. It needs to be mentioned there.
rioug
force-pushed
the
10997-add-new-relic-gem
branch
2 times, most recently
from
8441421
to
2bfe217
Compare
config/newrelic.yml
Outdated
| staging: | ||
| <<: *default_settings | ||
| app_name: 'Open Food Network (Staging)' | ||
| agent_enabled: false |
There was a problem hiding this comment.
How are we going to test changes before they go to production? It would be great to have this on staging as well. And testing locally?
There was a problem hiding this comment.
you can override the configuration by setting the env variable NEW_RELIC_AGENT_ENABLED to true
rioug
force-pushed
the
10997-add-new-relic-gem
branch
2 times, most recently
from
5c55908
to
f4b9a6f
Compare
| # Your application name. Renaming here affects where data displays in New | ||
| # Relic. For more details, see https://docs.newrelic.com/docs/apm/new-relic-apm/maintenance/renaming-applications | ||
| # Use NEW_RELIC_APP_NAME env variable to customize the application name | ||
| app_name: "Open Food Network" |
There was a problem hiding this comment.
I have one question:
Will all the instances share the same key? and NEW_RELIC_APP_NAME will variate depending on the instance location. for example: "Open Food Network (AU)", "Open Food Network (UK)" ?
There was a problem hiding this comment.
That's a good question. In the past we would have set up separate keys but if New Relic supports it then we should use the same key, I think. That we we have all the information in one place.
There was a problem hiding this comment.
My understanding is we should be able to use the same license key for all the instances, and set up a different name for each instance so we can differentiate the data. See below:
There is the possibility to create extra license key so we could potentially have a different license key for each instance, all reporting to the same New Relic account. Or have a different user with it's own license key for each instance, but that's probably not needed. It's recommended to create extra license for security reason (key rotation) .
More info here: https://docs.newrelic.com/docs/apis/intro-apis/new-relic-api-keys/
There was a problem hiding this comment.
Okay, I think that we should roll out with the same key and we can rotate later.
rioug
added
pr-staged-au
rioug
force-pushed
the
10997-add-new-relic-gem
branch
from
f4b9a6f
to
e2e975d
Compare
|
I tested this on |
It can be enable as needed by setting NEW_RELIC_AGENT_ENABLED env variable to true and setting a license key
rioug
force-pushed
the
10997-add-new-relic-gem
branch
from
e2e975d
to
0103942
Compare
What? Why?
Related to #10997
It uses the following env varirable to enable new relic agent, setup the licence key and customise application name:
The agent is disable by default so we can enable new relic for the production server that needs it.
What should we test?
Release notes
Changelog Category: Technical changes
The title of the pull request will be included in the release notes.