[metrics]: Use API common Attributes definition

[dyladan]

Related to open-telemetry/opentelemetry-js-api#130 but does not depend on it because the public interface of the API will not change.

Depends on #2586

[legendecas]

Submitted a spec issue on re-using the common Attribute interface: open-telemetry/opentelemetry-specification#2143

[dyladan]

Submitted a spec issue on re-using the common Attribute interface: open-telemetry/opentelemetry-specification#2143

Was advised by @jsuereth to emulate the collector behavior while the prometheus spec is finished.

[legendecas]

to emulate the collector behavior while the prometheus spec is finished

@dyladan can you elaborate on the emulation details? Like say do we need to export 0 and "0" to prometheus both as "0"?

[github-actions]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[dyladan]

Looks like the collector handles this here: https://github.com/open-telemetry/opentelemetry-collector/blob/model/v0.45.0/model/pdata/common.go#L370

The suggested mapping to follow the collector:

• "string" => "string"
• true => "true"
• false => "false"
• int => base 10 string representation
• floating point => same as go's strconv.FormatFloat(doubleVal, 'f', -1, 64)
  • format byte 'f' (-ddd.dddd, no exponent)
  • The special precision -1 uses the smallest number of digits necessary such that ParseFloat will return f exactly.
  • bitsize not important for javascript
• map => json.Marshal(a.MapVal().AsRaw())
  • probably fine to do JSON.stringify
• bytes => base64 encoded string
• array => JSON
• unknown type => fmt.Sprintf("<Unknown OpenTelemetry attribute value type %q>", a.Type())

[github-actions]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[gogreen53]

I'd like to put in a huge +1 to having this done. I'm working on metrics now and having to export a set of attributes to include in every call to an instrument is causing my eye to twitch. :)

Is there any ETA? Thanks!

[dyladan]

I'd like to put in a huge +1 to having this done. I'm working on metrics now and having to export a set of attributes to include in every call to an instrument is causing my eye to twitch. :)

Is there any ETA? Thanks!

Not sure this issue is what you think it is. This is just using the Attributes interface instead of SpanAttributes/MetricAttributes

[Flarna]

if we use type Attributes in metrics API it will depend on API 1.1.
In #2892 we moved to use API 1.0 and removed using of 1.1 types in SDK.

This seems to got in the opposite direction.

[dyladan]

There is no metrics API in 1.0 or 1.1 anyway so any metrics users will need to be at least on 1.2 (or whatever version we add metrics to the main API package).

[Flarna]

well, @opentelemetry/instrumentation depends on metrics API. So there are users effected not using metrics at all.

[vmarchaud]

@opentelemetry/instrumentation depends on metrics

I fail to see in which scenario this will be a problem, there will be breaking changes for instrumentation anyway since metrics types will change no ?

[Flarna]

It's not really a problem. I have no problme with API 1.x
Most people wouldn't expect a side effect from metics to instrumentations. We have ^1.0.0 range for API at most places (e.g. all instrumentations) but as their common base requires 1.1 that could be a bit confusing.
But NPM,... is hopefully able to dedupe this.

[vmarchaud]

We have ^1.0.0 range for API at most places (e.g. all instrumentations) but as their common base requires 1.1 that could be a bit confusing.

I meant that we'll need to bump all instrumentations to require 1.2 (or whatever will be the metrics version) since need to depend on the new API so duplicate issue will solves itself (at least until we ship another api minor) ?

[legendecas]

@dyladan Sorry, I just missed the fact that this issue was assigned before I got my hands on the stale issue #2585 😅.

if we use type Attributes in metrics API it will depend on API 1.1.

I'd find the most smooth adoption path for the Attributes interface is

• Stick to SpanAttributes before we merge the @opentelemetry/api-metrics with @opentelemetry/api. This would allow us to not have two versions of @opentelemetry/api in our test set ups.
• Migrate to Attributes once the two API packages are merged. Any packages that are expecting metrics APIs to be available in @opentelemetry/api, they'll need to use the latest version anyways.

WDYT?

[dyladan]

Sounds good to me