-
Notifications
You must be signed in to change notification settings - Fork 462
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
[AWS] Move S3, SQS, SNS and lambda lightweight module config into integration #3838
Conversation
🌐 Coverage report
|
@@ -13,7 +13,3 @@ | |||
type: object | |||
description: | | |||
Metric dimensions. | |||
- name: '*.metrics.*.*' |
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.
This is a change I do not understand! From what I understand this means that the events from this package do not contain this data structure anymore. But is this expected? I still see lambda.metrics.<fields>
(es https://github.com/elastic/integrations/pull/3838/files#diff-ab8d12efcf395a213dbef900621dfe0f32e523b7f5a9692896869971264a1a4eR40) in the events so I'm not sure I'm looking in the right place.
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.
Maybe this is because there is the renaming in the ingest pipeline, so in the end these fields will not in the stored event?
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.
Ha we are removing this field (tech debt, not related to lightweight module config) because the metrics we are collecting for lambda are already all listed and documented in fields.yml: https://github.com/elastic/integrations/blob/main/packages/aws/data_stream/lambda/fields/fields.yml#L16
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.
Thanks! So is this expected to be removed everywhere? Just to check for it in other integrations too ;)
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.
Yeah at least in aws package we added all the field names individually into fields.yml so we should remove this '*.metrics.*.*'
from all data streams under aws.
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.
Looks good to me! Just a nitpick, I would bump the minor and not the patch version.
😂 Too late haha I think all the PRs that moves lightweight module all only bumped the patch version 😂 Well...... I'm trying to be consistent haha |
What does this PR do?
This PR is to move lightweight module configuration from metricbeat into integrations for s3_daily_storage, s3_request, SQS, SNS, lambda.
Checklist
changelog.yml
file.Related issues