newrelic/newrelic-client-go

New Relic Client for the Go programming language

newrelic-client-go's Issues

Migrate Dashboards

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

Gain consistency about setting PolicyID on objects

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.

Migrate Plugins

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

Migrate NRQL Alert Conditions

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

Migrate Deployments

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

Migrate Synthetics from `dollarshaveclub/new-relic-synthetics-go`

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

Add version.go

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

Migrate Plugins Alert Conditions

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

Define client architecture

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

Configure CI

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

Implement anonymous usage metrics, etc

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.

Alert Policy Channels: Cannot add more than one channel to a policy at a time

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.

Migrate Applications

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

Infra Alert Conditions: support thresholds with float values

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.

Provide support for Applications Metrics and Metrics Data endpoints

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

Migrate Synthetics Alert Conditions

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

feat: Multi-location Synthetics Conditions

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

Custom unmarshal method for Payload and Header fields in ChannelConfiguration struct

Objective

Placeholder for this issue in the Terraform provider

Spike: Plan + document primary client features + responsibilities

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?

Migrate Alert Infra Conditions

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

Automate version increments for packages

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

Consolidate client HTTP abstraction layer

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

Migrate the Terraform Provider to use newrelic-client-go for Synthetics

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

Migrate Alert Channels

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

Migrate Labels

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

Gain consistency about time types in Alert objects

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:

newrelic-client-go/pkg/alerts/alerts_types.go

Lines 86 to 87 in 8eda621

	OpenedAt int `json:"opened_at,omitempty"`
	ClosedAt int `json:"closed_at,omitempty"`

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

newrelic-client-go/pkg/alerts/alerts_types.go

Lines 101 to 113 in 8eda621

	CreatedAt *serialization.EpochTime `json:"created_at_epoch_millis,omitempty"`
	Critical *InfrastructureConditionThreshold `json:"critical_threshold,omitempty"`
	Enabled bool `json:"enabled"`
	Event string `json:"event_type,omitempty"`
	ID int `json:"id,omitempty"`
	IntegrationProvider string `json:"integration_provider,omitempty"`
	Name string `json:"name,omitempty"`
	PolicyID int `json:"policy_id,omitempty"`
	ProcessWhere string `json:"process_where_clause,omitempty"`
	RunbookURL string `json:"runbook_url,omitempty"`
	Select string `json:"select_value,omitempty"`
	Type string `json:"type,omitempty"`
	UpdatedAt *serialization.EpochTime `json:"updated_at_epoch_millis,omitempty"`

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

newrelic-client-go/pkg/alerts/alerts_types.go

Lines 174 to 175 in 8eda621

	CreatedAt *serialization.EpochTime `json:"created_at,omitempty"`
	UpdatedAt *serialization.EpochTime `json:"updated_at,omitempty"`

(these are also pointers, which is fine, it can stay that way for representing optional fields)

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.

Add logging

AC:

• Add logging support to the client

Improve infra_process_running error handling

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.

Select a method of integrating with Nerdgraph

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

Publicly expose error types

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

Review http paging parser logic

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

Ensure consistency among all resource endpoints

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

Migrate Components, Component Metrics, and Component Metrics Data

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

Add client validation

Objective

Add validation for public/exported APIs.

Acceptance Criteria

• TBD

Synthetics Go Client research

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.

Provide support for Entity Tags

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.

Add support for deployment marker creation

Alerts Incidents management

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

Add unit test helpers for mocking

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

Migrate Key Transactions

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

Add support for deployment marker retrieval

Migrate go-newrelic

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

Migrate Types

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

Migrate Alert Conditions

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

Optimize testing utils/helpers into one package

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.

Migrate Alert Policies

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

Get feedback from devs regarding client UX

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

Decide on a refactoring for "_types.go" files

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

Migrate Alert Policy Channels

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

Values overwritten when more than 1 page of data available

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.