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.

Sign up for GitHub

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

Jump to bottom

log/slog: MarshalText and LogValue methods conflict when using slog.Info #59852

Closed
empire opened this issue Apr 26, 2023 · 3 comments
Closed

log/slog: MarshalText and LogValue methods conflict when using slog.Info #59852

empire opened this issue Apr 26, 2023 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.21

Comments

@empire
Copy link
Contributor

empire commented Apr 26, 2023

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

$ go version
go version devel go1.21-ce0b914312 Tue Apr 25 19:49:28 2023 +0000 darwin/arm64

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
GO111MODULE="on"
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/hossein/Library/Caches/go-build"
GOENV="/Users/hossein/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/hossein/.local/share/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/hossein/.local/share/go"
GOPRIVATE=""
GOPROXY=""
GOROOT="/Users/hossein/Projects/github.com/golang/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/hossein/Projects/github.com/golang/go/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="devel go1.21-ce0b914312 Tue Apr 25 19:49:28 2023 +0000"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/tmp/test/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/28/0tv7m8993k3859p1zrmyr0th0000gn/T/go-build3967094330=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I have an attribute that implements the MarshalText interface to customize its text representation. However, when I also define a LogValue method on the attribute to customize its log representation, the MarshalText method is not called anymore. This results in an incorrect output when I use slog.Info to log the attribute.

I don't know if it is correct to report this issue as a bug or not.

See https://go.dev/play/p/amo-8xYx_bh?v=gotip

Updates: #56345

What did you expect to see?

{"time":"2009-11-10T23:00:00Z","level":"INFO","msg":"sample","user":{"user_id":1,"user_name":"John Doe"}}
time=2009-11-10T23:00:00.000Z level=INFO msg=sample user="1:John Doe"

What did you see instead?

{"time":"2009-11-10T23:00:00Z","level":"INFO","msg":"sample","user":{"user_id":1,"user_name":"John Doe"}}
time=2009-11-10T23:00:00.000Z level=INFO msg=sample user.user_id=1 user.user_name="John Doe"
@seankhliao
Copy link
Member

cc @jba but I think this is working as expected
LogValue has the highest precedence

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 26, 2023
@dmitshur dmitshur added this to the Go1.21 milestone Apr 26, 2023
@empire
Copy link
Contributor Author

empire commented Apr 26, 2023

@seankhliao Thanks, for your reply.

I tried to remove the "// Special case: Source." comments from the handler.go file by adding MarshalText and LogValue to the Source struct to make the logic cleaner. I find the mentioned conflict. I'm not sure but I think if we refactor some parts of the handler module, it's possible to resolve the conflict as well.

@jba
Copy link
Contributor

jba commented May 1, 2023

@empire, if Source implements LogValue then a ReplaceAttr function won't ever get to see a Source, since values are resolved before being passed to ReplaceAttr. That is why it's a special case.

Regarding LogValue vs. MarshalText, it makes sense that LogValue takes precedence. LogValue is more specific: it says that you want the type to be logged in a certain way.

To get the behavior you want, either remove the LogValue method or define it to return a string value with the string you want.

To get different output for text and JSON, define MarshalText and MarshalJSON.

@jba jba closed this as completed May 1, 2023
@golang golang locked and limited conversation to collaborators Apr 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
5 participants
@empire @dmitshur @gopherbot @seankhliao @jba