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

Allow logs to go to stdout #676

Closed
hlascelles opened this issue Dec 14, 2021 · 14 comments · Fixed by #680
Closed

Allow logs to go to stdout #676

hlascelles opened this issue Dec 14, 2021 · 14 comments · Fixed by #680
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@hlascelles
Copy link

At present, logs go to stderr. For Datadog, this means they show up as Errors.

See https://docs.datadoghq.com/agent/docker/log/?tab=containerinstallation (screenshot below).

It would be good to be able to log to stdout natively in descheduler.

descheduler version: 0.22.1
descheduler chart version: 0.22.1

image

In action:

image

@hlascelles hlascelles added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 14, 2021
@damemi
Copy link
Contributor

damemi commented Dec 15, 2021

cc @ingvagabund

@ingvagabund
Copy link
Contributor

Checking https://github.com/kubernetes/klog/blob/main/klog.go there's no option for klog to write into stdout.

@ingvagabund
Copy link
Contributor

ingvagabund commented Dec 15, 2021

@hlascelles how does the datadog consume the data? Parsing container's stdout/stderr?

@damemi
Copy link
Contributor

damemi commented Dec 15, 2021

Just curious, what are we doing with this: https://github.com/kubernetes-sigs/descheduler/blob/e6314d2/cmd/descheduler/descheduler.go#L32

Which passes to this:
https://github.com/damemi/descheduler/blob/94888e653c6f8d0554f251256cd1dd9eafea4368/cmd/descheduler/app/server.go#L81

Seems like we are trying to specify the output as Stdout, but is that not respected anymore? It seems really odd that klog would not allow stdout

@ingvagabund
Copy link
Contributor

Based on https://github.com/spf13/cobra/blob/master/command.go#L247-L251:

// SetOut sets the destination for usage messages.
// If newOut is nil, os.Stdout is used.
func (c *Command) SetOut(newOut io.Writer) {

So only for the usage text.

@hlascelles
Copy link
Author

hlascelles commented Dec 16, 2021

@ingvagabund Yes, as I understand it the agent just streams the STDOUT and STDERR to the Datadog servers.

I have searched their docs (and the agent config) on a way to override the level, but I cannot find a way.

@ingvagabund
Copy link
Contributor

ingvagabund commented Dec 16, 2021

@hlascelles any way to verify the fix in #680 by building a container and having the datadog collect the logs?

@hlascelles
Copy link
Author

hlascelles commented Dec 20, 2021

I will have a look, thanks! In the meantime I see that Wordpress docker has the same issue: docker-library/wordpress#311 (comment)

@hlascelles
Copy link
Author

I am seeing:

docker build -t descheduler-fix . 
=>
Step 6/11 : RUN VERSION=${VERSION} make build.$ARCH
 ---> Running in 2c81f13f89a0
make: *** No rule to make target 'build.'.  Stop.

@hlascelles
Copy link
Author

I was never able to test this because of the build error above, sorry. I will try the new build.

Also, for the record, here is how (in Datadog) to change any stream with the first char as I,E,W to the correct status:

You will have to add a new Pipeline (go to Logs -> Configuration). Click on add a new pipeline, give it a name and click on create. Next, you will have to click on add processor for this pipeline and select grok parser. Set the following parsing rule:

level_rule %{regex("[IWEiwe]"):level}.*

Click on create button. This should now assign level attribute to your logs. Next, add another processor and select Status Remapper, and type level as the attribute, and click on create.

@damemi
Copy link
Contributor

damemi commented Jan 14, 2022

That is weird, I get the same error running docker build directly. However, running make image (which does a docker build) works for me. @hlascelles could you try make image instead?

@tamir-allc
Copy link

tamir-allc commented May 24, 2022

for some reason it still reproduces (Datadog user as well) on v0.23.1 and even v0.24.0.
We install it on a GKE cluster v1.21 using the official v0.24.0 Helm chart.

If I add the argument --logtostderr=false it works but that's gonna be deprecated and also misses the purpose of the fix.
Moreover, downloading the image locally and running it without flags, prints the following:

      --logtostderr                              log to standard error instead of files (default true) (DEPRECATED: will be removed in a future release, see https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components)

Am i missing something?

Opened a new bug #811.

@jmk47912204
Copy link

Hey Everyone,

Is there a way to fix this issue from descheduler itself rather than having workaround on the datadog level?

Thank you

@hlascelles
Copy link
Author

hlascelles commented Jun 23, 2024

I've never found a generic way, and I think there isn't one in this app. I've also found many other k8s utilities work in the same way. I've just added lots of Datadog parsing rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants