Use local timezone for TZ conversion in the FB system module #5647
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
This adds a `convert_timezone` fileset parameter that, when enabled,
does two things:
* Uses the `add_locale` processor in the FB proespector config
* Uses `{{ beat.timezone }}` as the `timezone` parameter for the
date processor in the Ingest Node pipeline. This parameter accepts
templates starting with ES 6.1.
For the moment the `convert_timezone` flag is off by default, to keep
backwards compatibility and because it results in an error when used
with ES < 6.1.
Closes elastic#3898.
For now this is only applied to the system module, but likely more
modules would benefit from this feature.
tsg
added
Filebeat
houndci-bot
reviewed
Nov 20, 2017
filebeat/fileset/fileset.go
Outdated
| if err != nil { | ||
| return "", fmt.Errorf("Error expanding vars on the ingest pipeline path: %v", err) | ||
| } | ||
|
|
||
| return formatPipelineID(fs.mcfg.Module, fs.name, path, beatVersion), nil | ||
| } | ||
|
|
||
| func (fs *Fileset) GetPipeline() (pipelineID string, content map[string]interface{}, err error) { | ||
| path, err := applyTemplate(fs.vars, fs.manifest.IngestPipeline) | ||
| func (fs *Fileset) GetPipeline(esVersion string) (pipelineID string, content map[string]interface{}, err error) { |
There was a problem hiding this comment.
exported method Fileset.GetPipeline should have comment or be unexported
andrewkroh
reviewed
Nov 21, 2017
There was a problem hiding this comment.
LGTM. Left a few minor comments about log messages. Consider testing out my nonewlines tool on the package 😄 .
filebeat/fileset/fileset.go
Outdated
| var exists bool | ||
| name, exists := vals["name"].(string) | ||
| if !exists { | ||
| return nil, fmt.Errorf("Variable doesn't have a string 'name' key") |
There was a problem hiding this comment.
Not sure it matters, but !exists in this case is telling you that either "name" does not exist or name is not a string. And so maybe calling the variable ok would be better since it allows more ambiguity.
Same comment for the other locations where the type assertion and map lookup signals are coalesced.
filebeat/fileset/fileset.go
Outdated
| return vars, fmt.Errorf("Error parsing version %s: %v", minESVersion["version"].(string), err) | ||
| } | ||
|
|
||
| logp.Debug("fileset", "Comparing ES version %s with %s", haveVersion, minVersion) |
There was a problem hiding this comment.
I suggest "Comparing ES version %s with requirement of %s" to make the meaning of the two versions more clear.
| @@ -155,18 +161,58 @@ func (fs *Fileset) evaluateVars() (map[string]interface{}, error) { | |||
| return vars, nil | |||
| } | |||
|
|
|||
| // turnOffElasticsearchVars re-evaluates the variables that have `min_elasticsearch_version` | |||
| // set. | |||
| func (fs *Fileset) turnOffElasticsearchVars(vars map[string]interface{}, esVersion string) (map[string]interface{}, error) { | |||
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.
This adds a
convert_timezonefileset parameter that, when enabled,does two things:
add_localeprocessor in the FB proespector config{{ beat.timezone }}as thetimezoneparameter for thedate processor in the Ingest Node pipeline. This parameter accepts
templates starting with ES 6.1.
For the moment the
convert_timezoneflag is off by default, to keepbackwards compatibility.
If ES is < 6.1, the flag is automatically considered off when creating the pipeline
(but the
add_localeprocessor is still used).Closes #3898.
For now this is only applied to the system module, but likely more
modules would benefit from this feature.
Todos: