feat(metrics-api): use common attributes definitions

[legendecas]

Which problem is this PR solving?

Use common attributes definitions.

Fixes #2585
Fixes #2587
Fixes #1482

Short description of the changes

• Allow non-string values like array, number, strings in MetricAttributes.
• Export non-string values to Prometheus with the transformation defined in https://github.com/open-telemetry/opentelemetry-specification/blob/v1.12.0/specification/common/README.md#attribute.

Type of change

• New feature (non-breaking change which adds functionality)

How Has This Been Tested?

• hashAttributes
• PrometheusSerializer
• OTLP transformer

Checklist:

• Followed the style guidelines of this project
• Unit tests have been added
• Documentation has been updated

[codecov]

Codecov Report

Merging #3038 (2bd0e24) into main (18dce78) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 2bd0e24 differs from pull request most recent head 43acd77. Consider uploading reports for the commit 43acd77 to get more accurate results

@@            Coverage Diff             @@
##             main    #3038      +/-   ##
==========================================
- Coverage   93.08%   93.08%   -0.01%     
==========================================
  Files         188      188              
  Lines        6246     6243       -3     
  Branches     1313     1313              
==========================================
- Hits         5814     5811       -3     
  Misses        432      432              

Impacted Files	Coverage Δ
...ages/opentelemetry-api-metrics/src/types/Metric.ts	100.00% <ø> (ø)
...ry-exporter-prometheus/src/PrometheusSerializer.ts	95.16% <100.00%> (ø)
...ckages/opentelemetry-sdk-metrics-base/src/utils.ts	97.72% <100.00%> (-0.15%)	⬇️

[dyladan]

LGTM I just want to make sure I und

[dyladan]

Are you sure it is safe to use this as a separator if we're allowing string values? I think it is possible to collide hashes, but do we think it is sufficiently unlikely?

[legendecas]

For string values, it is always possible to collide before or after this change, like:

• key1 = value1, and key2 = value2, results in |#key1:value1,key2:value2,
• key1 = value1,key2=value2, also results in |#key1:value1,key2:value2.

The change here is mainly to circumvent the string value of an array of strings, like ["value1", "value2"].toString() is value1,value2 in which values are separated with ",".

[dyladan]

Do we want to consider trying to make it impossible to collide? Or is this good enough?

[legendecas]

For what is in my mind now, it can be costly to do so. This hashing is very common across the operations in metrics SDK. I would be open to any improvements to this function but I'd be hesitant to make it a relatively costly one.

[legendecas]

@dyladan I've updated the hash function to support arbitrary attribute values with JSON.stringify. PTAL :)

[legendecas]

Just forgot to update the OTLP exporter tests 😅. Marking this as a draft again.

Done.

[legendecas]

@dyladan I noticed that you removed this PR from the metrics GA milestone. Is there any specific reason to exclude this from metrics GA?

[dyladan]

@dyladan I noticed that you removed this PR from the metrics GA milestone. Is there any specific reason to exclude this from metrics GA?

I excluded all PRs from the milestone since they already have issues. It was distorting the %completion number

[legendecas]

Thank you for the explanation. Make sense to me.

[pichlermarc]

Looks good 🙂
Thanks for working on this!

[dyladan]

Thanks. I prefer the more robust hash even if it is slightly more costly.

[legendecas]

Fixed tests introduced on the main branch which need to be updated accordingly in this PR.