-
Notifications
You must be signed in to change notification settings - Fork 457
[aws][1password] Fix field conflicts #2687
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
Conversation
/test |
f6d6c06
to
399a5db
Compare
@andrewkroh Should be gtg |
/test |
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. I expanded the PR description with a list of changes to help the other reviewers.
Looks like the aws.elb_logs needs a
|
@elastic/integrations can you please review the AWS and Azure fixes. |
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
Thanks for the fix!! @andrewkroh Is this considered breaking change for users who already have these integrations installed? |
@kaiyan-sheng I'd consider this a bugfix because the data didn't comply with ECS. Fleet will rollover their data streams given the change to the template and this will prevent mapping conflicts, but there will be a Kibana index pattern conflict for the between the old data and the new data. At least now the new data from these integrations will not conflict with each other in the |
bfd8d30
to
4f901a4
Compare
packages/aws/data_stream/elb_logs/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
packages/aws/data_stream/elb_logs/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
/test |
1 similar comment
/test |
[git-generate] cd packages/aws elastic-package test pipeline -g
0132e30
to
15951f6
Compare
I rebased this so that we can review the fixes separately from the formatting updates to the expected JSON pipeline test files. |
@andrewkroh thanks. Apologies for not staying on this, been super busy at work. |
No worries! We realize you have a two jobs working here and there 😉 . |
[git-generate] cd packages/azure elastic-package test pipeline -g
[git-generate] cd packages/1password elastic-package test pipeline -g
[git-generate] cd packages/aws/data_stream/elb_logs jq .expected[0] _dev/test/pipeline/test-alb.log-expected.json > sample_event.json elastic-package format
15951f6
to
585c700
Compare
/test |
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. Approving on behalf of elastic/security-external-integrations.
@elastic/integrations, can you please add your review for the aws and azure changes.
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.
aws and azure looks good. Thanks!
How should we handle other field conflicts such as this? New issue? event.duration is another one for the azure integration: This is a long everywhere else besides the azure activity logs. { |
I would do a new issue since this was already merged |
What does this PR do?
Resolve field conflicts.
NOTE: The first three commits only contain formatting changes and can be verified via git-generate. It's recommended that you ignore those during the review.
Data Stream Changes
1password.item_usages
event.created
- keyword -> date (per ECS)1password.signin_attempts
event.created
- keyword -> date (per ECS)aws.elb_logs
source.port
- keyword to long (per ECS)tracing.trace.id
renamed totrace.id
(per ECS)azure.auditlogs
client.ip
- keyword to ip (per ECS)azure.eventhub
azure-eventhub.offset
- keyword to longazure-eventhub.sequence_number
- keyword to longChecklist
changelog.yml
file.Related issues