[metricbeat] [auditbeat] Add formatted index option to metricbeat / auditbeat modules #15100
Conversation
faec
added
Metricbeat
|
|
||
| // KeepNull determines whether published events will keep null values or omit them. | ||
| KeepNull bool `config:"keep_null"` | ||
|
|
||
| common.EventMetadata `config:",inline"` // Fields and tags to add to events. | ||
| } | ||
|
|
||
| func NewConnector(pipeline beat.Pipeline, c *common.Config, dynFields *common.MapStrPointer) (*Connector, error) { | ||
| func NewConnector( |
There was a problem hiding this comment.
exported function NewConnector should have comment or be unexported
|
|
||
| // KeepNull determines whether published events will keep null values or omit them. | ||
| KeepNull bool `config:"keep_null"` | ||
|
|
||
| common.EventMetadata `config:",inline"` // Fields and tags to add to events. | ||
| } | ||
|
|
||
| func NewConnector(pipeline beat.Pipeline, c *common.Config, dynFields *common.MapStrPointer) (*Connector, error) { | ||
| func NewConnector( |
There was a problem hiding this comment.
exported function NewConnector should have comment or be unexported
| } | ||
| procs.AddProcessors(*userProcs) | ||
|
|
||
| return procs, nil |
There was a problem hiding this comment.
Nit: I quite liked the comments you had in the Filebeat implementation, numbering the processor additions and also noting that ordering was significant.
There was a problem hiding this comment.
Ah, I removed it because metricbeat processor config is simpler (only one configurable processor source), but you're right, an indication of intent here is good so I re-added a comment :-)
|
|
||
| // KeepNull determines whether published events will keep null values or omit them. | ||
| KeepNull bool `config:"keep_null"` | ||
|
|
||
| common.EventMetadata `config:",inline"` // Fields and tags to add to events. | ||
| } | ||
|
|
||
| func NewConnector(pipeline beat.Pipeline, c *common.Config, dynFields *common.MapStrPointer) (*Connector, error) { | ||
| func NewConnector( | ||
| beatInfo beat.Info, pipeline beat.Pipeline, |
There was a problem hiding this comment.
I've been torn about this sort of change in one of my related PRs as well. Basically, we're now passing an extra beat.Info parameter to this function. Looking at the calls to this constructor, they now look like this:
module.NewConnector(b.Info, b.Publisher, ...)where b is of type *beat.Beat.
I wonder if should just make this constructor take b *beat.Beat instead of beatInfo beat.Info, pipeline beat.Pipeline. I could go either way on this. WDYT?
There was a problem hiding this comment.
My usual preference is to pass the simpler information, both to reduce contextual load (a BeatInfo parameter implies a lot more about the function's behavior than a broadly-scoped container that could be affecting many things in non-obvious ways) and because it's easy to refactor to beat.Beat in the future if we ever really need to, but hard to take it back once it's in the API and people start depending on that field in unexpected ways :-)
There was a problem hiding this comment.
Yup, that's fair. I think my concern of adding more arguments is not as strong in comparison, at least not at this point, where the function would be taking in only 4 arguments. If that number ever grows to be really unwieldy, we can revist.
+1 for keeping this change as-is.
faec
changed the title
|
All remaining test failures are Jenkins timeouts that are also happening at head; Travis is green, so going ahead with merge. |
…uditbeat modules (elastic#15100) (cherry picked from commit ac9434b)
faec
added
v7.6.0
and removed
needs_backport
Resolves #14923 and #15065.
Adds an
indexformatted string option to all metricbeat modules that, when present, specifies the (elasticsearch) output index for events from that module.