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

Install New Relic infrastructure agent #900

Merged
merged 2 commits into from
Oct 10, 2023

Conversation

mkllnk
Copy link
Member

@mkllnk mkllnk commented Oct 5, 2023

I applied this to the three staging servers and the biggest three production servers. Let's see how that goes.

@mkllnk mkllnk self-assigned this Oct 5, 2023
Copy link
Member

@dacook dacook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

@@ -102,6 +102,11 @@
become: yes
tags: logrotate

- role: newrelic
become: yes
when: new_relic_api_key is defined
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This role also depends on new_relic_account_id and new_relic_region, so I was wondering if we should check for those here too.
But then if you forgot to set any of those, it would just silently fail.
👍 So I think this way is best: if you intended to switch it on, but left out a detail, then it would be better to get an error message which shows what's missing.

- name: Install New Relic command
ansible.builtin.command: "/tmp/new-relic-install.sh"
args:
creates: /usr/local/bin/newrelic
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to see some validation of the result.
I can think of a couple of other ways to validate:

  1. check for a non-error return code, or
  2. grep the output for the success message.

But I don't think it's worth spending more effort on figuring one of those out.

Comment on lines +23 to +29
- name: Use our standard names in dashboard
lineinfile:
name: "/etc/newrelic-infra.yml"
line: "display_name: {{ host_id }}"
regex: "^display_name:"
state: present
notify: restart newrelic_infra
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one!

New Relic provides an Ansible role but it's not compatible with our
current versions. So I created this tiny role which basically does the
same, just with fewer options.
This will display au-prod instead of openfoodnetwork.org.au and it will
show uk-prod instead of vps-d2442729. It makes it easier to see the
hosts and filter by production or staging.
@dacook
Copy link
Member

dacook commented Oct 10, 2023

✅ Green build!

@dacook dacook merged commit a50f9d5 into openfoodfoundation:master Oct 10, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants