Skip to content

Conversation

@andsel
Copy link
Contributor

@andsel andsel commented Apr 28, 2022

Release notes

[rn:skip]

What does this PR do?

This PR fixes some failing tests on main

There are two failures that this PR fixes:

  • tests that referred host or path attribute running against Logstash 8, which by default has ECS enabled.
    The fix consisted in surrounding the test with the ECS harness and use the ecs_select to refer attributes with proper name.
  • another fix regards the test having timed_out, the codec is auto flushed. It presented 2 problems:
    • the first is that to request the subject.stop to trigger the auto_flush call (called by exit_flush method).
    • the second regard a regression introduced with eliminate re-use of codecs across identities logstash-codec-multiline#70. The test worked because the the IdentityMapCodec used a shared codec. When this behavior was removed the test, that engage multiple threads, exposed a problem of overwriting of codec reference. This overwriting drove to the lost of data, used in test logic. The fix consists in clone redefinition to make it almost a singleton.

Why is it important/What is the impact to the user?

Make the test green increase the confidence in the code base.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [ ] I have made corresponding change to the default configuration files (and/or docker env variables)
  • [ ] I have added tests that prove my fix is effective or that my feature works

Related issues

andsel added 2 commits April 29, 2022 15:56
With the fix on multiline codec (logstash-plugins/logstash-codec-multiline#70) introduced
to avoid sharing of codec across multiple threads, it started to make fail this test that leveraged that shared instance
to tracing the codec calls.
The multithreading nature of the test and the lack of synchornizaion to access the IdentityCodecMap led to multiple threads
overwrite of the codec instances. This overwrite cleans the history of previous tracked calls making it failing.

This commit fixes it giving to che #clone a method a simil-singleton functionality, to don't loose the list of tracked calls.
@andsel andsel marked this pull request as ready for review April 29, 2022 15:44
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.

🥇

@andsel andsel merged commit 1891b34 into logstash-plugins:main May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants