[api-metrics]: Create or verify compliance of Counter

[dyladan]

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#counter

• Creation method
  • name (required)
  • unit (optional string)
  • description (optional string)
• Interface
  • Add
    • required increment amount
    • optional attributes
    • optional context
• Noop implementation

Current Status 10/27

Currently we have bound and unbound versions. The unbound version satisfies the value and attribute parameters. In #2559 @legendecas has made the unbound version the only version. The following still needs to be done:

• Add optional context parameter
• Remove unused option constantLabels
• Remove unused option component
• Remove SDK option boundaries (may need SDK changes)
• Remove SDK option aggregationTemporality (may need SDK changes)

[legendecas]

I didn't find any statements on Counter.Add and other synchronous instruments that they should have an optional parameter of context. But there is a statement about associating contexts with synchronous instruments:

Synchronous instruments (e.g. Counter) are meant to be invoked inline with application/business processing logic. For example, an HTTP client could use a Counter to record the number of bytes it has received. Measurements recorded by synchronous instruments can be associated with the Context.

[dyladan]

The "optional context" parameter in the issue description came from exactly that line. Of course you can always get context implicitly but as a general rule we also allow explicit context anywhere implicit context is allowed.

[legendecas]

The unused metrics options constantLabels and component should be removed as they are not part of the spec. boundaries and aggregationTemporality should be configured with metric readers so they should be removed from the MetricOptions too, but there are usages in the current SDK so it might need a refactor.

[legendecas]

IIUC, the attributes of the second parameter of Counter.add is already optional: https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-api-metrics/src/types/Metric.ts#L142

[dyladan]

IIUC, the attributes of the second parameter of Counter.add is already optional: main/experimental/packages/opentelemetry-api-metrics/src/types/Metric.ts#L142

Yes it is. Not sure what I was thinking :)

The unused metrics options constantLabels and component should be removed as they are not part of the spec. boundaries and aggregationTemporality should be configured with metric readers so they should be removed from the MetricOptions too, but there are usages in the current SDK so it might need a refactor.

Agree. Added to the issue description.