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.

Sign up for GitHub

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

WIP - multiple templates #9247

Closed
wants to merge 4 commits into from
Closed

WIP - multiple templates #9247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3d5beb0
WIP multiple templates
simitt Nov 23, 2018
33d118e
WIP remove fields from beat
simitt Nov 23, 2018
49ca0ad
backwardscompatible load config; export cmd uses loader
simitt Nov 26, 2018
7ff9c68
Example config.
simitt Nov 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion libbeat/asset/registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"bytes"
"compress/zlib"
"encoding/base64"
"errors"
"fmt"
"io/ioutil"
)

Expand All @@ -42,7 +44,7 @@ func SetFields(beat, name string, asset func() string) error {
return nil
}

// GetFields returns a byte array contains all fields for the given beat
// GetFields returns a byte array containing all fields for the given beat
func GetFields(beat string) ([]byte, error) {
var fields []byte
for _, data := range FieldsRegistry[beat] {
Expand All @@ -56,6 +58,25 @@ func GetFields(beat string) ([]byte, error) {
return fields, nil
}

// GetFieldsFor returns a byte array containing all fields for the given beat
// and given names
func GetFieldsFor(beat string, names []string) ([]byte, error) {
var fields []byte
for _, name := range names {
data, ok := FieldsRegistry[beat][name]
Copy link
Contributor

Choose a reason for hiding this comment

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

An idea I just had: What if we can allow to define dependencies when registering fields. For example the system fields depend on the common fields. And common fields depend on the ecs fields. Like this when generating the fields we could loop through the dependencies and get always the full result.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll look into that when implementing it, but generally not planning on making any dependencies configurable.

Copy link
Contributor

Choose a reason for hiding this comment

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

No need to look into it now. I think as you mentioned, outside of the scope of this PR.

if !ok {
return nil, errors.New(fmt.Sprintf("No fields available for %s", name))
Copy link

Choose a reason for hiding this comment

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

should replace errors.New(fmt.Sprintf(...)) with fmt.Errorf(...)

}
output, err := DecodeData(data)
if err != nil {
return nil, err
}

fields = append(fields, output...)
}
return fields, nil
}

// EncodeData compresses the data with zlib and base64 encodes it
func EncodeData(data string) (string, error) {
var zlibBuf bytes.Buffer
Expand Down
2 changes: 1 addition & 1 deletion libbeat/beat/beat.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ type Beat struct {

BeatConfig *common.Config // The beat's own configuration section

Fields []byte // Data from fields.yml
//Fields []byte // Data from fields.yml
Copy link

Choose a reason for hiding this comment

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

Why remove this? The libbeat/beat package is supposed to be the (most...) stable API beats devs are facing/interacting with. We should try to reduce the change of breakages... Maybe we've been better off if Fields would have been an interface or not been added at all to the Beats structure.

Was Fields even used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Afaics Fields was only used for creating the template, so it is not necessary any more with those changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case you consider removing this a breaking change, I can leave it. It wouldn't be used any more though, so to avoid confusion and errors, I prefer removing it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I did a quick look, I don't see it used outside of creating the template.
So it introduce a breaking change on the beat struct, we should have defined an interface for this to limit the risk.

Lets say that I've used Fields in my code how I move it to the new workflow?
Or How do I have access to the same data?


ConfigManager management.ConfigManager // config manager
}
Expand Down
34 changes: 5 additions & 29 deletions libbeat/cmd/export/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,18 @@ import (
"github.com/spf13/cobra"

"github.com/elastic/beats/libbeat/cmd/instance"
"github.com/elastic/beats/libbeat/common"
"github.com/elastic/beats/libbeat/paths"
"github.com/elastic/beats/libbeat/template"
)

func GenTemplateConfigCmd(settings instance.Settings, name, idxPrefix, beatVersion string) *cobra.Command {
genTemplateConfigCmd := &cobra.Command{
Use: "template",
Short: "Export index template to stdout",
Short: "Export index template(s) to stdout",
Run: func(cmd *cobra.Command, args []string) {
version, _ := cmd.Flags().GetString("es.version")
index, _ := cmd.Flags().GetString("index")

b, err := instance.NewBeat(name, idxPrefix, beatVersion)
b, err := instance.NewBeat(name, index, version)
if err != nil {
fmt.Fprintf(os.Stderr, "Error initializing beat: %s\n", err)
os.Exit(1)
Expand All @@ -48,39 +46,17 @@ func GenTemplateConfigCmd(settings instance.Settings, name, idxPrefix, beatVersi
os.Exit(1)
}

cfg := template.DefaultConfig
if b.Config.Template.Enabled() {
err = b.Config.Template.Unpack(&cfg)
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting template settings: %+v", err)
os.Exit(1)
}
}

tmpl, err := template.New(b.Info.Version, index, version, cfg)
loader, err := template.NewConsoleLoader(b.Config.Template, b.Info, version)
if err != nil {
fmt.Fprintf(os.Stderr, "Error generating template: %+v", err)
fmt.Fprintf(os.Stderr, "Error generating template loader: %+v", err)
os.Exit(1)
}

var templateString common.MapStr
if cfg.Fields != "" {
fieldsPath := paths.Resolve(paths.Config, cfg.Fields)
templateString, err = tmpl.LoadFile(fieldsPath)
} else {
templateString, err = tmpl.LoadBytes(b.Fields)
}

err = loader.Load()
if err != nil {
fmt.Fprintf(os.Stderr, "Error generating template: %+v", err)
os.Exit(1)
}

_, err = os.Stdout.WriteString(templateString.StringToPrint() + "\n")
if err != nil {
fmt.Fprintf(os.Stderr, "Error writing template: %+v", err)
os.Exit(1)
}
},
}

Expand Down
19 changes: 7 additions & 12 deletions libbeat/cmd/instance/beat.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import (
"go.uber.org/zap"

"github.com/elastic/beats/libbeat/api"
"github.com/elastic/beats/libbeat/asset"
"github.com/elastic/beats/libbeat/beat"
"github.com/elastic/beats/libbeat/cfgfile"
"github.com/elastic/beats/libbeat/cloudid"
Expand Down Expand Up @@ -196,11 +195,6 @@ func NewBeat(name, indexPrefix, v string) (*Beat, error) {
return nil, err
}

fields, err := asset.GetFields(name)
if err != nil {
return nil, err
}

id, err := uuid.NewV4()
if err != nil {
return nil, err
Expand All @@ -215,7 +209,7 @@ func NewBeat(name, indexPrefix, v string) (*Beat, error) {
Hostname: hostname,
UUID: id,
},
Fields: fields,
//Fields: fields,
Copy link
Contributor

Choose a reason for hiding this comment

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

We should remove this.. following the change in the struct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, once we decided to remove in the struct, I'll also remove it here. ATM both is commented out as discussions are going on.

}

return &Beat{Beat: b}, nil
Expand Down Expand Up @@ -719,7 +713,7 @@ func (b *Beat) loadDashboards(ctx context.Context, force bool) error {
// the elasticsearch output. It is important the the registration happens before
// the publisher is created.
func (b *Beat) registerTemplateLoading() error {
var cfg template.TemplateConfig
var cfg template.TemplatesConfig

// Check if outputting to file is enabled, and output to file if it is
if b.Config.Template.Enabled() {
Expand All @@ -741,13 +735,14 @@ func (b *Beat) registerTemplateLoading() error {
return err
}

if esCfg.Index != "" && (cfg.Name == "" || cfg.Pattern == "") && (b.Config.Template == nil || b.Config.Template.Enabled()) {
//TODO: what to do with this check with Indices?
if esCfg.Index != "" && (len(cfg.Templates) == 0 || cfg.Templates[0].Name == "" || cfg.Templates[0].Pattern == "") && (b.Config.Template == nil || b.Config.Template.Enabled()) {
return fmt.Errorf("setup.template.name and setup.template.pattern have to be set if index name is modified.")
}

if b.Config.Template == nil || (b.Config.Template != nil && b.Config.Template.Enabled()) {

// load template through callback to make sure it is also loaded
// load templates through callback to make sure it is also loaded
// on reconnecting
callback, err := b.templateLoadingCallback()
if err != nil {
Expand All @@ -760,14 +755,14 @@ func (b *Beat) registerTemplateLoading() error {
return nil
}

// Build and return a callback to load index template into ES
// Build and return a callback to load index templates into ES
func (b *Beat) templateLoadingCallback() (func(esClient *elasticsearch.Client) error, error) {
callback := func(esClient *elasticsearch.Client) error {
if b.Config.Template == nil {
b.Config.Template = common.NewConfig()
}

loader, err := template.NewLoader(b.Config.Template, esClient, b.Info, b.Fields)
loader, err := template.NewESLoader(b.Config.Template, esClient, b.Info)
if err != nil {
return fmt.Errorf("Error creating Elasticsearch template loader: %v", err)
}
Expand Down
26 changes: 0 additions & 26 deletions libbeat/cmd/instance/setup.go
View file
Open in desktop

This file was deleted.

67 changes: 57 additions & 10 deletions libbeat/template/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,61 @@

package template

import "github.com/elastic/beats/libbeat/common"
import (
"github.com/elastic/beats/libbeat/common"
)

func Unpack(c *common.Config) (*TemplatesConfig, error) {
Copy link

Choose a reason for hiding this comment

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

exported function Unpack should have comment or be unexported

Copy link

Choose a reason for hiding this comment

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

We need to export this?

Why no func (tc *TemplatesConfig) Unpack(c *common.Config) error ? This would be used by go-ucfg.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically because I didn't see a lot of value in manually creating an empty TemplatesConfig outside and then call the unpack function on it. But I see how this makes sense from a consistency point, so will change.

var templatesRaw = struct {
Templates []*common.Config `config:"templates"`
}{}

if err := c.Unpack(&templatesRaw); err != nil {
return nil, err
}

var tc TemplatesConfig

// use `settings.template.templates` if configured
for _, t := range templatesRaw.Templates {
var tmplCfg = DefaultTemplateCfg()
t.Unpack(&tmplCfg)
tc.Templates = append(tc.Templates, tmplCfg)
}

// fallback if no `settings.template.templates` was configured
if len(tc.Templates) == 0 {
var tmplCfg = DefaultTemplateCfg()
c.Unpack(&tmplCfg)
tc.Templates = append(tc.Templates, tmplCfg)
}

return &tc, nil
}

type TemplateConfig struct {
Enabled bool `config:"enabled"`
AppendFields common.Fields `config:"append_fields"`

Name string `config:"name"`
Pattern string `config:"pattern"`
Fields string `config:"fields"`
Modules string `config:"modules"`
JSON struct {
Enabled bool `config:"enabled"`
Path string `config:"path"`
Name string `config:"name"`
} `config:"json"`
AppendFields common.Fields `config:"append_fields"`
Overwrite bool `config:"overwrite"`
Settings TemplateSettings `config:"settings"`

//TODO: check for overwrites
Settings TemplateSettings `config:"settings"`
Enabled bool `config:"enabled"`
Overwrite bool `config:"overwrite"`
}

type TemplatesConfig struct {
Copy link

Choose a reason for hiding this comment

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

exported type TemplatesConfig should have comment or be unexported

Templates []TemplateConfig `config:"-"`

Settings TemplateSettings `config:"settings"`
}

type TemplateSettings struct {
Expand All @@ -40,9 +80,16 @@ type TemplateSettings struct {
}

var (
// DefaultConfig for index template
DefaultConfig = TemplateConfig{
Enabled: true,
Fields: "",
}
// Defaults used in the template
defaultDateDetection = false
defaultTotalFieldsLimit = 10000
defaultNumberOfRoutingShards = 30
)

func DefaultTemplateCfg() TemplateConfig {
Copy link

Choose a reason for hiding this comment

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

exported function DefaultTemplateCfg should have comment or be unexported

Copy link

Choose a reason for hiding this comment

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

We need to export this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, will change.

return TemplateConfig{
Enabled: true,
Overwrite: false,
Fields: "",
}
}
Loading