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

[Logstash] Fix node cel script type checking issue #9556

Merged
merged 3 commits into from
Apr 17, 2024
Merged

[Logstash] Fix node cel script type checking issue #9556

merged 3 commits into from
Apr 17, 2024

Conversation

robbavey
Copy link
Member

@robbavey robbavey commented Apr 9, 2024

An update to the cel-go library added stricter type checking, and made certain overload operations invalid in CEL scripts. This is a work-around to "hide" the type in an array, until a better fix is available.

Closes: #9546,#9319

@robbavey
Fix Logstash integration type checking issue
eb5c19b 
An update to the cel-go library added stricter type checking, and made drop invalid.
This is a work-around to "hide" the type in an array, until a better fix is available.

Closes: elastic#9546,elastic#9319
@robbavey robbavey requested a review from a team as a code owner April 9, 2024 20:00
@robbavey
Copy link
Member Author

robbavey commented Apr 9, 2024

Test failure looks like an instance of #9366, which is a known bug for Logstash - I believe it is an instance of elastic/logstash#14335

efd6
efd6 approved these changes Apr 9, 2024
View reviewed changes
Copy link
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

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

CEL code LGTM

@robbavey robbavey enabled auto-merge (squash) April 15, 2024 13:56
@robbavey
Copy link
Member Author

/test

@robbavey robbavey mentioned this pull request Apr 17, 2024
Closed
@robbavey
Copy link
Member Author

/test

@elasticmachine
Copy link

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@elasticmachine
Copy link

💚 Build Succeeded

History

@robbavey robbavey merged commit ff95860 into elastic:main Apr 17, 2024
3 checks passed
@elastic-sonarqube Elastic SonarQube
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@elasticmachine
Copy link

Package logstash - 2.4.4 containing this change is available at https://epr.elastic.co/search?package=logstash

@IanLee1521
Copy link
Contributor

IanLee1521 commented Apr 17, 2024 •
edited
Loading

FYI -- I can confirm this fixed the metrics not coming in issue I reported to support: https://support.elastic.co/cases/500Vv000002ryV2IAI

  • Agents at 8.11.4, Logstash integration at 2.4.3 --> metrics working
  • Updated Agents to 8.13.2 --> metrics stopped flowing
  • Updated Logstash integration to 8.13.2 --> metrics flowing again.

Thanks much!

@efd6
Copy link
Contributor

efd6 commented Apr 17, 2024

@IanLee1521 Thank you for the report, it has spurred additional work that is resolving the issue at a deeper level as well.

@IanLee1521
Copy link
Contributor

Glad to hear! Feel free to ping me if you need any additional testing. Cheers.

@robbavey
Copy link
Member Author

@IanLee1521 Thanks for the report, and the testing. Please feel free to reach out to me with any feedback you have with this issue, or the integration in general

@IanLee1521
Copy link
Contributor

@robbavey -- happy to! I pinged you on the community Slack, but just let me know if there is a better way to connect up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efd6 efd6 efd6 approved these changes

@miltonhultgren miltonhultgren miltonhultgren approved these changes

Assignees
No one assigned
Labels
Integration:logstash Logstash
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Logstash Metrics] logstash.node.stats failing to index stats due to error in CEL expression.
6 participants
@robbavey @elasticmachine @IanLee1521 @efd6 @miltonhultgren @andrewkroh