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

NOOP Application #99

Closed
whuang8 opened this issue Aug 20, 2019 · 5 comments
Closed

NOOP Application #99

whuang8 opened this issue Aug 20, 2019 · 5 comments

Comments

@whuang8
Copy link

whuang8 commented Aug 20, 2019

Hello! I would like to propose the idea of a NoopApplication that implements the normal Application interface and does nothing in all of the methods.

Problem

When an error occurs during newrelic.NewApplication(), a nil application is returned. If we want to proceed with the execution of the go program, every time we want to use app, we would have to first make sure that it is not nil.

var app newrelic.Application

func main() {
        config := newrelic.NewConfig("Application Name", "invalidLicense")
        // Invalid license will cause error
        app, err := newrelic.NewApplication(config)
        if err != nil {
                log.Warn("Failed to create New Relic application")
        }

        doSomething()
}

func doSomething() {
        if app != nil {
                txn := app.StartTransaction("doSomething", nil, nil)
                defer txn.End()
        }

        // Doing something
}

Proposal

If newrelic.NewApplication() returns a nil application, newrelic.NewNoopApplication() can be called to generate a NoopApplication that does nothing. This eliminates the need for nil checks every time the application is used.

var app newrelic.Application

func main() {
        config := newrelic.NewConfig("Application Name", "invalidLicense")
        // Invalid license will cause error
        app, err := newrelic.NewApplication(config)
        if err != nil {
                log.Warn("Failed to create New Relic application")
                app = newrelic.NewNoopApplication()
        }

        doSomething()
}

func doSomething() {
        txn := app.StartTransaction("doSomething", nil, nil)
        defer txn.End()

        // Doing something
}

Thanks for reading my proposal! Let me know what you think.

Thank you for the awesome work on this package so far. Integrating it in my application was a breeze because of the 💯 documentation.

@newrelic-eheinlein
Copy link
Contributor

Hi @whuang8 , thank you for your suggestion (and your kind words)! We agree that this is a useful idea and will be investigating further as a team; we'll let you know what the outcome is.

@whuang8
Copy link
Author

whuang8 commented Aug 20, 2019

@newrelic-eheinlein Sounds good! Thanks for the quick reply :)

@newrelic-eheinlein
Copy link
Contributor

Hi @whuang8 - we had some initial discussion about this idea and want to delve into it further. We have some possible changes in mind for the agent that might work well alongside this idea, so we are going to do some further planning. We will get back to you once we come to a decision, but that may not be for a little while. Thank you for your suggestion and sparking this discussion!

@purple4reina
Copy link
Contributor

Hi @whuang8!

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 are thinking of changing the Application and Transaction types from interfaces to structs. This would solve your issue of needing to do nil checks everywhere because the new types would be nil safe throughout.

There are some other changes in there as well and we'd love your thoughts on all of them!

@purple4reina
Copy link
Contributor

Hey @whuang8!

We just released v3.0.0 of the agent which removes the Application and Transaction interfaces and replaces them with structs instead. Thanks a ton for opening this issue and letting us know about your pain points!

Let us know what you think of the new version!

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

No branches or pull requests

3 participants