There is no need to use atomic.Value, atomic.Pointer should also
be more performant here.
Change-Id: I04cc1befbe1112665a9d9e16197ea3caef5e5fa3
GitHub-Last-Rev: 577902519f
GitHub-Pull-Request: golang/go#62288
Reviewed-on: https://go-review.googlesource.com/c/go/+/523035
TryBot-Result: Gopher Robot <gobot@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Mateusz Poliwczak <mpoliwczak34@gmail.com>
In CL 421877 and CL 444278, time.Time.AppendFormat has been
specially optimized for the time.RFC3339Nano representation.
Relying on that optimization and modify the output to obtain the
fixed-width millisecond resolution that slog uses.
This both removes a lot of code and also improves performance:
name old time/op new time/op delta
WriteTime 93.0ns ± 1% 80.8ns ± 0% -13.17% (p=0.000 n=8+9)
Change-Id: I61e8f4476c111443e3e2098a45b2c21a76137345
Reviewed-on: https://go-review.googlesource.com/c/go/+/478757
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
Change-Id: Ic4fcfe7335dab219790c19ded3bbb7265857404f
GitHub-Last-Rev: afc69c79b2
GitHub-Pull-Request: golang/go#62062
Reviewed-on: https://go-review.googlesource.com/c/go/+/519955
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Change-Id: Ib3daffb8a4cc018d62ed6e5741355b1c1a206034
Reviewed-on: https://go-review.googlesource.com/c/go/+/515775
Run-TryBot: Andy Pan <panjf2000@gmail.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
If the shared slice in a copied is modified, make a copy of it
and insert an attribute that warns of the bug.
Previously, we panicked, and panics in logging code should be avoided.
Change-Id: I24e9b0bf5c8cd09cf733e7dae8a82d025ef214e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/513760
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Fixes#61648
Change-Id: I6b7f4948ca89142a358d74624607daf42ea8b304
Reviewed-on: https://go-review.googlesource.com/c/go/+/514135
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Andy Pan <panjf2000@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
This extra newline causes pkg.go.dev and gopls to only show the bottom
half of this comment; I'm pretty sure this entire thing is meant to be
in the docs.
Change-Id: I5bbf081fb2072d9d773d5a995bc3693dc44f65ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/511855
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
This causes instances commonHandler created by withAttrs or withGroup to
share a mutex with their parent preventing concurrent writes to their
shared writer.
Fixes#61321
Change-Id: Ieec225e88ad51c84b41bad6c409fac48c90320b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/509196
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Passing nil for a context is discouraged. We should avoid it.
Fixes#61219.
Change-Id: I664387070aacb56af580b6b0791ca40982d2a711
Reviewed-on: https://go-review.googlesource.com/c/go/+/508437
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Handlers should not display empty groups.
A group with no attributes is certainly empty. But we also want to
consider a group to be empty if all its attributes are empty groups.
The built-in handlers did not handle this second case properly.
This CL fixes that.
There are two places in the implementation that we need to consider.
For Values of KindGroup, we change the GroupValue constructor to omit
Attrs that are empty groups. A Group is then empty if and only if it
has no Attrs. This avoids a recursive check for emptiness.
It does require allocation, but that doesn't worry us because Group
values should be relatively rare.
For groups established by WithGroup, we avoid opening such groups
unless the Record contains non-empty groups. As we did for values, we
avoid adding empty groups to records in the first place, so we only
need to check that the record has at least one Attr.
We are doing extra work, so we need to make sure we aren't slowing
things down unduly. Benchmarks before and after this change show
minimal differences.
Fixes#61067.
Change-Id: I684c7ca834bbf69210516faecae04ee548846fb7
Reviewed-on: https://go-review.googlesource.com/c/go/+/508436
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Adds an optional close quote in the expected log message regex for TestConnections to prevent failing when the source filepath is surrounded in quotes due to it containing one or more spaces.
Fixes#61161
Change-Id: I0dd71fb4389bff963bbfdc668ef4e4dfe787eafc
Reviewed-on: https://go-review.googlesource.com/c/go/+/508055
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
It said that Attrs with an empty key are ignored.
In fact, zero Attrs are ignored.
Fixes#60870.
Change-Id: I221d3a25b0f0cc9001e06e9cc76bab29292c0741
Reviewed-on: https://go-review.googlesource.com/c/go/+/504535
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Add a shortlink in the doc to a guide to writing handlers, which is a work
in progress.
Change-Id: I1b01c90468382ffe53d9ad6f38253906e3f44857
Reviewed-on: https://go-review.googlesource.com/c/go/+/495920
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Previously, handleState.prefix was nil if and only if the length of
the prefix was zero. Now, prefix is never nil.
Fix the nil check in the code by also checking if the length is non-zero.
Change-Id: I9f69c0029cb1c73fe6c2919c78fee7d4085bfd85
Reviewed-on: https://go-review.googlesource.com/c/go/+/495977
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
It doesn't make sense to call Logger.WithGroup with the empty string.
Make it a no-op by returning the receiver.
This relieves handlers of the burden of detecting that case themselves.
Less importantly, but for consistency, if Logger.With is called with
no args, make it a no-op by returning the receiver.
Along the way, fix obsolete mentions of "the Logger's context" in the
doc.
Change-Id: Ia6caa4f1ca70c1c4b0cab3e222b2fda48be73fef
Reviewed-on: https://go-review.googlesource.com/c/go/+/496175
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Add the slog static analysis pass to `go vet`.
Vendor in golang.org/x/tools@master to pick up the pass.
Tweak a test in slog to avoid triggering the vet check.
Change-Id: I55ceac9a4e6876c8385897784542761ea0af2481
Reviewed-on: https://go-review.googlesource.com/c/go/+/496156
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
It's possible that the replacement for a built-in attribute is a Group.
That would cause a nil pointer exception because the handleState.prefix
field isn't set until later, in appendNonBuiltIns.
So create the prefix field earlier, at the start of commonHandler.handle.
Once we do this, we can simplify the code by creating and freeing the
prefix in newHandleState.
Along the way I discovered a line that wasn't being tested:
state.prefix.WriteString(h.groupPrefix)
so I modified an existing test case to cover it.
Change-Id: Ib385e3c13451017cb093389fd5a1647d53e610bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/494037
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Test the built-in handlers using the testing/slogtest package.
Change-Id: I6464b6730293461c38b84fb2bf33cdd09173dd6e
Reviewed-on: https://go-review.googlesource.com/c/go/+/488255
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Add tags to the fields of Source that lower-cases their names for JSON.
The implementation still treats Source specially for performance, but
now the result would be identical if it did not.
Change-Id: I5fd2e500f1a301db62af87be8b877ecd954a26ec
Reviewed-on: https://go-review.googlesource.com/c/go/+/494035
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Remove the special-case handling of NaN and infinities from
appendJSONValue, making JSONHandler behave almost exactly like
a json.Encoder without HTML escaping.
The only differences are:
- Encoding errors are turned into strings, instead of causing the Handle method to fail.
- Values of type `error` are displayed as strings by calling their `Error` method.
Fixes#59345.
Change-Id: Id06bd952bbfef596e864bd5fd3f9f4f178f738c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/486855
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Move the "Attrs and Values" section lower. It describes an optimization;
the API it covers is not essential.
Also, move the brief section on Logger.With up to the first section.
It was in the "Groups" section but didn't belong there.
Change-Id: I0e36ef654e95f918d5b480566ec58d9990d26b40
Reviewed-on: https://go-review.googlesource.com/c/go/+/487856
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
The Group function takes a key and a ...any, which is converted
into attrs.
Fixes#59204.
Change-Id: Ib714365dcda2eda37863ce433f3dd8cf5eeda610
Reviewed-on: https://go-review.googlesource.com/c/go/+/487855
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
There is now one constructor function for each built-in handler, with
signature
NewXXXHandler(io.Writer, *HandlerOptions) *XXXHandler
Fixes#59339.
Change-Id: Ia02183c5ce0dc15c64e33ad05fd69bca09df2d2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/486415
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Leaked goroutines are the only explanation I can think of for excess
allocs in TestDiscard, and TestOutputRace is the only place I can see
where the log package leaks goroutines. Let's fix that leak and see if
it eliminates the TestDiscard flakes.
Fixes#58797 (maybe).
Change-Id: I2d54dcba3eb52bd10a62cd1c380131add6a2f651
Reviewed-on: https://go-review.googlesource.com/c/go/+/487356
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Add a struct called Source that holds the function, file and line
of a location in the program's source code.
When HandleOptions.AddSource is true, the ReplaceAttr function will
get an Attr whose key is SourceKey and whose value is a *Source.
We use *Source instead of Source to save an allocation. The pointer
and the value each cause one allocation up front: the pointer when it
is created, and the value when it is assigned to the `any` field of a
slog.Value (handle.go:283). If a ReplaceAttr function wanted to modify
a Source value, it would have to create a new slog.Value to return,
causing a second allocation, but the function can modify a *Source in
place.
TextHandler displays a Source as "file:line".
JSONHandler displays a Source as a group of its non-zero fields.
This replaces the previous design, where source location was always a
string with the format "file:line". The new design gives users more
control over how to output and consume source locations.
Fixes#59280.
Change-Id: I84475abd5ed83fc354b50e34325c7b246cf327c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/486376
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Remove calls to Value.Resolve from Record.AddAttrs, Record.Add and Logger.With.
Handlers must resolve values themselves; document that in Handler.
Call Value.Resolve in the built-in handlers.
Updates #59292.
Change-Id: I00ba2131be0b16e3b1a22741249fd6f81c3efde1
Reviewed-on: https://go-review.googlesource.com/c/go/+/486375
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
If a LogValue call panics, recover and return an error instead.
The error contains some stack information to make it easier to
find the problem. A number of people complained that panics
in fmt.Formatter.Format functions are hard to debug because
there is no context.
This is an example of the error text:
LogValue panicked
called from log/slog.panickingLogValue.LogValue (/usr/local/google/home/jba/repos/go/src/log/slog/value_test.go:221)
called from log/slog.Value.resolve (/usr/local/google/home/jba/repos/go/src/log/slog/value.go:465)
called from log/slog.Value.Resolve (/usr/local/google/home/jba/repos/go/src/log/slog/value.go:446)
called from log/slog.TestLogValue (/usr/local/google/home/jba/repos/go/src/log/slog/value_test.go:192)
called from testing.tRunner (/usr/local/google/home/jba/repos/go/src/testing/testing.go:1595)
(rest of stack elided)
Fixes#59141.
Change-Id: I62e6ff6968d1aa34873e955c2d606d25418a673b
Reviewed-on: https://go-review.googlesource.com/c/go/+/484097
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Record.Attrs stops as soon as its argument function returns false.
Fixes#59060.
Change-Id: I578d64635e0e52b0fcdbc57f6d5a27a6efac8c70
Reviewed-on: https://go-review.googlesource.com/c/go/+/484096
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Specify that Handlers should ignore zero-valued Attrs.
Implement that policy in the built-in handlers.
Fixes#59282.
Change-Id: I4430686b61f49bdac849ee300daaabfac9895849
Reviewed-on: https://go-review.googlesource.com/c/go/+/484095
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Replace the default Logger in some examples with a locally constructed
Logger.
Calling SetDefault changes global state that could affect other tests.
Although we could use a defer to restore the state, that clutters
the example and would not work if tests were run concurrently.
Change-Id: Ib2595c57f8e6c3e0b39b982f682ba287c2ae249d
Reviewed-on: https://go-review.googlesource.com/c/go/+/482475
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Before slog.SetDefault is called the first time, calls to slog's
default Logger invoke log's default Logger.
Originally, this was done by calling log.Output. This caused source
line information to be wrong sometimes, because log.Output requires a
call depth and the code invoking it could not know how many calls were
between it and the original logging call (slog.Info, etc.). The line
information would be right if the default handler was called directly,
but wrong if it was wrapped by another handler. The handler has the pc
of the logging call, but it couldn't give that pc to the log package.
This CL fixes the problem by adding a function in the log package
that uses the pc instead of a call depth, and making that function
available to slog.
The simplest way to add pc functionality to the log package is to add
a pc argument to Logger.output, which uses it only if it's not zero.
To make that function visible to slog without exporting it, we store
the function in a variable that lives in the new log/internal package.
Change-Id: I0bb6daebb4abc518a7ccc4e6d2f3c1093b1d0fe4
Reviewed-on: https://go-review.googlesource.com/c/go/+/482239
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
The existing error log in check doesn't report the got/want hostname
even though that can be the cause of the error. Log those as well.
While we're here, also report os.Hostname() errors.
For #59568.
Change-Id: Ia277f85eddc541f2e78d719bc731db24e4513754
Reviewed-on: https://go-review.googlesource.com/c/go/+/483915
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Fixes#58141
Co-authored-by: Richard Musiol <neelance@gmail.com>
Co-authored-by: Achille Roussel <achille.roussel@gmail.com>
Co-authored-by: Julien Fabre <ju.pryz@gmail.com>
Co-authored-by: Evan Phoenix <evan@phx.io>
Change-Id: I49b66946acc90fdf09ed9223096bfec9a1e5b923
Reviewed-on: https://go-review.googlesource.com/c/go/+/479627
Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Bypass: Ian Lance Taylor <iant@golang.org>
json.Marshal doesn't do what one might hope on many Go error values.
Errors created with errors.New marshal as "{}". So JSONHandler treats
errors specially, calling the Error method instead of json.Marshal.
However, if the error happens to implement json.Marshaler, then
JSONHandler should call json.Marshal after all. This CL makes
that change.
Change-Id: I2154246b2ca8fa13d4f6f1256f7a16aa98a8c24a
Reviewed-on: https://go-review.googlesource.com/c/go/+/480155
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Comparing two Values with == is sensitive to the internal
representation of Values, and may not correspond to
equality on the Go values they represent. For example,
StringValue("X") != StringValue(strings.ToUpper("x"))
because Go ends up doing a pointer comparison on the data
stored in the Values.
So make Values non-comparable by adding a non-comparable field.
Updates #56345.
Change-Id: Ieedbf454e631cda10bc6fcf470b57d3f1d2182cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/479516
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Format Group values like a []Attr, rather than a *Attr.
Also, use fmt.Append in Value.append.
Updates #56345.
Change-Id: I9db1a8ec47f8e99c1ac3225d78e152013116bff3
Reviewed-on: https://go-review.googlesource.com/c/go/+/479515
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Add a suite of benchmarks for the LogAttrs method, which is intended
to be fast.
Updates #56345.
Change-Id: If43f9f250bd588247c539bed87f81be7f5428c6d
Reviewed-on: https://go-review.googlesource.com/c/go/+/478200
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Give an example illustrating the problem with dots inside groups
or keys. Clarify that to fix it in general, you need to do more
than escape the keys, since that won't distinguish the group "a.b"
from the two groups "a" and "b".
Updates #56345.
Change-Id: Ide301899c548d50b0a1f18e93e93d6e11ad485cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/478199
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Sean Liao
Mateusz Poliwczak
Artem Teleshev
Joe Tsai
Jes Cok
Andy Pan
Jonathan Amsterdam
zikaeroh
Will Roden
Merrick Clay
Cuong Manh Le
Christopher Taylor
nilsocket
Bryan C. Mills
Michael Pratt
Johan Brandhorst-Satzkorn