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.

Sign up for GitHub

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

Jump to bottom

ecs: add v8 alias to v1 implementation #68

Merged
merged 5 commits into from
Nov 11, 2021
Merged

ecs: add v8 alias to v1 implementation #68

merged 5 commits into from
Nov 11, 2021

Conversation

yaauie
Copy link
Contributor

@yaauie yaauie commented Jul 28, 2021

No description provided.

@yaauie yaauie requested a review from kares July 28, 2021 17:25
kares
kares approved these changes Jul 29, 2021
View reviewed changes
Copy link
Contributor

@kares kares left a comment

Choose a reason for hiding this comment

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

🍒

kares
kares approved these changes Jul 29, 2021
View reviewed changes
Copy link
Contributor

@kares kares left a comment

Choose a reason for hiding this comment

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

🍨

@kares kares self-requested a review July 29, 2021 10:13
kares
kares suggested changes Jul 29, 2021
View reviewed changes
Copy link
Contributor

@kares kares left a comment

Choose a reason for hiding this comment

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

sorry wrong tab on previous ✔️

since we need to pass ecs_compatibility to the grok filter we need to make sure logstash-filter-grok is updated first and it's version requirement is updated afterwards with v8 support, sample failing test:

https://app.travis-ci.com/github/logstash-plugins/logstash-input-syslog/jobs/527490671#L516-L517

@yaauie
Copy link
Contributor Author

yaauie commented Nov 5, 2021

upstream grok PR logstash-plugins/logstash-filter-grok#175

@yaauie yaauie mentioned this pull request Nov 5, 2021
Merged
11 tasks
yaauie
yaauie commented Nov 5, 2021
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
kares
kares approved these changes Nov 8, 2021
View reviewed changes
Copy link
Contributor

@kares kares left a comment

Choose a reason for hiding this comment

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

LGTM, the failing CI against 8.0-SNAPSHOT is unrelated

(an issue with timestamp formatting due changes in LS 8.0 e.g. expected: "2021-10-26T20:19:25.000Z" got: "2021-10-26T20:19:25Z")

@yaauie yaauie requested a review from kares November 10, 2021 20:04
@yaauie
Copy link
Contributor Author

yaauie commented Nov 10, 2021

@kares I resolved the failing spec on LS8 with a8c0036 which uses a helper introduced in devutils 2.3 to make assertions about timestamps that are not bound to the serialization length.

kares
kares approved these changes Nov 11, 2021
View reviewed changes
Copy link
Contributor

@kares kares left a comment

Choose a reason for hiding this comment

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

LGTM, 💚 CI

@yaauie yaauie merged commit 7a54456 into logstash-plugins:main Nov 11, 2021
@yaauie yaauie mentioned this pull request Nov 17, 2021
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kares kares kares approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yaauie @kares @roaksoax