[FLINK-38704][runtime] Convert config values to String #27743
[FLINK-38704][runtime] Convert config values to String #27743mukul-8 wants to merge 1 commit intoapache:masterfrom
Conversation
…perties Numeric YAML values are stored as Integer/Long but Properties.getProperty() only returns Strings. Convert all values to String to fix MetricConfig getString() returning null for numeric configuration values.
| entry.getKey().substring(prefix.length(), entry.getKey().length()); | ||
|
|
||
| props.put(keyWithoutPrefix, entry.getValue()); | ||
| props.put(keyWithoutPrefix, entry.getValue().toString()); |
There was a problem hiding this comment.
can the entry.getValue() ever be null? Maybe add a test for this case.
There was a problem hiding this comment.
Good catch. I've verified that entry.getValue() cannot be null. The Configuration.setValueInternal() method ensures only non-null values are stored in the internal map, so entry.getValue() will always return a non-null object at this point.
There was a problem hiding this comment.
I've identified an issue with the current implementation. The .toString() conversion affects complex types like HashMap, which produces non-deterministic string representations due to unordered iteration. This causes the PythonDependencyUtilsTest.testPythonFiles test to fail in CI (though it passed locally due to the non-deterministic nature).
I'll update the implementation to only convert primitive types (Number, Boolean) to String while preserving the original behavior for complex types. This will fix the metrics reporter bug without breaking existing functionality.
What is the purpose of the change
Fixes the Prometheus metrics reporter (and potentially other reporters) not loading configured port values.
The issue occurs because numeric YAML configuration values are stored as Integer/Long objects, but
Properties.getProperty()only returns Strings. WhenMetricConfig.getString()is called for numeric values likemetrics.reporter.prom.port, it returns null, causing reporters to fall back to default values.Note: The bug only reproduces when a single number is assigned to the port (e.g.,
9999). It works correctly when a port-range is used (e.g.,9000-9100) because ranges are stored as Strings.Safety: This change is safe and non-breaking. Properties are inherently String-based (all values are stored and retrieved as Strings), so converting values to String at insertion time maintains the expected behavior while fixing the type mismatch issue.
Workaround: Quote the port value in YAML configuration:
metrics.reporter.prom.port: "9999"Brief change log
addAllToPropertiesmethod to convert all configuration values to String before adding to PropertiesVerifying this change
This change fixes the bug where:
metrics.reporter.prom.port=9999After the fix, the configured port is correctly applied.
Added test case to verify this change is working fine.
Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation