Objective

We need to publicly expose the client's error types since they are important to state tracking in terraform-provider-newrelic.

Acceptance Criteria

• Publicly expose the error types in internal/errors.go

Objective

Migrate alert_policies.go from go-newrelic to our project.

Acceptance Criteria

• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

AC:

• Add logging support to the client

Objective

Add validation for public/exported APIs.

Acceptance Criteria

• TBD

Objective (pending license result)

Migrate synthetics.go from dollarshaveclub/new-relic-synthetics-go to our project. We probably want to break this large file out into domain-specific files.

Acceptance Criteria

• Do not include any redundant code that might already exist in go-newrelic since it will likely already exist in this new repo - e.g. GET /alerts_synthetics_conditions exists in go-newrelic and in new-relic-synthetics-go, but we will likely already have migrated the go-newrelic code.
• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

Description

Attempting to add multiple channels to an alert policy does not work. Only

Current behavior

When callingalerts.UpdatePolicyChannels(321, []int{123345, 678899}) with multiple channel IDs only the last channel in the slice of IDs is added to the policy. The client generates a call that looks like the URL below.

Example API call:

PUT https://api.newrelic.com/v2/alerts_policy_channels.json?channel_ids=<id1>&channel_ids=<id2>&policy_id=<policyID>

Our current client implementation doesn't seem to handle the multiple instances of the query param channel_ids. Instead of create an array of IDs, it only uses the last one in the query string.

Expected behavior

Attempting to add multiple channel IDs to an alert policy should ensure all IDs passed to the method are added to the associated policy ID.

Steps To Reproduce

Steps to reproduce the behavior:

1. Call alerts.UpdatePolicyChannels(policyID, []channelIDs) with a known existing policy ID and at least 2 known existing channel IDs. Can be set up in an integration test.
2. What the output in the terminal when running the test/call.
3. Check your New Relic account for search for the newly generated alert policy and notice that only one notification channel was added when there should be 2 channels added.

References or Related Issues

This is needed to help support the ease of adding multiple channels to an alert policy via the Terraform provider.

Objective

Migrate labels.go from go-newrelic to our project.

Acceptance Criteria

• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

Objective

Migrate alert_channels.go from go-newrelic to our project.

Acceptance Criteria

• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

Objective

Add support for the recently GA Multi-location Synthetics Condition type. Documentation: https://rpm.newrelic.com/api/explore/alerts_location_failure_conditions/create

Acceptance Criteria

• New Condition type created
• Ability to CRUD the condition type

Objective

Support the CRUD operations for Entity Tags as provided by Nerdgraph.

Acceptance Criteria

• Support the entity tag mutations available via Nerdgraph:
  • taggingAddTagsToEntity
  • taggingDeleteTagFromEntity
  • taggingDeleteTagValuesFromEntity
  • taggingReplaceTagsOnEntity
• Establish a reusable pattern for implementing additional Nerdgraph functionality
• Take note of how this implementation might be generated via Nerdgraph schema introspection and gather notes and insights to that end.

Objective

Migrate alert_nrql_conditions.go from go-newrelic to our project.

Acceptance Criteria

• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

Objective

Migrate types.go from go-newrelic to our project.

Consider whether or not we should put types within their specific domain instead of having all of the types in a single file, which is how go-newrelic has done it. Something like alert_channels_types.go and dashboard_types.go could be a design pattern worth discussing. What advantages/disadvantages would this introduce?

Acceptance Criteria

• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

Objective

Migrate applications.go from go-newrelic to our project.

Acceptance Criteria

• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

Objective

Enable automation of testing, linting, etc via CircleCI.

Feel free to use CircleCI's guide for Go to get things started.

Acceptance Criteria

• Tests run on each commit
• Linting runs on each commit
• PRs are blocked if tests or linting fail

The client should support the applications metrics and metrics data endpoionts:

• https://rpm.newrelic.com/api/explore/applications/metric_names
• https://rpm.newrelic.com/api/explore/applications/metric_data

Objective

Decide on a path forward for integrating with the features available in the Nerdgraph API.

Acceptance Criteria

• Research the available options
  • https://github.com/shurcooL/graphql (Included on the graphql site under clients)
  • https://github.com/machinebox/graphql
  • net/http
• Select an approach to be used

Objective

Infrastructure alert condition thresholds can apparently be set to float values via the New Relic UI. Our Go client should support this as well. The type update would happen here.

The original issue was reported in the terraform-provider-newrelic project.

Acceptance Criteria

• Infrastructure alert condition thresholds support using float values.

Objective

Add a version.go file for our primary client package to facilitate querying for usage metrics in New Relic.

Acceptance Criteria

• A version.go file should follow a similar format to this version.go
• The exported Version variable should be applied to the userAgent header to facilitate querying for usage metrics in New Relic.
  • An example can be seen here in client.go

We need a high-level overview of the primary features and responsibilities that the client should incorporate.

Starting point:

• Include all current capabilities in paultyng/go-newrelic
• Include all current capabilities in dollarshaveclub/new-relic-synthetics-go

Some preliminary questions:

• Will this project contain the CLI?
• How does Terraform fit into the mix?
  • Currently terraform-provider-newrelic leverages paultyng/go-newrelic and dollarshaveclub/new-relic-synthetics-go. If we include a CLI in this project, will we include Terraform features in it?
• Will this project contain more than one package?
  • If so, what will those packages be? (This ties directly to project architecture)
• What additional 3rd party integrations are worthy of being included, if any?
  • i.e. CloudFormation
• Super long term question: Do New Relic Logs come into play for this project?

Objective

Three Alerts objects have time-like fields, but are represented in two different ways. These should ideally be the same way, my preference is for them all to be plain int types, discussed below.

Incident has OpenedAt and ClosedAt timestamp fields represented by an int:

InfrastructureCondition has CreatedAt and UpdatedAt represented by the internal *serialization.EpochTime (a redeclaration of time.Time):

Policy has CreatedAt and UpdatedAt represented by the internal *serialization.EpochTime (a redeclaration of time.Time):

Acceptance Criteria

• These ideally should be the same type
• I'd prefer them being plain ints, possibly with helper functions on the type to translate to time.Time for other uses like displaying to humans.
• An additional improvement would be changing the Incident.ClosedAt field to be a pointer to reflect its optionality rather than being Epoch 0 (January 1, 1970)

Will comment below on the specific application I'm working on and why I'd prefer int types, happy to have the conversation in squaring it with other uses of this library.

Objective

Migrate dashboards.go from go-newrelic to our project.

Acceptance Criteria

• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

Objective

Updating package versions should be automated in some way so that we do not have to remember to update them.

Acceptance Criteria

• Version updates to any package within /pkg/* are automated / required.
• Update to package version in /pkg/* automate / require change to /newrelic/version.go
• /newrelic/version.go is kept in sync with the git tag

Related to

• #24

We have permission to use go-newrelic as the base for the client, we need to:

• Move code over (drop git history?)
• Add AUTHORS / CONTRIBUTORS / what not file referencing prior work

Objective

Add support for Alerts Incidents and the following endpoints:

https://rpm.newrelic.com/api/explore/alerts_incidents/show
https://rpm.newrelic.com/api/explore/alerts_incidents/acknowledge
https://rpm.newrelic.com/api/explore/alerts_incidents/close

Acceptance Criteria

• Able to list current Incidents(show)
• Able to Ack an incident
• Able to Close an incident

Objective

Migrate alert_infra_conditions.go from go-newrelic to our project.

Acceptance Criteria

• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

Objective

Provide a consistent contract for all methods.

Acceptance Criteria

• Favor slices of pointers over slices of structs for endpoints that return collections.
• Favor multiple return values for all endpoints, always returning the object(s) being operated on.
  • e.g. always return a tuple func UpdateEntity(entity Entity) (*Entity, error) {} even if the endpoint this method uses doesn't really return us a response body

Objective

Migrate components.go, component_metrics.go, and component_metric_data.go from go-newrelic to our project.

Acceptance Criteria

• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

Objective

Consolidate the current HTTP client implementations from paultyng/go-newrelic and dollarshaveclub/new-relic-synthetics-go into a single HTTP abstraction layer in our new project. Thanks for Paul and David, we have a pretty good foundation to build from.

Acceptance Criteria

• Support the following APIs (also support international based accounts)
  • REST API - api.newrelic.com/v2 (api.eu.newrelic.com/v2 - eu)
  • Infrastructure API - infra-api.newrelic.com/v2 (infra-api.eu.newrelic.com/v2 - eu)
  • Synthetics API - synthetics.newrelic.com/synthetics/api/v3
• Include basic test coverage
  • Potential testing library testify to facilitate tests, assertions, etc. Note this may not be necessary given how Go testing is already baked into the language.
  • Use go-newrelic client_test.go as a potential reference

We might want to avoid using an HTTP client library if possible, but some potential HTTP libraries to use are:

• req
• resty (currently used in paultyng/go-newrelic)
  ^{These could also be used for inspiration for writing something simple ourselves.}

References:

• Use the go-newrelic HTTP client implementation as a reference (even potentially reuse parts of it)
• Our new HTTP client layer will also replace the dollarshaveclub/new-relic-synthetics-go HTTP client implementation

Objective

We have some test helpers that are available per package, but they since they are repeated per package, we can consolidate into a test helpers package to use in all the packages.

Some examples of these helpers are newMockResponse and newIntegrationTestClient that are located in the {basePackage}_test.go files - i.e. in apm_test.go.

Acceptance Criteria

• Helper methods that make sense to consolidate are put into a centralized testing package.

Objective

Migrate plugins.go from go-newrelic to our project.

Acceptance Criteria

• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

Objective

Migrate alert_plugins_conditions.go from go-newrelic to our project.

Acceptance Criteria

• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

Objective

We need some preliminary feedback from internal developers.

Acceptance Criteria

• An RFC issue has been open and includes some general feedback from New Relic devs

Objective

Migrate key_transactions.go from go-newrelic to our project.

Acceptance Criteria

• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

Objective

Migrate alert_synthetics_conditions.go from go-newrelic to our project.

Acceptance Criteria

• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

https://github.com/terraform-providers/terraform-provider-newrelic/issues/293

Objective

Currently, some objects around alerts.Condition and soon to be alerts.MultiLocationSyntheticsCondition are inconsistent about how they make use of the PolicyID. In both cases, the API object does not contain the PolicyID, but in the case of alerts.Condition, we create an extra field for PolicyID and attache the ID of the policy to the object after the call is made, before returning the object to the user.

We should determine which approach we want to take here and make the code consistent.

Acceptance Criteria

• PolicyID fields that are not present in the API should be managed on conditions, or not.
• Usage consistency of PolicyID fields.

Objective

Placeholder for this issue in the Terraform provider

Objective

Currently, we pass the entire response body to the internal http client to for page handling. If the responses are particularly large, that is a bunch of data to pass around.

Acceptance Criteria

• Paging parser logic is reviewed
• Decision is made if there is to be a refactor
• Refactoring completed if necessary

Research if we can migrate the existing synthetics implementation or if we should rewrite.

• Repository
• MIT License

Rewrite is out of scope, migration would remain in scope.

Objective

Migrate deployments.go from go-newrelic to our project.

Acceptance Criteria

• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

Description

Infra API appears to return the data that caused an error and a 400 HTTP response for invalid config of infra_process_running, which we do not correctly handle.

Current behavior

The newrelic-client-go attempts to unmarshal the JSON into an int, which does not work.

Expected behavior

Concise error returned to the caller.

References or Related Issues

Reported in here on the Terraform provider.

We need to define the layout including but not limited to:

• Code organization
• Vendor directory
• Internal / public packages

Some useful information can be found in golang-standards/project-layout

Objective

Provide the cleanest representation of resource types possible for the project.

Acceptance Criteria

• Decide as a team on the best possible refactoring. Some options:
  • Provide a types.go file per package
  • Move types into their associated resource files
  • None required

Hi,

I have been playing with new relic terraform plugin which is using newrelic-client-go. I noticed that when I was searching for an alert notification channel - depending on environment - sometimes it was found, sometimes not. When I tried to compare the difference between environments - the only one was number of already defined channels. I thought that problem might be with plugin getting just one page of data - but since it is using newrelic-client-go - it should receive all data regardless how many is defined in NewRelic.

Playing with the code - I've noticed that data returned from newrelic-client-go ListChannels method is corrupted - multiple entries were duplicated. I think it is caused by using reference for response (&response) when calling alerts.client.Get - as same object is being reuse - it does not reallocate memory, which cause entries in array overwritten. Call ends up with proper number of elements in alertChannels slice, but with duplicated entries.

I've checked that assigning fresh value for response works as expected. As I'm not a Go expert maybe there is a better solution.

func (alerts *Alerts) ListChannels() ([]*Channel, error) {
	response := alertChannelsResponse{}
	alertChannels := []*Channel{}
	nextURL := "/alerts_channels.json"

	for nextURL != "" {
		resp, err := alerts.client.Get(nextURL, nil, &response)

		if err != nil {
			return nil, err
		}

		alertChannels = append(alertChannels, response.Channels...)

		paging := alerts.pager.Parse(resp)
		nextURL = paging.Next

		response = alertChannelsResponse{}
	}

	return alertChannels, nil
}

Thanks.

Objective

Migrate alert_policy_channels.go from go-newrelic to our project.

Acceptance Criteria

• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

Objective

Migrate alert_conditions.go from go-newrelic to our project.

Acceptance Criteria

• Migrate the code and make any reasonable updates to the implementation pattern
  • Note: The design pattern should be agreed upon by the dev team and should be consistent with all other features/resources
• Include tests

Objective

To facilitate better test coverage and ease of writing tests, we need a some helper functions to help create generic mocks.

Acceptance Criteria

• Create helper method to mock application response data
• TBD

TBD

In order to provider user metrics, we would need a user's permission to track their usage statistics. This is not an uncommon request these days in CLI frameworks.

This would also likely require a Terms and Privacy Policy acceptance as well.