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

Refactor metadata generator to support adding metadata across resources #14875

Merged
merged 10 commits into from
Jan 14, 2020

Conversation

vjsamuel
Copy link
Contributor

@vjsamuel vjsamuel commented Dec 2, 2019

This PR attempts to refactor metadata generator to support enrichment of resources metadata on top of other resources.

Ex: enriching a pod object with information from a node and namespace object.

This PR does the same only for the autodiscover provider of kubernetes. It adds a configuration add_resource_metadata to allow specifying what resources should be used for enrichment.

Sample configuration:

metricbeat.autodiscover:
  providers:
    - type: kubernetes
      kube_config: ${HOME}/.kube/config
      resource: service
      annotations.dedot: false
      add_resource_metadata:
        namespace.enabled: true
      builders:
        - type: hints

output.console.pretty: true

this would ensure that we additionally add namespace metadata along with the service metadata.

pods can be enriched with node and namespace metadata. the namespace and node parameters of add_resource_metadata can take include_*, exlude_* configs similar to the parent resource.

closes #13873

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

1 similar comment
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

"github.com/elastic/beats/libbeat/logp"
)

func init() {
autodiscover.Registry.AddProvider("kubernetes", AutodiscoverBuilder)
}

type Eventer interface {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported type Eventer should have comment or be unexported

"github.com/elastic/beats/libbeat/logp"
)

func init() {
autodiscover.Registry.AddProvider("kubernetes", AutodiscoverBuilder)
}

type Eventer interface {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported type Eventer should have comment or be unexported

"github.com/elastic/beats/libbeat/logp"
)

func init() {
autodiscover.Registry.AddProvider("kubernetes", AutodiscoverBuilder)
}

type Eventer interface {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported type Eventer should have comment or be unexported

config *Config
}

func NewResourceMetadataGenerator(config *Config) *resource {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported func NewResourceMetadataGenerator returns unexported type *metadata.resource, which can be annoying to use
exported function NewResourceMetadataGenerator should have comment or be unexported

}

return p.Generate(po, opts...)
} else {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary)

resource *resource
}

func NewPodMetadataGenerator(cfg *Config, acfg *AddResourceMetadataConfig, client k8s.Interface, options kubernetes.WatchOptions) (MetaGen, error) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function NewPodMetadataGenerator should have comment or be unexported


type FieldOptions func(common.MapStr) error

func WithFields(key string, value interface{}) func(common.MapStr) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function WithFields should have comment or be unexported

Stop()
}

type FieldOptions func(common.MapStr) error

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported type FieldOptions should have comment or be unexported

}
}

func (c *Config) ApplyConfig(cfg *common.Config) error {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported method Config.ApplyConfig should have comment or be unexported

Namespace *common.Config `config:"config"`
}

func DefaultConfig() Config {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function DefaultConfig should have comment or be unexported

libbeat/common/kubernetes/metadata/config.go Show resolved Hide resolved
@@ -38,6 +38,10 @@ type MetaGenerator interface {
ContainerMetadata(pod *Pod, container string, image string) common.MapStr
}

type MetaGen interface {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported type MetaGen should have comment or be unexported

}
}

func NewInformer(client kubernetes.Interface, resource Resource, opts WatchOptions) (cache.SharedInformer, string, error) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function NewInformer should have comment or be unexported

}

return s.Generate(po, opts...)
} else {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary)

libbeat/common/kubernetes/metadata/service.go Show resolved Hide resolved

type resource = Config

func NewResourceMetadataGenerator(cfg *common.Config) *resource {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported function NewResourceMetadataGenerator should have comment or be unexported

libbeat/common/kubernetes/metadata/pod.go Show resolved Hide resolved
}

return n.Generate(no, opts...)
} else {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary)

libbeat/common/kubernetes/metadata/config.go Show resolved Hide resolved

if indexers != nil {
return cache.NewSharedIndexInformer(listwatch, resource, opts.SyncTimeout, indexers), objType, nil
} else {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if block ends with a return statement, so drop this else and outdent its block


}

// OnAdd does a no-op on a delete event

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment on exported method NoOpEventHandlerFuncs.OnDelete should be of the form "OnDelete ..."


}

// OnAdd does a no-op on an update event

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment on exported method NoOpEventHandlerFuncs.OnUpdate should be of the form "OnUpdate ..."

