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

Add New Relic backend #85

Merged
merged 3 commits into from
May 14, 2019
Merged

Add New Relic backend #85

merged 3 commits into from
May 14, 2019

Conversation

chloeruka
Copy link

@chloeruka chloeruka commented May 13, 2019

Description

Adds New Relic Insights monitoring integration to buildkite-agent-metrics. I discovered NR does not accept custom Cloudwatch metrics out of the box so this change was born out of a need to write glue code to forward these metrics to New Relic. This work has been tested and is functional in Culture Amp's continuous integration environment as a Lambda using AWS SAM to deploy and manage it. Further rationale for why this might be considered is that New Relic's go-agent is, mercifully, a standalone dependency. 🎉

Changes

  • Added backend/newrelic.go driver to support publishing New Relic custom events
  • Brief tests; admittedly this doesn't have great code cov as I didn't really want to invoke mocking libraries for something so minor
  • Necessary changes to lambda/main.go to enable this as an optional backend for the AWS Lambda binary
  • Adds New Relic go-agent dependency
  • Updated README and command line messages to reflect the new backend option

Notable areas

  • Line 110 in lambda/main.go invokes a typecast check and a call to a disposal method. Since as far as I can tell the NR backend is the only one that might require this, I didn't think it necessary to add it to the base interface for all backend implementations. Still, I'd appreciate your thoughts if there's a cleaner way to approach this (I'm pretty new to golang 😳).

backend/newrelic.go Outdated Show resolved Hide resolved
lambda/main.go Show resolved Hide resolved
lambda/main.go Outdated Show resolved Hide resolved
lambda/main.go Outdated Show resolved Hide resolved
@lox
Copy link
Contributor

lox commented May 13, 2019

This looks great! Some extremely minor nitpicks, although would happily merge as is.

- Rename Dispose to Close to be more idiomatic with Go
- Use strings.EqualFold for case insensitive comparison
- Implement a backend Closer interface to be more generic
@chloeruka
Copy link
Author

@lox Made some updates w.r.t. your suggestions, please re-review 😄

lambda/main.go Outdated Show resolved Hide resolved
@lox
Copy link
Contributor

lox commented May 14, 2019

Made some more minor nitpicks @ChloeHutchinson!

@chloeruka
Copy link
Author

Made another update, thanks @lox

@lox
Copy link
Contributor

lox commented May 14, 2019

🙌🏻

@lox lox merged commit 68d13f4 into buildkite:master May 14, 2019
@lox
Copy link
Contributor

lox commented May 14, 2019

Will put this out in a new version shortly!

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.

2 participants