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

idea for how to check for an invalid license key #74

Conversation

cjcjameson
Copy link

@cjcjameson cjcjameson commented Jul 10, 2018

Proposal for new behavior: Invalid License Key

Current Behavior

If the user configures the Application with a 40-digit license key, there's no
further validation at Application-creation time.
Then if they run the application, the behavior StartTransaction() is
indistinguishable from if they had set enabled: false.

I suggest that a user of this library should have a way to distinguish between
a disabled app versus a misconfigured one.

Proposal 1

A new public method on internal_app.app:

func (app *app) ValidateConfig() error

The implementation of this would check that the license key corresponds to an
actual NewRelic account, perhaps by hitting some 'ping' endpoint you might have
available, or by starting a demo transaction.

We wouldn't want to add it to the Application interface in a minor version
bump, because the interface is used as a return value from NewApplication
(rather than a concrete struct).
It could affect clients or their tests that use it.

Proposal 2

Could this be checked as part of newAppRun()?

PR vs. Issues

I'm filing this as a PR, but I'd rather have filed an issue. Is there a reason
issues aren't enabled for your repo?

@crenshaw
Copy link

Thanks for your proposal. It's a bit wonky that the issues tab is closed on this repo. New Relic's esteemed and talented support staff are best equipped at handling issues. As the README.md states,

Any questions or issues should be directed to our support site or our community forum.

Regarding your proposal, the Go Agent already does a couple of things to help customers discern whether they are attempting to connect with an invalid key. First, the API call returns a non-nil error when the application cannot connect:

See:

go-agent/application.go

Lines 55 to 63 in 462a146

// NewApplication creates an Application and spawns goroutines to manage the
// aggregation and harvesting of data. On success, a non-nil Application and a
// nil error are returned. On failure, a nil Application and a non-nil error
// are returned.
//
// Applications do not share global state (other than the shared log.Logger).
// Therefore, it is safe to create multiple applications.
func NewApplication(c Config) (Application, error) {
return newApp(c)

Second, the Go Agent logs a couple of messages indicating that an invalid license key prevented the application from connecting. For example:

go run main.go 
...
(1998) 2018/07/12 16:35:26.520859 {"level":"debug","msg":"rpm failure","context":{"command":"preconnect","error":"NewRelic::Agent::LicenseException: Invalid license key, please contact support@newrelic.com","url":"https://collector.newrelic.com/agent_listener/invoke_raw_method?license_key=2...2222\u0026marshal_format=json\u0026method=preconnect\u0026protocol_version=16"}}
(1998) 2018/07/12 16:35:26.521162 {"level":"debug","msg":"rpm response","context":{"command":"preconnect","response":null,"url":"https://collector.newrelic.com/agent_listener/invoke_raw_method?license_key=2...22222\u0026marshal_format=json\u0026method=preconnect\u0026protocol_version=16"}}

It's hard to do better than the error and the logging; the Go Agent doesn't know whether the license is valid until some background routine finds out.

@crenshaw crenshaw closed this Jul 12, 2018
@purple4reina
Copy link
Contributor

Hi @cjcjameson,

We've recently published some proposed changes to the agent for a 3.0 major version release #106. We're wondering what you think of these changes and if they will fit your needs.

Particularly, we've proposed some changes around how an Application would be configured. What do you think of this? Do you think it will fit your needs? If not, what do you think could improve it?

@cjcjameson cjcjameson mentioned this pull request Oct 3, 2019
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.

None yet

3 participants