Improve some logging messages for add_kubernetes_metadata processor #16866
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
flaper87
force-pushed
the
fp/kubernetes-logging
branch
2 times, most recently
from
7d58101
to
a41cb96
Compare
andresrc
added
[zube]: Inbox
Team:Platforms
|
Pinging @elastic/integrations-platforms (Team:Platforms) |
blakerouse
reviewed
Mar 6, 2020
There was a problem hiding this comment.
@flaper87 Change looks good. Need to add a changelog entry.
Switch from Debug to Error when unrecoveral events happen and add extra debug messages when indexing and matching pods.
flaper87
force-pushed
the
fp/kubernetes-logging
branch
from
a41cb96
to
3715681
Compare
Done, thanks for the review :) |
kaiyan-sheng
reviewed
Mar 6, 2020
| @@ -86,7 +86,7 @@ func (f *LogPathMatcher) MetadataIndex(event common.MapStr) string { | |||
| logp.Debug("kubernetes", "Incoming log.file.path value: %s", source) | |||
|
|
|||
| if !strings.Contains(source, f.LogsPath) { | |||
| logp.Debug("kubernetes", "Error extracting container id - source value does not contain matcher's logs_path '%s'.", f.LogsPath) | |||
| logp.Err("Error extracting container id - source value does not contain matcher's logs_path '%s'.", f.LogsPath) | |||
There was a problem hiding this comment.
We are in process of refactoring log related like logp.Err in #15699. I can create a separate PR for this after this PR gets merged.
kaiyan-sheng
approved these changes
Mar 6, 2020
|
I can't check the failures in great detail right now. Are these related to the PR or can I send the PR through? |
|
@flaper87 they seem unrelated |
exekias
added
the
needs_backport
5 tasks
exekias
added
v7.7.0
and removed
needs_backport
exekias
pushed a commit
to exekias/beats
that referenced
this pull request
Mar 9, 2020
…lastic#16866) Switch from Debug to Error when unrecoveral events happen and add extra debug messages when indexing and matching pods. (cherry picked from commit 1d6323f)
|
backport was missing, I've opened it: #16893 |
kaiyan-sheng
pushed a commit
that referenced
this pull request
Mar 17, 2020
…16866) (#16893) Switch from Debug to Error when unrecoveral events happen and add extra debug messages when indexing and matching pods. (cherry picked from commit 1d6323f) Co-authored-by: Flavio Percoco <[email protected]>
rdner
added a commit
to rdner/beats
that referenced
this pull request
Nov 16, 2022
This was happening due to the error level logging when the log path matcher detected a `log.file.path` that does not start with a standard Docker container log folder `/var/lib/docker/containers` because AKS dropped support for Docker in September 2022 and switched to containerd. It looks like this message was not supposed to be on the error level in the first place since it just means that the matcher didn't match and it's not an error. But it was mistakenly promoted from the debug level in elastic#16866 most likely because the message started with `Error` and looked confusing. This a partial fix to unblock our customers, but we still need to come up with the full AKS/containerd support in a follow up change.
rdner
added a commit
that referenced
this pull request
Nov 16, 2022
This was happening due to the error level logging when the log path matcher detected a `log.file.path` that does not start with a standard Docker container log folder `/var/lib/docker/containers` because AKS dropped support for Docker in September 2022 and switched to containerd. It looks like this message was not supposed to be on the error level in the first place since it just means that the matcher didn't match and it's not an error. But it was mistakenly promoted from the debug level in #16866 most likely because the message started with `Error` and looked confusing. This a partial fix to unblock our customers, but we still need to come up with the full AKS/containerd support in a follow up change.
mergify bot
pushed a commit
that referenced
this pull request
Nov 16, 2022
This was happening due to the error level logging when the log path matcher detected a `log.file.path` that does not start with a standard Docker container log folder `/var/lib/docker/containers` because AKS dropped support for Docker in September 2022 and switched to containerd. It looks like this message was not supposed to be on the error level in the first place since it just means that the matcher didn't match and it's not an error. But it was mistakenly promoted from the debug level in #16866 most likely because the message started with `Error` and looked confusing. This a partial fix to unblock our customers, but we still need to come up with the full AKS/containerd support in a follow up change. (cherry picked from commit 29f0b4c)
mergify bot
pushed a commit
that referenced
this pull request
Nov 16, 2022
This was happening due to the error level logging when the log path matcher detected a `log.file.path` that does not start with a standard Docker container log folder `/var/lib/docker/containers` because AKS dropped support for Docker in September 2022 and switched to containerd. It looks like this message was not supposed to be on the error level in the first place since it just means that the matcher didn't match and it's not an error. But it was mistakenly promoted from the debug level in #16866 most likely because the message started with `Error` and looked confusing. This a partial fix to unblock our customers, but we still need to come up with the full AKS/containerd support in a follow up change. (cherry picked from commit 29f0b4c)
rdner
added a commit
that referenced
this pull request
Nov 17, 2022
This was happening due to the error level logging when the log path matcher detected a `log.file.path` that does not start with a standard Docker container log folder `/var/lib/docker/containers` because AKS dropped support for Docker in September 2022 and switched to containerd. It looks like this message was not supposed to be on the error level in the first place since it just means that the matcher didn't match and it's not an error. But it was mistakenly promoted from the debug level in #16866 most likely because the message started with `Error` and looked confusing. This a partial fix to unblock our customers, but we still need to come up with the full AKS/containerd support in a follow up change. (cherry picked from commit 29f0b4c) Co-authored-by: Denis <[email protected]>
rdner
added a commit
that referenced
this pull request
Nov 17, 2022
This was happening due to the error level logging when the log path matcher detected a `log.file.path` that does not start with a standard Docker container log folder `/var/lib/docker/containers` because AKS dropped support for Docker in September 2022 and switched to containerd. It looks like this message was not supposed to be on the error level in the first place since it just means that the matcher didn't match and it's not an error. But it was mistakenly promoted from the debug level in #16866 most likely because the message started with `Error` and looked confusing. This a partial fix to unblock our customers, but we still need to come up with the full AKS/containerd support in a follow up change. (cherry picked from commit 29f0b4c) Co-authored-by: Denis <[email protected]>
chrisberkhout
pushed a commit
that referenced
this pull request
Jun 1, 2023
This was happening due to the error level logging when the log path matcher detected a `log.file.path` that does not start with a standard Docker container log folder `/var/lib/docker/containers` because AKS dropped support for Docker in September 2022 and switched to containerd. It looks like this message was not supposed to be on the error level in the first place since it just means that the matcher didn't match and it's not an error. But it was mistakenly promoted from the debug level in #16866 most likely because the message started with `Error` and looked confusing. This a partial fix to unblock our customers, but we still need to come up with the full AKS/containerd support in a follow up change.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Switch from Debug to Error when unrecoveral events happen and add extra debug messages when indexing and matching pods.
I was trying to debug why my configs wouldn't work and I was forced to enable debug to notice that the
add_kubernetes_metadataprocessor was failing because my configs were wrong.Sadly, my configs weren't working even after I fixed the above and it was a bit painful to figure out what was going on because there weren't enough debug messages to understand what index keys, matches, and metadata was being processed by filebeat.
Why is it important?
It makes operations and debugging easier
Checklist