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

The log format should be configurable or customizable #18

Open
xdlai opened this issue Jul 12, 2019 · 5 comments
Open

The log format should be configurable or customizable #18

xdlai opened this issue Jul 12, 2019 · 5 comments

Comments

@xdlai
Copy link

xdlai commented Jul 12, 2019

After import automaxprocs, the process will print "2019/07/09 14:18:22 maxprocs: Leaving GOMAXPROCS=2: CPU quota undefined" through STDOUT.
There is no information to identify this line is an INFO, WARNING or ERROR log.

If the log format is configurable or customizable, the service using automaxprocs can generate the log message properly.

@dpanic
Copy link

dpanic commented Jul 13, 2019

I agree

@prestonvanloon
Copy link

I would like to disable this log all together.

@prashantv
Copy link
Contributor

For the simple integration case (import _ "go.uber.org/automaxprocs"), it won't be possible to customize the behaviour as the initfunction ofautomaxprocs` would run before anything else could set the logger format, and it would require global configurations which will cause confusion if multiple packages try to set different values.

Instead, users who want customizations should use go.uber.org/automaxprocs/maxprocs and call Set with options. By default, maxprocs.Set doesn't log anything. You can customize where the output goes, but it's not possible to control the format. It probably makes sense to add a hook with information about what changes Set makes to allow users to log in any format.

I would like to disable this log all together.

You can use Set above which won't log. We wanted to log by default, as setting a global like GOMAXPROCS is a significant change which would be surprising if it was done silently because some package imported automaxprocs.

caarlos0 pushed a commit to goreleaser/goreleaser that referenced this issue Apr 2, 2023
<!--

Hi, thanks for contributing!

Please make sure you read our CONTRIBUTING guide.

Also, add tests and the respective documentation changes as well.

-->

Currently Goreleaser uses `runtime.NumCPU()` as the default value if
`--parallelism` is not set.
However, this will get the number of CPUs on the host even when
Goreleaser is run in a container with a limit on the maximum number of
CPUs that can be used (typically in a Kubernetes pod).
Actually, `docker run --cpus=1 goreleaser/goreleaser --debug` shows
`parallelism: 4` on my machine.
This behavior causes CPU throttling, which increases execution time and,
in the worst case, terminates with an error.
I ran into this problem with Jenkins where the agent runs on pod
([Kubernetes plugin for
Jenkins](https://plugins.jenkins.io/kubernetes/)).

This commit introduces
[automaxprocs](https://github.com/uber-go/automaxprocs) to fix this
issue.
This library sets `GOMAXPROCS` to match Linux container CPU quota.
I have also looked for a library that can get CPU quota more directly,
but this seems to be the best I could find.
The reason it is set in a different notation from the automaxprocs
README is to prevent logs from being displayed
([comment](uber-go/automaxprocs#18 (comment))).

I would have liked to write a test, but this change is dependent on the
number of CPUs in the execution environment, so I could not.
Instead, I wrote a Dockerfile for testing

```Dockerfile
FROM golang:1.20.2

WORKDIR /go/app
RUN sh -c "$(curl --location https://taskfile.dev/install.sh)" -- -d -b /usr/local/bin
COPY . .
RUN task build
```

and confirmed built binary shows expected parallelism by following
commands:

```sh
docker build --file Dockerfile.test . -t test-goreleaser
docker run --cpus=1 test-goreleaser ./goreleaser build --snapshot --debug # parallelism: 1
docker run test-goreleaser ./goreleaser build --snapshot --debug # parallelism: 4
```

I also ran the built binary on my Macbook and it was fine.
ruudk added a commit to ruudk/frankenphp that referenced this issue Oct 9, 2023
dunglas pushed a commit to dunglas/frankenphp that referenced this issue Oct 9, 2023
@dolmen
Copy link

dolmen commented Nov 9, 2023

The simple answer to this request is: use package go.uber.org/automaxprocs/maxprocs instead of go.uber.org/automaxprocs.

So the remaining issue is about documentation: go.uber.org/automaxprocs/maxprocs is not mentioned in the package documentation for automaxprocs.

joshklop added a commit to NethermindEth/juno that referenced this issue Jan 4, 2024
When imported using the blank identifier `_`, the automaxprocs package emits
a log after it changes GOMAXPROCS. According to [1], the authors
   wanted to log by default, as setting a global like GOMAXPROCS is a
   significant change which would be surprising if it was done silently
   because some package imported automaxprocs.
Since no one should be importing `cmd/juno` and because we want our log
messages to be consistent, we disable the log by calling Set() explicitly.
[1] uber-go/automaxprocs#18
joshklop added a commit to NethermindEth/juno that referenced this issue Jan 4, 2024
When imported using the blank identifier `_`, the automaxprocs package
emits a log after it changes GOMAXPROCS. According to [1], the authors

   wanted to log by default, as setting a global like GOMAXPROCS is a
   significant change which would be surprising if it was done silently
   because some package imported automaxprocs.

Since no one should be importing `cmd/juno` and because we want our log
messages to be consistent, we disable the log by calling Set()
explicitly.

[1] uber-go/automaxprocs#18
joshklop added a commit to NethermindEth/juno that referenced this issue Jan 4, 2024
When imported using the blank identifier `_`, the automaxprocs package
emits a log after it changes GOMAXPROCS. According to [1], the authors

   wanted to log by default, as setting a global like GOMAXPROCS is a
   significant change which would be surprising if it was done silently
   because some package imported automaxprocs.

Since no one should be importing `cmd/juno` and because we want our log
messages to be consistent, we disable the log by calling Set()
explicitly.

[1] uber-go/automaxprocs#18
joshklop added a commit to NethermindEth/juno that referenced this issue Jan 11, 2024
When imported using the blank identifier `_`, the automaxprocs package
emits a log after it changes GOMAXPROCS. According to [1], the authors

   wanted to log by default, as setting a global like GOMAXPROCS is a
   significant change which would be surprising if it was done silently
   because some package imported automaxprocs.

Since no one should be importing `cmd/juno` and because we want our log
messages to be consistent, we disable the log by calling Set()
explicitly.

[1] uber-go/automaxprocs#18
omerfirmak pushed a commit to NethermindEth/juno that referenced this issue Jan 12, 2024
When imported using the blank identifier `_`, the automaxprocs package
emits a log after it changes GOMAXPROCS. According to [1], the authors

   wanted to log by default, as setting a global like GOMAXPROCS is a
   significant change which would be surprising if it was done silently
   because some package imported automaxprocs.

Since no one should be importing `cmd/juno` and because we want our log
messages to be consistent, we disable the log by calling Set()
explicitly.

[1] uber-go/automaxprocs#18
@tzq0301
Copy link

tzq0301 commented Apr 28, 2024

Manually calling the maxprocs.Set(..) like

maxprocs.Set(maxprocs.Logger(log.Printf))
works for me:

package main

import (
	"runtime"
	
	"go.uber.org/automaxprocs/maxprocs"
)

func main() {
	maxprocs.Set()  // <- here 

	println("GOMAXPROCS: ", runtime.GOMAXPROCS(0))
	println("NumCPU:     ", runtime.NumCPU())
}

test with:

docker run --rm $(docker build . -q)
#docker run --rm --cpus="2" $(docker build . -q)

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

No branches or pull requests

6 participants