grpctest: minor improvements to the test logger implementation

[asheshvidyut]

Fixes: #7475

• Unexport the test logger implementation which is currently a package global here:
  grpc-go/internal/grpctest/tlogger.go

• Make the above package global accessed atomically from tests. We have had test failures because of data races in the past because of this.

• Change receiver names in method of the tLogger type to be tl instead of g

RELEASE NOTES: N/A

[codecov]

Codecov Report

Attention: Patch coverage is 76.13636% with 21 lines in your changes missing coverage. Please review.

Project coverage is 82.41%. Comparing base (20bd1e7) to head (a4b8b96).

Files with missing lines	Patch %	Lines
internal/grpctest/tlogger.go	75.58%	19 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8370      +/-   ##
==========================================
- Coverage   82.44%   82.41%   -0.04%     
==========================================
  Files         413      413              
  Lines       40424    40432       +8     
==========================================
- Hits        33328    33321       -7     
- Misses       5742     5754      +12     
- Partials     1354     1357       +3     

Files with missing lines	Coverage Δ
internal/grpctest/grpctest.go	78.43% <100.00%> (ø)
internal/grpctest/tlogger.go	70.92% <75.58%> (-3.52%)	⬇️

... and 22 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[arjan-bal]

@easwars, the logger is mutated only once, in the init() function of this package. The race usually happens in the Update method (e.g: b/416700146#comment4) when it calls grpclog.SetLoggerV2:

grpc-go/grpclog/loggerv2.go

Lines 35 to 41 in 6dfe07c

	func SetLoggerV2(l LoggerV2) {
	if _, ok := l.(*componentData); ok {
	panic("cannot use component logger as grpclog logger")
	}
	internal.LoggerV2Impl = l
	internal.DepthLoggerV2Impl, _ = l.(internal.DepthLoggerV2)
	}

Ideally Update should be called serially at the beginning of a test function. I've seen race conditions reported when a test doesn't wait for its goroutines to return before the test function exits. Using an atomic for LoggerV2Impl and DepthLoggerV2Impl might hide those errors and make it appear as though the next test has logged something. Do you think we should really change this to use an atomic?

[arjan-bal]

Talked to @easwars and he mentioned that we can revert the changes to make the logger an atomic.

[arjan-bal]

LGTM, pending the removal of the atomic. Over to Easwar for the second review.

[easwars]

LGTM, modulo minor nits

[easwars]

Do we really need this function?

[easwars]

While you are here, you could reorder this such that we read the env var first, and then initialize the tLogger. That way, you could directly set the v field using the literal struct initialization syntax.

Also, you could use os.LookupEnv instead of os.GetEnv to distinguish between an empty value and an unset value.

[easwars]

While you are here, could we unexport the Update and EndTest methods on tLogger?

[easwars]

LGTM, modulo minor nits

[asheshvidyut]

@arjan-bal is it fine if we print this message?