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

How should client apps properly mute log message emitted by your package? #4

Closed
atc0005 opened this issue Mar 24, 2020 · 3 comments
Closed
Labels
enhancement New feature or request
Milestone

Comments

@atc0005
Copy link
Collaborator

atc0005 commented Mar 24, 2020

I'm fairly new to Go still, so there is probably an obvious way to handle this, but is there an easy way to mute log messages generated by your package when errors are encountered?

For example:

go-teams-notify/send.go

Lines 49 to 54 in feea812

// do the request
res, err := c.httpClient.Do(req)
if err != nil {
log.Println(err)
return err
}

Here the message is both logged and returned to the caller. From my app I've got the error message that was returned, but I also get the log output.

To work around that I've used an approach like this to temporarily mute all log output:

    log.SetOutput(ioutil.Discard)
    if err := sendMessage(webhook.WebhookURL, msgCard); err != nil {
        // our error handling here
    }
    log.SetOutput(os.Stdout)

Is this the recommended approach, or is there a way to more directly target log messages generated by this package?

Thanks in advance for your time.

@dasrick
Copy link
Owner

dasrick commented Mar 26, 2020

You are right. This package should only return an error instead of doing both (logging and returning).
I will take it into one of the next changes ...

@atc0005
Copy link
Collaborator Author

atc0005 commented Mar 26, 2020

Awesome, thank you.

I did some digging last night to see if Go handles this in the same way as Python does and got back mixed information. One example I did find common to both was setting up a package level (exported) logger instance/object and setting its output to disabled by default. The package user could then opt to enable the package-level logger if they need to see relevant troubleshooting/verbose details from the package. Not sure if that's overkill for this package or if it would be "freeing" in a sense to allow for more potential logging.

I think I might be rambling at this point, so I'll leave off here.

@dasrick dasrick added this to the v2 milestone Mar 26, 2020
@dasrick dasrick added the enhancement New feature or request label Mar 26, 2020
@dasrick
Copy link
Owner

dasrick commented Mar 29, 2020

solved in versions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants