-
Notifications
You must be signed in to change notification settings - Fork 116
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
feat: introduce --fail-on-ingest-warnings to detect, and treat as errors, warnings from ingest pipeline #1755
feat: introduce --fail-on-ingest-warnings to detect, and treat as errors, warnings from ingest pipeline #1755
Conversation
It would be nice to surface these warnings all the time, not just when The problem here is I didn't even know these logs existed in the first place. What's worse is they are discovered by end users and not devs. Even if we had a way to disable them for elastic-package (like other validations), the warnings will still surface for users. |
7faad5f
to
c067c8f
Compare
…ors, warnings from ingest pipeline
c067c8f
to
96e5755
Compare
+1, it'd be nice if we can just fail always on these errors, as they are actual issues. We introduced in the past some checks based on errors found in elastic-agent logs (#1256). @pkoutsovasilis could you try to always enable this check and test with all integrations (adding a comment |
9f3534b
to
7c96c1f
Compare
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#9561 |
Overall integrations testing do not look that bad https://buildkite.com/elastic/integrations/builds/10324 although there is an integration that is not detected (cisco_ios). Just pushed a commit that hopefully address this |
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#9561 |
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#9561 |
@jsoriano any idea why cisco_ftd in the integration test has a different log format compared to others like this
do we need to support such a format in our log parsing? 🙂 |
test integrations |
f3fd006
to
f9e060c
Compare
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#9561 |
Not sure if this is the reason, but
If the reason is the version of Elasticsearch, in principle we need to support since 7.14, the first version where Fleet was GA. So we need to support any log formats used between these versions. Or if possible, other approach could be to configure Elasticsearch to use the same logging format independently of the version. |
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#9561 |
…asilis/support_pipeline_warnings
ff55ea3
to
b771542
Compare
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#9561 |
@jsoriano all integrations having failures related to this feature (pipeline warnings) are fixed and I think this is now good to fly with this check always enabled. integrations run |
/test |
@pkoutsovasilis there are some tests consistently failing here, could you take a look to them? |
Here is the related part of the log about the failure: https://buildkite.com/elastic/elastic-package/builds/2822#018ee670-b8c2-4475-86a2-567e849247e6/126-547 |
sure thing @jsoriano , thanks for catching that |
🤦
|
95a7819
to
0b51e62
Compare
0b51e62
to
b3da911
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.
Thanks!
💚 Build Succeeded
History
|
This PR intoduces
--fail-on-ingest-warnings
that instructs elastic-package to check for warnings on the log of elasticsearch when running the pipeline test.For example, clone integration repo at this SHA elastic/integrations@389e006
With the code of this PR running the pipeline test inside
packages/cisco_asa
results in the following:However, the code at main completes the test without any error. Having such a flag that we can opt in and check for warning is useful to prevent issues, such as this one in the future.
Closes #1754