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

Allow multiple object types per field. #9772

Merged

Conversation

simitt
Copy link
Contributor

@simitt simitt commented Dec 22, 2018

Allow configuring multiple object types for the same field, to allow creating multiple dynamic templates on dynamic attributes, where keys are unknown.

Usage:

- name: tags
  object_type_params:
  -  object_type: boolean
  -  object_type: scaled_float
      scaling_factor: 1000000

precondition for elastic/apm-server#1712

@simitt simitt requested a review from a team as a code owner December 22, 2018 15:20
Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Could you add a changelog entry and make Hound happy?

libbeat/common/field.go Outdated Show resolved Hide resolved
libbeat/template/processor.go Show resolved Hide resolved
libbeat/template/processor_test.go Outdated Show resolved Hide resolved
libbeat/common/field.go Outdated Show resolved Hide resolved
libbeat/common/field.go Outdated Show resolved Hide resolved
libbeat/common/field.go Outdated Show resolved Hide resolved
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Interesting addition. Only some small comments.

libbeat/common/field.go Show resolved Hide resolved
libbeat/template/processor_test.go Show resolved Hide resolved
libbeat/common/field_test.go Outdated Show resolved Hide resolved
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the PR comments, this LGTM, I am only wondering now if object_type_params is a good name for this, but I don't have a better proposal...

Failing tests should be solved in master, could you please update to see all green?

@simitt
Copy link
Contributor Author

simitt commented Jan 3, 2019

@ruflin PR is ready for another review, failing tests are unrelated from what I see.

@@ -119,20 +121,28 @@ func (p *Processor) integer(f *common.Field) common.MapStr {
return property
}

func (p *Processor) scaledFloat(f *common.Field) common.MapStr {
func (p *Processor) scaledFloat(f *common.Field, params ...common.MapStr) common.MapStr {
property := getDefaultProperties(f)
property["type"] = "scaled_float"

if p.EsVersion.IsMajor(2) {
Copy link
Member

Choose a reason for hiding this comment

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

@urso Not related to this PR but stumbled over it here. Should we remove these checks in master to clean up the code? 7.0 with 2 is not really supported.

@simitt simitt merged commit faac6b5 into elastic:master Jan 7, 2019
@simitt simitt added the v6.7.0 label Jan 7, 2019
simitt added a commit to simitt/beats that referenced this pull request Jan 11, 2019
@simitt simitt deleted the allow-multiple-dynamic-templates-per-field branch August 4, 2020 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants