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

proposal: log: wrap testlog.SetLogger() Visible to the public in package log. #40720

Closed
liu-xuewen opened this issue Aug 12, 2020 · 10 comments
Closed
Labels
FrozenDueToAge Proposal WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@liu-xuewen
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
1.15

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
go version go1.15 linux/amd64

What did you do?

package log

import (
	"internal/testlog"
)

// SetTestLogger Set log for the testlog
func SetTestLogger(log testlog.Interface)  {
	testlog.SetLogger(log)
}
@gopherbot gopherbot added this to the Proposal milestone Aug 12, 2020
@davecheney
Copy link
Contributor

@liu-xuewen could you please describe the problem that you would be able to solve if this proposal were accepted.

@ianlancetaylor ianlancetaylor added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Aug 12, 2020
@liu-xuewen
Copy link
Contributor Author

@liu-xuewen could you please describe the problem that you would be able to solve if this proposal were accepted.

Package testlog provides a back-channel communication path between tests and package os, so that wen can see which environment variables and files a test consults.
However, testlog is not allowed to be used in the internal package. We can use testlog to do some logging to complete the above functions if this proposal were accepted. The implementation is also very simple.

@ianlancetaylor
Copy link
Contributor

As you say, the testlog is an internal mechanism. We do not want to expose it for general use. If other packages start to use it, that will make it impossible to update the internal mechanism.

Can you explain more precisely what it is that you want to do, and why? At the moment I think it is quite unlikely that this proposal would be accepted.

@liu-xuewen
Copy link
Contributor Author

liu-xuewen commented Aug 13, 2020

I want to do more debug log information about the go process net, such as which files are opened, such as methods os.Open(). Thank you.
Because of the method( like os.Open )Logging is implemented internally. It is only useful when testing. I think it is also used in business debugging.

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Aug 13, 2020
@ianlancetaylor
Copy link
Contributor

If we were going to support more debug logging in the standard library, we would not do it via the internal/testlog mechanism. That has a specific purpose related to test caching. It's not for debug logging.

In general debug logging sounds like something that should be implemented as a wrapper around the standard library, not inside the standard library. But it's true that there are exceptions such as the net/http/httptest package.

@liu-xuewen
Copy link
Contributor Author

I see what you mean, thank you. Maybe net logging can share a set of underlying testlog methods with testing.

@rsc
Copy link
Contributor

rsc commented Jul 28, 2021

The exact proposal here is unclear. Changing the behavior of internal/testlog would change nothing user-visible.

@rsc rsc moved this from Incoming to Active in Proposals (old) Jul 28, 2021
@rsc
Copy link
Contributor

rsc commented Jul 28, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Aug 4, 2021

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc rsc moved this from Active to Likely Decline in Proposals (old) Aug 4, 2021
@rsc rsc moved this from Likely Decline to Declined in Proposals (old) Aug 11, 2021
@rsc
Copy link
Contributor

rsc commented Aug 11, 2021

No change in consensus, so declined.
— rsc for the proposal review group

@rsc rsc closed this as completed Aug 11, 2021
@golang golang locked and limited conversation to collaborators Aug 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Proposal WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
No open projects
Development

No branches or pull requests

5 participants