Skip to content

[Backport 8.x] Reimplement LogStash::String setting in Java (#16576)#16959

Merged
andsel merged 3 commits intoelastic:8.xfrom
andsel:backport_16576_8.x
Jan 27, 2025
Merged

[Backport 8.x] Reimplement LogStash::String setting in Java (#16576)#16959
andsel merged 3 commits intoelastic:8.xfrom
andsel:backport_16576_8.x

Conversation

@andsel
Copy link
Copy Markdown
Member

@andsel andsel commented Jan 27, 2025

Cherry-picked from commit 03b11e9

Non clean backport of #16576

The main differences with main are:

  • missed rename of setting due to Logtash modules code removal in main 179e2ef
  • use of log4j spy instead of mocking log on removed code: 9ad3e95

Reimplements `LogStash::Setting::String` Ruby setting class into the `org.logstash.settings.SettingString` and exposes it through `java_import` as `LogStash::Setting::SettingString`.
Updates the rspec tests in two ways:
- logging mock is now converted to real Log4J appender that spy log line that are later verified
- verifies `java.lang.IllegalArgumentException` instead of `ArgumentError` is thrown because the kind of exception thrown by Java code, during verification.
@elastic-sonarqube
Copy link
Copy Markdown

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube

@elasticmachine
Copy link
Copy Markdown

💚 Build Succeeded

@andsel andsel changed the title Backport 16576 8.x [Backport 8.x] Reimplement LogStash::String setting in Java (#16576) Jan 27, 2025
@andsel andsel marked this pull request as ready for review January 27, 2025 14:16
@andsel andsel requested a review from kaisecheng January 27, 2025 14:23
end
end

java_import org.logstash.settings.NullableSetting
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it not in use and can be removed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If we remove this then we fail the test https://github.com/elastic/logstash/blob/8.x/logstash-core/spec/logstash/settings/nullable_spec.rb and it's reason why we changed the test a little bit

expect(nullable_setting).to be_a_kind_of(LogStash::Setting::NullableSetting)

Copy link
Copy Markdown
Contributor

@kaisecheng kaisecheng left a comment

Choose a reason for hiding this comment

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

LGTM

@andsel andsel merged commit a0378c0 into elastic:8.x Jan 27, 2025
@andsel
Copy link
Copy Markdown
Member Author

andsel commented Jan 27, 2025

@logstashmachine backport 8.17

github-actions bot pushed a commit that referenced this pull request Jan 27, 2025
…16959)

Non clean backport of #16576

----

Reimplements `LogStash::Setting::String` Ruby setting class into the `org.logstash.settings.SettingString` and exposes it through `java_import` as `LogStash::Setting::SettingString`.
Updates the rspec tests in two ways:
- logging mock is now converted to real Log4J appender that spy log line that are later verified
- verifies `java.lang.IllegalArgumentException` instead of `ArgumentError` is thrown because the kind of exception thrown by Java code, during verification.

* Fixed the rename of NullableString to SettingNullableString

* Fixed runner test to use real spy logger from Java Settings instead of mock test double

(cherry picked from commit a0378c0)
@andsel
Copy link
Copy Markdown
Member Author

andsel commented Jan 27, 2025

@logstashmachine backport 8.16

andsel added a commit to andsel/logstash that referenced this pull request Jan 27, 2025
…16576) (elastic#16959)

Non clean backport of elastic#16576

----

Reimplements `LogStash::Setting::String` Ruby setting class into the `org.logstash.settings.SettingString` and exposes it through `java_import` as `LogStash::Setting::SettingString`.
Updates the rspec tests in two ways:
- logging mock is now converted to real Log4J appender that spy log line that are later verified
- verifies `java.lang.IllegalArgumentException` instead of `ArgumentError` is thrown because the kind of exception thrown by Java code, during verification.

* Fixed the rename of NullableString to SettingNullableString

* Fixed runner test to use real spy logger from Java Settings instead of mock test double
andsel added a commit that referenced this pull request Jan 27, 2025
…16959) (#16960)

Clean backport of #16959 from 8.x to 8.17

----

Reimplements `LogStash::Setting::String` Ruby setting class into the `org.logstash.settings.SettingString` and exposes it through `java_import` as `LogStash::Setting::SettingString`.
Updates the rspec tests in two ways:
- logging mock is now converted to real Log4J appender that spy log line that are later verified
- verifies `java.lang.IllegalArgumentException` instead of `ArgumentError` is thrown because the kind of exception thrown by Java code, during verification.

* Fixed the rename of NullableString to SettingNullableString

* Fixed runner test to use real spy logger from Java Settings instead of mock test double

(cherry picked from commit a0378c0)

Co-authored-by: Andrea Selva <selva.andre@gmail.com>
andsel added a commit that referenced this pull request Jan 27, 2025
…16959) (#16961)

Non clean backport of #16959 from 8.x to 8.16 (which is a non clean backport of #16576 to 8.x)

Changes respect to 8.x:
- in 8.x the `with_deprecated_alias` receives also a parameter specifying the version where the setting is removed. In this case we removed the extra parameter to the method https://github.com/elastic/logstash/blob/a0378c05cbcc8eda3ec0b2de286fb9ebde97df96/logstash-core/lib/logstash/environment.rb#L79
- fixed the test removing the check for the remove version https://github.com/elastic/logstash/blob/a0378c05cbcc8eda3ec0b2de286fb9ebde97df96/logstash-core/spec/logstash/settings/setting_with_deprecated_alias_spec.rb#L157

----

Reimplements `LogStash::Setting::String` Ruby setting class into the `org.logstash.settings.SettingString` and exposes it through `java_import` as `LogStash::Setting::SettingString`.
Updates the rspec tests in two ways:
- logging mock is now converted to real Log4J appender that spy log line that are later verified
- verifies `java.lang.IllegalArgumentException` instead of `ArgumentError` is thrown because the kind of exception thrown by Java code, during verification.

* Fixed the rename of NullableString to SettingNullableString

* Fixed runner test to use real spy logger from Java Settings instead of mock test double
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants