Move Field(s) to common. #5271
Move Field(s) to common. #5271
Conversation
| // This includes all entries without special handling for different versions. | ||
| // Currently this is: | ||
| // long, geo_point, date, short, byte, float, double, boolean | ||
| func (f *Field) other() common.MapStr { |
There was a problem hiding this comment.
This logic and the other methods regarding transformation moved to template.Processor.
| return key | ||
| } | ||
|
|
||
| type dynamicType struct{ value interface{} } |
There was a problem hiding this comment.
moved to common.Field
| @@ -1,325 +0,0 @@ | |||
| package template | |||
There was a problem hiding this comment.
the tests are mostly moved over to template/processor_test, tests regarding unpacking are moved to common/field_test.go
| // HasKey checks if inside fields the given key exists | ||
| // The key can be in the form of a.b.c and it will check if the nested field exist | ||
| // In case the key is `a` and there is a value `a.b` false is return as it only | ||
| // returns true if it's a leave node |
There was a problem hiding this comment.
moved to common.Field
Make type Field(s) reusable and seperate from transformation logic.
simitt
force-pushed
the
move-fields-to-common
branch
from
a681d6b to
6752d5d
Compare
| return nil | ||
| } | ||
|
|
||
| func (p *Processor) other(f *common.Field) common.MapStr { |
There was a problem hiding this comment.
I personally preferred that these were methods of Field as all the parts in this methods are applied to on the passed field anyway. What is the reason you changed this logic? Also this increases the number of params to be passed.
There was a problem hiding this comment.
I think it makes sense to make the fields structure the same and therefore reusable between different processors.
This is the reason I would not bind processor specific logic to the data structure itself. The logic that is generally applicable for transformation to a ES template and also to a Kibana index pattern could be moved to the common.fields. However, I might do this in a seperate step if we encounter processor logic that is generally applicable.
Another advantage is that this makes it easier to test when you pass a data structure into a method instead having a large interface with lots of methods.
There was a problem hiding this comment.
Got it. I think we could perhaps somehow compose the overall fields out of TemplateFields and KibanaFields so the TemplateFields would still keep it's method and not have to be passed each time. But as they overlap this could become trickier.
I think I miss the part on how it simplifies testing.
I suggest we move forward as is right now as functionality will be identical. We can still do refactoring when we have all the changes in.
There was a problem hiding this comment.
E.g some of the methods use p.EsVersion for decisions on how to transform. I would not consider an ES Version to be a general attribute of a Field.
I fully agree though that we should share logic between ES and Kibana specific transformations when it overlaps or depends on each other.
Make type Field(s) reusable and seperate from transformation logic.
This is the first part for implementing #4899.
This PR moves the type
FieldandFieldsto packagecommonand seperates the types from the ES transformation logic. This is a first step to reuse the data structure for transforming information from thefields.ymlalso for Kibana index pattern creation.Changes:
template.Field->common.Fieldtemplate.Fields->common.Fieldtemplate.processinglogic fromtemplate.Fieldsandtemplate.Fieldin one filetemplate.Processor