Remove with_ prefix from prometheus exporter property names

[jack-berg]

See open-telemetry/opentelemetry-specification#5056 (review)

• without_scope_info -> scope_info_enabled (default true)
• without_target_info/development -> target_info_enabled/development (default true)
• with_resource_constant_labels -> resource_constant_labels (default none are added)

Corresponding spec PR was merged: open-telemetry/opentelemetry-specification#5071

[marcalff]

Looks much better without double negations removing the without_ prefix.

A few questions:

1. why still draft, looks ready for review

2. why a boolean property foo_enabled, as opposed to just foo.
   I am sure this has been discussed elsewhere, but can not find the reference.

resource_constant_labels is not named resource_constant_labels_enabled, so this looks inconsistent.

I would prefer just scope_info, target_info and resource_constant_labels, unless there is a good reason for the enabled suffix.

[jack-berg]

why still draft, looks ready for review

No reason for it to be a draft but it shouldn't be merged before corresponding spec PR (link in description).

why a boolean property foo_enabled, as opposed to just foo.
I am sure this has been discussed elsewhere, but can not find the reference.

I'm not sure this has been discussed actually - at least not in the context of the declarative config schema. There has been discussion at the spec level. Currently, the spec guidance for for env vars was to included either *_ENABLED or *_DISABLED based on making sure a default value of false aligns with the desired default behavior. This was clarified to only apply to env vars (i.e. not other config interfaces like programmatic or declarative config) here.

The original issue for that PR discusses prior art in the java agent and collector standardizing on *_ENALBED for all properties, regardless of their default.

So IMO, the realistic options for us in declarative config are:

• *_enabled suffix for all boolean properties: Aligns with prior art in collector and java agent. Provides an obvious signal that the property is boolean type (questionably value).
• omit any *_enabled / *_disabled: Name boolean types like any other properties without any special indication of their type in their name.

I will say that names like scope_info / target_info feel less self describing to me that scope_info_enabled / target_info_enabled. Its not clear to me what a scope_info property does, but I have better intuition that scope_info_enabled is a toggle to turn scope info on and off.

resource_constant_labels is not named resource_constant_labels_enabled, so this looks inconsistent.

resource_constant_labels is more than just a boolean - its a description of which resources should be added as labels. In the case of the declarative config schema, we use IncludeExclude to model this:

https://github.com/open-telemetry/opentelemetry-specification/blame/5d59108fe78264b7546dc1c971814956decdb916/specification/metrics/sdk_exporters/prometheus.md#L136

[jack-berg]

FYI, the upstream spec PR has been merged: open-telemetry/opentelemetry-specification#5071

We could re-open the conversation at the spec level, or adopt a stance in declarative config schema that we generally omit _enabled suffixes on boolean types.

But the simpler approach is just to follow the naming of the upstream spec, which is what I advocate for.

[marcalff]

LGTM