Prometheus: improve the conversion of metric labels to data point and resource attributes

specification/compatibility/prometheus_and_openmetrics.md

@@ -29,6 +29,7 @@
   * [Dropped Types](#dropped-types)
   * [Exemplars](#exemplars)
   * [Instrumentation Scope](#instrumentation-scope)
+  * [Metric Attributes](#metric-attributes)
   * [Resource Attributes](#resource-attributes)
 - [OTLP Metric points to Prometheus](#otlp-metric-points-to-prometheus)
   * [Metric Metadata](#metric-metadata-1)
@@ -40,7 +41,7 @@
     + [Histograms as Prometheus NHCB](#histograms-as-prometheus-nhcb)
   * [Exponential Histograms](#exponential-histograms)
   * [Summaries](#summaries-1)
-  * [Metric Attributes](#metric-attributes)
+  * [Metric Attributes](#metric-attributes-1)
   * [Exemplar Conversion](#exemplar-conversion)
   * [Resource Attributes](#resource-attributes-1)
 
@@ -368,42 +369,118 @@
 the translation from Prometheus to OpenTelemetry (e.g. the collector's
 Prometheus receiver).
 
+### Metric Attributes
+
+**Status**: [Development](../document-status.md)
+
+Labels of metrics MUST be copied to data point attributes, except for the
+following labels, which MUST NOT be copied:
+
+- `__name__`,
+- `job` and `instance`,
+- the `le` label of classic Prometheus histogram buckets,
+- the `quantile` label of Prometheus summaries,
+- labels with the `otel_scope_` prefix, which are handled according to the
+  [Instrumentation Scope](#instrumentation-scope) rules.
+
+Labels of the `target` info metric are implicitly excluded, as `target` info
+metric samples are not turned into data points.
+
 ### Resource Attributes
 
 **Status**: [Development](../document-status.md)
 
 When scraping a Prometheus endpoint, resource attributes MUST be added to the
 scraped metrics to distinguish them from metrics from other Prometheus
-endpoints. In particular, `service.name` and `service.instance.id`, are needed
-to ensure Prometheus exporters can disambiguate metrics using
-[`job` and `instance` labels](https://prometheus.io/docs/concepts/jobs_instances/)
-as [described below](#resource-attributes-1).
+endpoints.
+
+NOTE(krajo): this only talks about scraping, but similar rules apply to remote
+write receiver which uses `target` info metric. There are some differences
+though.
+
+TODO(krajo): deal with the remote write receiver use case.
+
+In Prometheus metrics the `job` and `instance` are considered identifying,
+therefore a new resource MUST be created for each `job` and `instance` pair
+encountered in the metrics labels, except for the `target` info metric.
+
+NOTE(krajo): both the prometheusreceiver and prometheusremotewritereceiver
+work this way, thus both scrape and remote write can carry multiple resources
+with the `job` and `instance` being the join key between regular metrics and
+the `target` info metric.
+
+If there is no `job` label on a metric and the metrics are being scraped,
+then the `job` label MUST be considered to be equal to the `job` label of the
+scrape target. During scrape, the `job` label of the target may be equal to the scrape
+job name or customized by relabel rules.
+
+If there is no `job` label on a metric and there is no `job` label for the
+scrape target, the scrape MUST fail. Note that this is different from the
+Prometheus behavior that still scrapes the metrics without `job`.
+
+If there is no `instance` label and the metrics are being scraped, the
+`instance` label MUST be considered to be equal to the `instance` label of the
+scrape target. Note that the `instance` label of the target may be equal to the
+`__address__` of the target or customized by relabel rules.
+
+For each resource the `service.name` attribute MUST be set to the
+`service.name` label in `target` info metric, if that label exists in the
+`target` info metric with the same `job` and `instance` key as the resource.
+Otherwise `service.name` MUST be set to the value of the `job` label.
+
+NOTE(krajo): when an SDK uses default translation, then `service.name` is
+translated into `service_name`, thus the `job` is copied into `service.name`
+and `service_name` is also kept as a resource attribute.
+This doesn't happen if there is no translation.
+
+For each resource the `service.instance.id` attribute MUST be set to the
+`service.instance.id` label in `target` info metric, if that label exists in the
+`target` info metric with the same `job` and `instance` key as the resource.
+Otherwise `service.instance.id` MUST be set to the value of the `instance`
+label.
+
+NOTE(krajo): when an SDK uses default translation, then `service.instance.id` is
+translated into `service_instance_id`, thus the `instance` is copied into
+`service.instance.id` and `service_instance_id` is also kept as a resource
+attribute.
+This doesn't happen if there is no translation.
 
-The following attributes MUST be associated with scraped metrics as resource
-attributes, and MUST NOT be added as metric attributes:
+The following attributes SHOULD be associated with scraped metrics as resource
+attributes, and MUST NOT be added as metric
+attributes:
 
 | OTLP Resource Attribute | Description |
 | ----------------------- | ----------- |
-| `service.name` | The configured name of the service that the target belongs to |
-| `service.instance.id` | A unique identifier of the target.  By default, it should be the `<host>:<port>` of the scraped URL |
+| `server.address` | The `<host>` portion of the `instance` unless it is localhost, a loopback address, or an unspecified address |
+| `server.port` | The `<port>` portion of the `instance` |
+| `url.scheme` | `http` or `https` from the `__scheme__` label on the target |
 
-The following attributes SHOULD be associated with scraped metrics as resource
-attributes, and MUST NOT be added as metric attributes:
+NOTE(krajo): the receiver sets empty string for `server.port` if it cannot be
+deduced, but perheaps this is a bug?
 
-| OTLP Resource Attribute | Description |
-| ----------------------- | ----------- |
-| `server.address` | The `<host>` portion of the target's URL that was scraped |
-| `server.port` | The `<port>` portion of the target's URL that was scraped |
-| `url.scheme` | `http` or `https` |
+NOTE(krajo): there is a bug in the receiver whereby it ignores the relabelling
+of the `__scheme__`, so `url.scheme` always reflects the pre-relabelling state.
+
+Further attributes (such as from k8s service discovery metadata labels) MAY be
+added to the resource attributes. Labels from the `target` info metric take
+precedence over all derived resource attributes.
+
+To summarize, `service.name` and `service.instance.id`, are needed
+to ensure Prometheus exporters can disambiguate metrics using
+[`job` and `instance` labels](https://prometheus.io/docs/concepts/jobs_instances/)
+as [described below](#resource-attributes-1), therefore this specification
+ensures that `service.name` and `service.instance.id` are always defined.
 
 In addition to the attributes above, the
-[target](https://github.com/prometheus/OpenMetrics/blob/v1.0.0/specification/OpenMetrics.md#supporting-target-metadata-in-both-push-based-and-pull-based-systems)
+[`target`](https://github.com/prometheus/OpenMetrics/blob/v1.0.0/specification/OpenMetrics.md#supporting-target-metadata-in-both-push-based-and-pull-based-systems)
  info metric is used to supply additional resource attributes. If present,
-the `target` info metric MUST be dropped from the batch of metrics, and all labels from
-the `target` info metric MUST be converted to resource attributes
-attached to all other metrics which are part of the scrape. By default, label
-keys and values MUST NOT be altered (such as replacing `_` with `.` characters
-in keys).
+the `target` info metric MUST be dropped from the batch of metrics, and all labels
+except `job`, `instance` and `__name__` from the `target` info metric MUST be
+converted to resource attributes attached to all other metrics which are part of
+the same resource identified by `job` and `instance`.
+
+By default, label keys and values of the `target` info metric MUST NOT be
+altered (such as replacing `_` with `.` characters in keys).
 
 ## OTLP Metric points to Prometheus
 
@@ -774,6 +851,15 @@
 `<service.name>` if namespace is empty, to form the `job` metric label.  The
 `service.instance.id` attribute, if present, MUST be converted to the
 `instance` label; otherwise, `instance` should be added with an empty value.
+
+NOTE(krajo): implicitly: existing `job` data point attribute would be
+overwritten, however it is kept if there is no `service.name`.
+
+NOTE(krajo): both prometheusremotewriteexporter and the Prometheus OTLP endpoint
+keep track of existing `target` info metrics by the full label set, not just
+`job` and `instance` , so theoretically you can end up with two `target` info
+metric that have the same `job` and `instance`, but different attributes.
+
 Other resource attributes SHOULD be converted to a
 [target](https://github.com/prometheus/OpenMetrics/blob/v1.0.0/specification/OpenMetrics.md#supporting-target-metadata-in-both-push-based-and-pull-based-systems)
 info metric, or MUST be dropped. The `target` metric is an info-typed metric