-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[libbeat] Change aws_elb autodiscover provider field name to aws.elb.* #16402
[libbeat] Change aws_elb autodiscover provider field name to aws.elb.* #16402
Conversation
"aws": common.MapStr{ | ||
"elb": lbl.toMap(), | ||
}, | ||
"cloud": lbl.toCloudMap(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the move out of meta and why now in both places?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I forgot to add some explanation in PR description for this. This is to update aws_elb
provider to have all the meta fields into autodiscover template by adding it to the root of the event. Please see #14823 (comment) for more details. Thanks!
"aws": common.MapStr{ | ||
"elb": lbl.toMap(), | ||
}, | ||
"cloud": lbl.toCloudMap(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, thanks for the explanation. Looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, if I understand this patch correctly we're replacing aws_elb
with aws.elb
in the output documents but not changing the provider name from aws_elb
is that correct?
Is it correct that YAML configs should not be affected by this, but the event schema is. Is that correct?
In other words, am I right in my understanding that docs like https://github.com/elastic/beats/blob/master/heartbeat/docs/aws-credentials-examples.asciidoc won't need to change?
@andrewvc Thanks for reviewing! Please see my answer in line.
We are changing the output event field from
Yes and no 😄 https://github.com/elastic/beats/blob/master/heartbeat/docs/aws-credentials-examples.asciidoc won't need to change when user is using
Then, with this change, user needs to change the config to use
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What does this PR do?
This PR is to change
aws_elb
autodiscover provider fields fromelb_listener.*
toaws.elb.*
. For example, the output event will look like below from unit test:meta
represents what will end up as metadata in the output of modules that get launched. This PR also updatedaws_elb
provider to have all the meta fields into autodiscover template by adding it to the root of the event. Please see #14823 (comment) for more details.Why is it important?
With this change, fields from
aws_elb
andaws_ec2
autodiscover providers will match.Checklist
Related issues