-
Notifications
You must be signed in to change notification settings - Fork 831
support environment variables in properties #1828
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds an environment-variable configuration layer and standardizes property naming to snake_case while preserving backward compatibility via key normalization.
Changes:
- Introduces
PropertySourceto unify external, environment, and “regular” (system/file/classpath) properties with defined precedence. - Adds environment-variable loading (
IO_PROMETHEUS_*) and camelCase→snake_case normalization for file/system/external keys. - Updates config docs, test resources, and tests to use the new snake_case property names and new loader APIs.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| prometheus-metrics-config/src/main/java/io/prometheus/metrics/config/PropertySource.java | New abstraction for multi-source property lookup with precedence and key transformations. |
| prometheus-metrics-config/src/main/java/io/prometheus/metrics/config/PrometheusPropertiesLoader.java | Loads properties from file/classpath/system, adds env var layer, normalizes keys, validates leftovers. |
| prometheus-metrics-config/src/main/java/io/prometheus/metrics/config/Util.java | Refactors property lookup helpers to read from PropertySource and improve key formatting in errors. |
| prometheus-metrics-config/src/main/java/io/prometheus/metrics/config/MetricsProperties.java | Migrates internal property-name constants to snake_case and updates loading/validation paths. |
| prometheus-metrics-config/src/main/java/io/prometheus/metrics/config/ExporterProperties.java | Switches exporter property keys to snake_case and loads via PropertySource. |
| prometheus-metrics-config/src/main/java/io/prometheus/metrics/config/ExporterHttpServerProperties.java | Updates prefix/key naming to snake_case and loads via PropertySource. |
| prometheus-metrics-config/src/main/java/io/prometheus/metrics/config/ExporterFilterProperties.java | Renames filter keys to snake_case and updates loader wiring. |
| prometheus-metrics-config/src/main/java/io/prometheus/metrics/config/ExporterOpenTelemetryProperties.java | Renames OTel keys to snake_case and updates loader wiring. |
| prometheus-metrics-config/src/main/java/io/prometheus/metrics/config/ExporterPushgatewayProperties.java | Renames pushgateway keys to snake_case and updates loader wiring. |
| prometheus-metrics-config/src/main/java/io/prometheus/metrics/config/ExemplarsProperties.java | Renames exemplars keys to snake_case and loads via PropertySource. |
| prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/UtilTest.java | Updates tests to use PropertySource-based APIs. |
| prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/PrometheusPropertiesTest.java | Updates loading tests to use PropertySource and asserts properties are consumed. |
| prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/PrometheusPropertiesLoaderTest.java | Updates loader tests to snake_case keys. |
| prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/MetricsPropertiesTest.java | Updates expected exception messages and property keys to snake_case. |
| prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/ExporterPropertiesTest.java | Updates keys and loader helper to use PropertySource. |
| prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/ExporterHttpServerPropertiesTest.java | Updates keys and loader helper to use PropertySource. |
| prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/ExporterFilterPropertiesTest.java | Updates keys and loader helper to use PropertySource. |
| prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/ExporterOpenTelemetryPropertiesTest.java | Updates keys and loader helper to use PropertySource. |
| prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/ExporterPushgatewayPropertiesTest.java | Updates loader helper to use PropertySource. |
| prometheus-metrics-config/src/test/java/io/prometheus/metrics/config/ExemplarsPropertiesTest.java | Updates keys and loader helper to use PropertySource. |
| prometheus-metrics-config/src/test/resources/prometheus.properties | Updates test properties file to snake_case keys. |
| prometheus-metrics-config/src/test/resources/emptyUpperBounds.properties | Updates test properties key to snake_case. |
| docs/content/config/config.md | Documents env var support, precedence, and snake_case naming; updates examples/tables. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
prometheus-metrics-config/src/main/java/io/prometheus/metrics/config/PropertySource.java
Outdated
Show resolved
Hide resolved
...us-metrics-config/src/main/java/io/prometheus/metrics/config/PrometheusPropertiesLoader.java
Show resolved
Hide resolved
...us-metrics-config/src/main/java/io/prometheus/metrics/config/PrometheusPropertiesLoader.java
Show resolved
Hide resolved
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
1d05224 to
8ee2b21
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private static String normalize(String metricName) { | ||
| return metricName.replace(".", "_"); | ||
| } |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MetricPropertiesMap.normalize() only replaces '.' with '_' but does not apply the same casing normalization as PrometheusPropertiesLoader.normalizePropertyKey() (which lowercases). With the new loader behavior, metric-specific properties for metrics containing uppercase letters will not be found via getMetricProperties("MyMetric"). Consider making metric-name normalization consistent between storage and lookup (either preserve case throughout, or normalize both sides the same way).
| @@ -124,7 +165,7 @@ public Builder defaultMetricsProperties(MetricsProperties defaultMetricsProperti | |||
| } | |||
|
|
|||
| public Builder metricProperties(Map<String, MetricsProperties> metricProperties) { | |||
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Builder.metricProperties(...) now merges the provided map into the existing MetricPropertiesMap instead of replacing it. If callers invoke metricProperties(...) multiple times, they can no longer override/clear earlier entries. If the prior behavior was “setter” semantics, consider restoring replacement semantics (e.g., by clearing first) or renaming the method to make additive behavior explicit.
| public Builder metricProperties(Map<String, MetricsProperties> metricProperties) { | |
| public Builder metricProperties(Map<String, MetricsProperties> metricProperties) { | |
| this.metricProperties.clear(); |
| /** Properties starting with io.prometheus.exporter.httpServer */ | ||
| public class ExporterHttpServerProperties { | ||
|
|
||
| private static final String PORT = "port"; | ||
| private static final String PREFER_UNCOMPRESSED_RESPONSE = "preferUncompressedResponse"; | ||
| private static final String PREFIX = "io.prometheus.exporter.httpServer"; | ||
| private static final String PREFER_UNCOMPRESSED_RESPONSE = "prefer_uncompressed_response"; | ||
| private static final String PREFIX = "io.prometheus.exporter.http_server"; |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class-level comment still says properties start with io.prometheus.exporter.httpServer, but the effective prefix is now io.prometheus.exporter.http_server (with camelCase supported only via normalization). Please update the Javadoc to reflect the current canonical property name/prefix to avoid confusing users.
| io.prometheus.metrics.http_duration_seconds.histogramClassicUpperBounds = .005, .01, .015, .02 | ||
| io.prometheus.metrics.summaryQuantiles = 0.5, 0.95, 0.99 | ||
| io.prometheus.metrics.summaryQuantileErrors = 0.01, 0.01, 0.001 | ||
| io.prometheus.metrics.histogram_classic_upper_bounds = .005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5 |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test properties file dropped the last default classic histogram bound (", 10"). That also forces multiple tests to switch from expecting 11 to 10 bounds, which seems unrelated to adding env-var support and diverges from the documented/default bucket list. If this was accidental, restore the trailing 10 and keep the existing expectations.
| io.prometheus.metrics.histogram_classic_upper_bounds = .005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5 | |
| io.prometheus.metrics.histogram_classic_upper_bounds = .005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5, 10 |
| void testPrometheusConfig() { | ||
| PrometheusProperties result = PrometheusProperties.get(); | ||
| assertThat(result.getDefaultMetricProperties().getHistogramClassicUpperBounds()).hasSize(11); | ||
| assertThat(result.getDefaultMetricProperties().getHistogramClassicUpperBounds()).hasSize(10); |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assertion was updated to expect 10 classic upper bounds, which is driven by the change in src/test/resources/prometheus.properties dropping one bound. If the intent is only to rename keys / add env-var support, consider restoring the original bounds list and keeping this expectation at 11 to avoid an unrelated behavioral change in the test fixture.
| assertThat(result.getDefaultMetricProperties().getHistogramClassicUpperBounds()).hasSize(10); | |
| assertThat(result.getDefaultMetricProperties().getHistogramClassicUpperBounds()).hasSize(11); |
| PrometheusProperties prometheusProperties = PrometheusPropertiesLoader.load(); | ||
| assertThat(prometheusProperties.getDefaultMetricProperties().getHistogramClassicUpperBounds()) | ||
| .hasSize(11); | ||
| .hasSize(10); |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test now expects 10 default histogram classic upper bounds because the test classpath prometheus.properties dropped one entry. If the bound removal wasn’t intentional, restore the original list (including 10) and keep the expectation at 11 so the test continues to validate the full default bucket set.
| .hasSize(10); | |
| .hasSize(11); |
| // Package-private constructor for PrometheusPropertiesLoader and Builder | ||
| PrometheusProperties( | ||
| MetricsProperties defaultMetricsProperties, | ||
| Map<String, MetricsProperties> metricProperties, | ||
| MetricPropertiesMap metricProperties, | ||
| ExemplarsProperties exemplarProperties, | ||
| ExporterProperties exporterProperties, | ||
| ExporterFilterProperties exporterFilterProperties, | ||
| ExporterHttpServerProperties httpServerConfig, | ||
| ExporterPushgatewayProperties pushgatewayProperties, | ||
| ExporterOpenTelemetryProperties otelConfig) { |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PrometheusProperties constructor is no longer public (now package-private). That is a source/binary breaking API change for any users instantiating PrometheusProperties directly, and it also conflicts with the PR description claiming “No Breaking Changes”. Consider keeping this constructor public, or update the public API/docs to provide an alternative migration path and call out the breaking change explicitly.
Summary
No Breaking Changes. This is fully backward compatible:
Migration Guide
No migration required. However, to adopt the new features:
To use environment variables: Convert property names to uppercase with underscores
io.prometheus.exporter.httpServer.port→IO_PROMETHEUS_EXPORTER_HTTP_SERVER_PORTTo use snake_case (recommended): Update properties files
io.prometheus.metrics.exemplarsEnabled→io.prometheus.metrics.exemplars_enabledNotes