-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Upgrade elastic-agent-autodiscover dependency; use InitDefaults() to init default configuration #37458
Upgrade elastic-agent-autodiscover dependency; use InitDefaults() to init default configuration #37458
Conversation
…metadata config - elastic/elastic-agent-autodiscover#72 Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
This pull request doesn't have a |
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
go.mod
Outdated
@@ -405,7 +405,7 @@ replace ( | |||
github.com/docker/go-plugins-helpers => github.com/elastic/go-plugins-helpers v0.0.0-20200207104224-bdf17607b79f | |||
github.com/dop251/goja => github.com/andrewkroh/goja v0.0.0-20190128172624-dd2ac4456e20 | |||
github.com/dop251/goja_nodejs => github.com/dop251/goja_nodejs v0.0.0-20171011081505-adff31b136e6 | |||
|
|||
github.com/elastic/elastic-agent-autodiscover => /Users/tetianakravchenko/go/src/github.com/elastic/elastic-agent-autodiscover |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: replace with elastic/elastic-agent-autodiscover#72 when released
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder: We need the same for elastic-agent repo: https://github.com/elastic/elastic-agent/blob/main/go.mod#L15
Scope: "node", | ||
AddResourceMetadata: metadata.GetDefaultResourceMetadataConfig(), | ||
} | ||
func (c *kubeAnnotatorConfig) InitDefaults() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refers to the Unpack
https://github.com/elastic/go-ucfg/blob/main/reify.go#L86-L87
💔 Build Failed
Expand to view the summary
Build stats
Test stats 🧪
Steps errorsExpand to view the steps failures
|
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
❕ Build Aborted
Expand to view the summary
Build stats
Test stats 🧪
Test errorsExpand to view the tests failures
|
@@ -161,13 +161,7 @@ func NewResourceMetadataEnricher( | |||
return &nilEnricher{} | |||
} | |||
|
|||
// GetPodMetaGen requires cfg of type Config | |||
commonMetaConfig := metadata.Config{} | |||
if err := base.Module().UnpackConfig(&commonMetaConfig); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might miss smth here, but I am not sure why it was used here. cfg
is used later to pass it to metadata.GetPodMetaGen
, metadata.NewServiceMetadataGenerator
and metadata.NewNamespaceAwareResourceMetadataGenerator
, as result we get the default metadata.Config
:
All those functions are using cfg
in NewResourceMetadataGenerator
- where in the end happens the same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MichaelKatsoulis as discussed - I've reverted this change and added missing metadata enrichment parameters to the configs 0215971
@tetianakravchenko can you update the description with what is the motivation of this PR changes. Was it driven from an issue or another PR ? |
@MichaelKatsoulis done, main reason - elastic/elastic-agent#3636 |
…a configuration on the metricset level Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
return &nilEnricher{} | ||
} | ||
cfg, _ := conf.NewConfigFrom(&commonMetaConfig) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you just moved the cfg creation higher in the code?
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
❕ Build Aborted
Expand to view the summary
Build stats
Test stats 🧪
Steps errorsExpand to view the steps failures
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding approval for files owned by the Elastic Agent team.
💚 Build Succeeded
Expand to view the summary
Build stats
❕ Flaky test reportNo test was executed to be analysed. 🤖 GitHub commentsExpand to view the GitHub comments
To re-run your PR in the CI, just comment with:
|
…init default configuration (#37458) * use InitDefaults(); elastic-agent-autodiscover: fix default resource metadata config - elastic/elastic-agent-autodiscover#72 Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * revert unintended changes Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * upgrade elastic-agent-autodiscover lib to v0.6.6 Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * run make update; fix linter issues Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * revert changes related to the 'commonMetaConfig'; add missing metadata configuration on the metricset level Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * x-pack/metricbeat: run make update Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> --------- Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> (cherry picked from commit 8c4a40f) # Conflicts: # NOTICE.txt # go.mod # go.sum
…init default configuration (#37458) * use InitDefaults(); elastic-agent-autodiscover: fix default resource metadata config - elastic/elastic-agent-autodiscover#72 Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * revert unintended changes Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * upgrade elastic-agent-autodiscover lib to v0.6.6 Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * run make update; fix linter issues Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * revert changes related to the 'commonMetaConfig'; add missing metadata configuration on the metricset level Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * x-pack/metricbeat: run make update Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> --------- Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> (cherry picked from commit 8c4a40f) # Conflicts: # go.sum
…; use InitDefaults() to init default configuration (#37512) * Upgrade elastic-agent-autodiscover dependency; use InitDefaults() to init default configuration (#37458) * use InitDefaults(); elastic-agent-autodiscover: fix default resource metadata config - elastic/elastic-agent-autodiscover#72 Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * revert unintended changes Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * upgrade elastic-agent-autodiscover lib to v0.6.6 Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * run make update; fix linter issues Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * revert changes related to the 'commonMetaConfig'; add missing metadata configuration on the metricset level Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * x-pack/metricbeat: run make update Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> --------- Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> (cherry picked from commit 8c4a40f) # Conflicts: # go.sum * fix merge conflicts Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * Revert "fix merge conflicts" This reverts commit 88c08fc. * fix conflicts only in the go.sum Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> --------- Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> Co-authored-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
…; use InitDefaults() to init default configuration (#37511) * Upgrade elastic-agent-autodiscover dependency; use InitDefaults() to init default configuration (#37458) * use InitDefaults(); elastic-agent-autodiscover: fix default resource metadata config - elastic/elastic-agent-autodiscover#72 Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * revert unintended changes Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * upgrade elastic-agent-autodiscover lib to v0.6.6 Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * run make update; fix linter issues Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * revert changes related to the 'commonMetaConfig'; add missing metadata configuration on the metricset level Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * x-pack/metricbeat: run make update Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> --------- Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> (cherry picked from commit 8c4a40f) # Conflicts: # NOTICE.txt # go.mod # go.sum * fix conflicts Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * update indirect dependencies Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * Revert "update indirect dependencies" This reverts commit 7cd0de8. * revert accidently removed garble_macho_executable binary Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> --------- Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> Co-authored-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
…init default configuration (elastic#37458) * use InitDefaults(); elastic-agent-autodiscover: fix default resource metadata config - elastic/elastic-agent-autodiscover#72 Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * revert unintended changes Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * upgrade elastic-agent-autodiscover lib to v0.6.6 Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * run make update; fix linter issues Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * revert changes related to the 'commonMetaConfig'; add missing metadata configuration on the metricset level Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * x-pack/metricbeat: run make update Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> --------- Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Proposed commit message
kubeAnnotatorConfig
add_resource_metadata
configuration.InitDefaults
fromgo-ucfg
to ensure that the default config is always used before unpacking configuration, similar to Fix NewContainerMetadataEnricher to use default config for kubernetes module. #16857include_labels
, etc can be used on the metricset configuration levelChecklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Author's Checklist
How to test this PR locally
Related issues
Use cases
AddResourceMetadata
is used for 3 different components:config:
config:
filebeat config:
metricbeat - this processor does not work:
TODO: update documentation and remove this processor from the metricbeat
Screenshots
Logs