Clarify that a few Prometheus translation_strategy enum options are unstable#543
Conversation
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
|
I have little to no experience in this repository. I'd appreciate guidance if the changes I made were not the right ones to declare config options stable/unstable :) |
| description: Configure how metric names are translated to Prometheus metric names. | ||
| defaultBehavior: underscore_escaping_with_suffixes is used | ||
| ExperimentalPrometheusTranslationStrategy: | ||
| PrometheusTranslationStrategy: |
There was a problem hiding this comment.
So this is an interesting case:
- Declarative config is not stable but will be soon
- Prometheus SDK exporter spec is not stable but the goal is to stabilize soon
- My understanding is that the prometheus SIG wants to stabilize the prometheus SDK exporter spec with mixed stability, specifically calling out that that the translation strategy will have mixed stability as described in this PR
- The stability of declarative config types is bound by the spec, so from a sequencing perspective, the spec needs to stabilize first.
My read on what we're trying to accomplish in this PR is to prepare the proemtheus exporter and related types for this anticipated mixed stability at the spec level. My recommendation:
- Keep
PrometheusTranslationStrategyasExperimentalPrometheusTranslationStrategy. This only impacts SDK implementations - users are not exposed to this. - Add the
/developmentsuffix to theunderscore_escaping_without_suffixes,no_utf8_escaping_with_suffixes,no_translationvalue types, as you've already done.
The /development suffixes on the value types are technically redundant since the whole ExperimentalPrometheusTranslationStrategy is still experimental, but what this has the effect of preparing the UX for the upcoming stabilization of the prometheus exporter spec.
With these changes, a user's config might look like:
file_format: 1.0-rc.3
meter_provider:
readers:
- pull:
exporter:
prometheus/development:
translation_strategy: no_translation/development
When prometheus exporter spec is stabilized, ExperimentalPrometheusTranslationStrategy -> PrometheusTranslationStrategy, ExperimentalPrometheusMetricExporter -> PrometheusMetricExporter, strip /development from prometheus/development property. The user config changes to:
file_format: 1.0-rc.3
meter_provider:
readers:
- pull:
exporter:
prometheus:
translation_strategy: no_translation/development
There was a problem hiding this comment.
This seems ok, with the understanding that the translation strategy may take more time to stabilize, are there other areas of the exporter that would prevent stabilization soon?
There was a problem hiding this comment.
Question for @ArthurSens / @open-telemetry/prometheus-interoperability SIG I assume
There was a problem hiding this comment.
I think the main other option that might need more time is without_target_info, given the resource spec still needs to be updated to properly handle entities...
There was a problem hiding this comment.
Ok, I've just removed the two latest commits that were changing the stability of translation strategy :)
And I agree with David that handling target info is not 100% clear for us yet, and there's a good chance that with Entities being just around the corner we might need to rethink this
There was a problem hiding this comment.
I'm happy with that being included in this PR or in a separate PR
c703ae6 to
f9892c8
Compare
translation_strategytranslation_strategy enum options are unstable
After #540, we can declare enum values unstable for experimentation with new options. This was blocking the stabilization of Prometheus
translation_strategyconfig option, since not all SDKs can easily adopt certain enum values.In this PR I'm declaring the translation strategy stable, while declaring certain enum options unstable.