return &config
}

func (r *Resource) Generate(obj kubernetes.Resource, options ...FieldOptions) common.MapStr {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported method Resource.Generate should have comment or be unexported

"github.com/elastic/beats/libbeat/common/safemapstr"
)

type Resource = Config

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported type Resource should have comment or be unexported

return &config
}

func (r *Resource) Generate(obj kubernetes.Resource, options ...FieldOptions) common.MapStr {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported method Resource.Generate should have comment or be unexported

"github.com/elastic/beats/libbeat/common/safemapstr"
)

type Resource = Config

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported type Resource should have comment or be unexported

@vjsamuel vjsamuel changed the title Meta refactor Refactor metadata generator to support adding metadata across resources Dec 3, 2019
resource *Resource
}

// NewPodMetadataGenerator creates a metagen for namespace resources

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment on exported function NewNamespaceMetadataGenerator should be of the form "NewNamespaceMetadataGenerator ..."

resource *Resource
}

// NewPodMetadataGenerator creates a metagen for service resources

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment on exported function NewNodeMetadataGenerator should be of the form "NewNodeMetadataGenerator ..."

resource *Resource
}

// NewPodMetadataGenerator creates a metagen for namespace resources

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment on exported function NewNamespaceMetadataGenerator should be of the form "NewNamespaceMetadataGenerator ..."

resource *Resource
}

// NewPodMetadataGenerator creates a metagen for service resources

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment on exported function NewNodeMetadataGenerator should be of the form "NewNodeMetadataGenerator ..."

resource *Resource
}

// NewPodMetadataGenerator creates a metagen for service resources

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment on exported function NewServiceMetadataGenerator should be of the form "NewServiceMetadataGenerator ..."

resource *Resource
}

// NewPodMetadataGenerator creates a metagen for service resources

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment on exported function NewNodeMetadataGenerator should be of the form "NewNodeMetadataGenerator ..."

resource *Resource
}

// NewPodMetadataGenerator creates a metagen for service resources

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment on exported function NewServiceMetadataGenerator should be of the form "NewServiceMetadataGenerator ..."

@vjsamuel vjsamuel marked this pull request as ready for review December 3, 2019 08:11
@vjsamuel vjsamuel requested a review from a team as a code owner December 3, 2019 08:11
@vjsamuel
Copy link
Contributor Author

vjsamuel commented Dec 3, 2019

#14738 needs to be merged first

@vjsamuel
Copy link
Contributor Author

vjsamuel commented Dec 3, 2019

@exekias since this breaks the schema can you let me know places where that information need to be updated on Beats side?

@vjsamuel
Copy link
Contributor Author

vjsamuel commented Jan 9, 2020

@odacremolbap i have made the necessary changes. i will create the git issue at the time of merge if that is ok?

@odacremolbap
Copy link
Contributor

lgtm but there is still this request by @exekias #14875 (comment)

would you mindadding docs on the parameter?

@odacremolbap
Copy link
Contributor

would you mindadding docs on the parameter?

I missed that you already added those docs.
/lgtm

@odacremolbap odacremolbap self-requested a review January 13, 2020 11:50
@exekias
Copy link
Contributor

exekias commented Jan 13, 2020

ok to test

@exekias exekias merged commit dba8f74 into elastic:master Jan 14, 2020
exekias pushed a commit to exekias/beats that referenced this pull request Jan 14, 2020
…es (elastic#14875)

* Refactor metagen to allow multiple resources to be enriched

(cherry picked from commit dba8f74)
@exekias exekias added the v7.6.0 label Jan 14, 2020
exekias pushed a commit that referenced this pull request Jan 14, 2020
…es (#14875) (#15528)

* Refactor metagen to allow multiple resources to be enriched

(cherry picked from commit dba8f74)

Co-authored-by: Vijay Samuel <vjsamuel1990@gmail.com>
@odacremolbap odacremolbap mentioned this pull request Jan 15, 2020
19 tasks
@vjsamuel vjsamuel deleted the meta_refactor branch January 16, 2020 18:35
@odacremolbap odacremolbap added the test-plan-ok This PR passed manual testing label Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autodiscovery needs testing notes review Team:Integrations Label for the Integrations team test-plan Add this PR to be manual test plan test-plan-ok This PR passed manual testing v7.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add namespace labels to kubernetes metadata
7 participants