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.

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

Repeated deprecation setting error on main after #15679 #16505

Closed
andsel opened this issue Oct 3, 2024 · 2 comments · Fixed by #16506
Closed

Repeated deprecation setting error on main after #15679 #16505

andsel opened this issue Oct 3, 2024 · 2 comments · Fixed by #16506
Assignees
Labels

Comments

@andsel
Copy link
Contributor

andsel commented Oct 3, 2024

Description of the problem including expected versus actual behavior:
After merge of PR #15679 which introduced new Java classes for base, aliased and Boolean setting translating from Ruby existing code, if Logstash runs with the reload of a pipeline flag (both as command line --config.reload.automatic or configuring it in config/logstash.yml, it starts repeating the following error log:

[2024-10-03T14:03:12,018][WARN ][org.logstash.settings.DeprecatedAlias] The value of setting `api.enabled` has been queried by its deprecated alias `http.enabled`. Code should be updated to query `api.enabled` instead

This happens only when the pipeline is configured in config/pipelines.yml, both as config.string or with path path.config.
Instead, it doesn't happen when running directly the pipeline form the file with logstash -f simple_pipeline.conf

Steps to reproduce:

Please include a minimal but complete recreation of the problem,
including (e.g.) pipeline definition(s), settings, locale, etc. The easier
you make for us to reproduce it, the more likely that somebody will take the
time to look at it.

  1. edit config/pipelines.yml adding
- pipeline.id: test
  config.string: "input{ tcp { port => 12345 } } output { stdout{} }"
  1. Run Logstash with
bin/logstash --config.reload.automatic

If the reload it's not enabled, it doesn't happen.
Also note that it happend every 3 seconds (the default value of config.reload.interval) and appears always twice

Provide logs (if relevant):

[2024-10-03T14:12:36,209][INFO ][logstash.agent           ] Pipelines running {:count=>1, :running_pipelines=>[:test], :non_running_pipelines=>[]}


[2024-10-03T14:12:39,242][WARN ][org.logstash.settings.DeprecatedAlias] The value of setting `api.enabled` has been queried by its deprecated alias `http.enabled`. Code should be updated to query `api.enabled` instead
[2024-10-03T14:12:39,245][WARN ][org.logstash.settings.DeprecatedAlias] The value of setting `api.enabled` has been queried by its deprecated alias `http.enabled`. Code should be updated to query `api.enabled` instead


[2024-10-03T14:12:42,234][WARN ][org.logstash.settings.DeprecatedAlias] The value of setting `api.enabled` has been queried by its deprecated alias `http.enabled`. Code should be updated to query `api.enabled` instead
[2024-10-03T14:12:42,235][WARN ][org.logstash.settings.DeprecatedAlias] The value of setting `api.enabled` has been queried by its deprecated alias `http.enabled`. Code should be updated to query `api.enabled` instead


[2024-10-03T14:12:45,230][WARN ][org.logstash.settings.DeprecatedAlias] The value of setting `api.enabled` has been queried by its deprecated alias `http.enabled`. Code should be updated to query `api.enabled` instead
[2024-10-03T14:12:45,233][WARN ][org.logstash.settings.DeprecatedAlias] The value of setting `api.enabled` has been queried by its deprecated alias `http.enabled`. Code should be updated to query `api.enabled` instead
@andsel
Copy link
Contributor Author

andsel commented Oct 3, 2024

This problem raises from the fact the Agent cyclically (if autoreload is enabled) reload and resolve pipeline configuration, in checking if Settings instances has changed, it invokes the equals method which goes deep to the DeprecatedAlias.value and so the log.

In more details:

converge_state_and_update unless stopped?

converge_result = resolve_actions_and_converge_state(results.response)

pipeline_actions = resolve_actions(pipeline_configs)

@state_resolver.resolve(@pipelines_registry, pipeline_configs)

if pipeline_config != pipeline.pipeline_config

def ==(other)
return false unless other.kind_of?(::LogStash::Settings)
self.to_hash == other.to_hash
end

In checking equality of hash map both keys and values of hash maps are accessed. The map (due to the with_deprecated_alias) contains 2 settings one for api.enabled which is the SettingWithDeprecatedAlias proxy and another for http.enabled which is the instance of DeprecatedAlias. So it seems that every time this comparison process accesses an instance of DeprecatedAlias then the value() method is also invoked.

@andsel
Copy link
Contributor Author

andsel commented Oct 3, 2024

The motivation of access the value is that Settings#to_hash method does it:

def to_hash
hash = {}
@settings.each do |name, setting|
next if setting.kind_of? Setting::DeprecatedAlias
hash[name] = setting.value
end
hash
end

andsel added a commit that referenced this issue Oct 4, 2024
)

Update Settings to_hash method to also skip Java DeprecatedAlias and not just the Ruby one.
With PR #15679 was introduced org.logstash.settings.DeprecatedAlias which mirrors the behaviour of Ruby class Setting::DeprecatedAlias. The equality check at Logstash::Settings, as descibed in #16505 (comment), is implemented comparing the maps.
The conversion of Settings to the corresponding maps filtered out the Ruby implementation of DeprecatedAlias but not the Java one.
This PR adds also the Java one to the list of filter.
github-actions bot pushed a commit that referenced this issue Oct 11, 2024
)

Update Settings to_hash method to also skip Java DeprecatedAlias and not just the Ruby one.
With PR #15679 was introduced org.logstash.settings.DeprecatedAlias which mirrors the behaviour of Ruby class Setting::DeprecatedAlias. The equality check at Logstash::Settings, as descibed in #16505 (comment), is implemented comparing the maps.
The conversion of Settings to the corresponding maps filtered out the Ruby implementation of DeprecatedAlias but not the Java one.
This PR adds also the Java one to the list of filter.

(cherry picked from commit 5d4825f)
andsel added a commit that referenced this issue Oct 11, 2024
… other than Ruby's one

Update Settings to_hash method to also skip Java DeprecatedAlias and not just the Ruby one.
With PR #15679 was introduced org.logstash.settings.DeprecatedAlias which mirrors the behaviour of Ruby class Setting::DeprecatedAlias. The equality check at Logstash::Settings, as descibed in #16505 (comment), is implemented comparing the maps.
The conversion of Settings to the corresponding maps filtered out the Ruby implementation of DeprecatedAlias but not the Java one.
This PR adds also the Java one to the list of filter.

(cherry picked from commit 5d4825f)

Co-authored-by: Andrea Selva <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants