open-telemetry / opentelemetry-specification

Specifications for OpenTelemetry
https://opentelemetry.io
Apache License 2.0
3.75k stars 889 forks source link

Stabilize synchronous gauge instrument API #3893

Closed jack-berg closed 6 months ago

jack-berg commented 8 months ago

Synchronous gauge support was added in #3540 (August 2023). This is a tracking issue for stability.

Below are the PRs / issues tracking prototyping of this in various languages.

This comment seems to indicate that there may be prototypes available in C++ and .NET, but I couldn't find evidence of that.

Maintainers, please chime in on this issue with any relevant updates on prototypes.

joaopgrassi commented 8 months ago

I noticed the sync gauge is not listed in the table here: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#sum-aggregation. Should it?

pichlermarc commented 7 months ago

There is an API and SDK prototype for JavaScript now, we'll soon merge an experimental version of this. https://github.com/open-telemetry/opentelemetry-js/pull/4528

jack-berg commented 7 months ago

Thanks @pichlermarc - updated the issue descriptino to reflect that.

I noticed the sync gauge is not listed in the table here: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#sum-aggregation. Should it?

@joaopgrassi yes it should be.

trask commented 6 months ago

we have 3 prototypes now:

I think that may be enough to move forward with stabilization?

jack-berg commented 6 months ago

Its a bit of a grey area on whether something counts as a prototype if its a PR which hasn't been merged, or maybe the PR has been merged but it isn't consumable in a published artifact.

I'm inclined to say that this counts as a prototype, so long as the maintainers of that language confirm they didn't uncover any design issues along the way. After all, when we say that spec PRs much have associated prototypes, those are almost always just draft PRs or branches since a stronger requirement would cause a chicken and the egg problem.

@open-telemetry/javascript-maintainers can you comment on the JS PR linked above? Did you uncover any design issues along the way?

@open-telemetry/specs-metrics-approvers WDYT? Any objection to moving forward to stabilizing?

reyang commented 6 months ago

@open-telemetry/specs-metrics-approvers WDYT? Any objection to moving forward to stabilizing?

I don't see issue here - synchronous gauge is something we were hoping to add but got deprioritized as we wanted to ship the initial version of metrics API/SDK spec. It is just yet-another-instrument so I don't anticipate a lot of controversy.

pichlermarc commented 6 months ago

can you comment on the JS PR linked above? Did you uncover any design issues along the way?

No design issues. :slightly_smiling_face:

In the JS SIG, we just don't have a proper mechanism in place yet to publish an experimental addition to the API, hence we cannot merge the PR until the spec stable or we've come up with such a mechanism.

Prototyping for experimental API features it is held up on a tooling update (https://github.com/open-telemetry/opentelemetry-js/pull/4648)

MrAlias commented 6 months ago

Go PoC: https://github.com/open-telemetry/opentelemetry-go/pull/5280

cijothomas commented 6 months ago

OTel Rust PoC (available under otel-unstable feature flags). https://github.com/open-telemetry/opentelemetry-rust/pull/1410 Tagging @lalitb who implemented the above as well.

cijothomas commented 6 months ago

This https://github.com/open-telemetry/opentelemetry-specification/issues/2318#issuecomment-1868252688 seems to indicate that there may be prototypes available in C++ and .NET, but I couldn't find evidence of that.

No prototype in .NET yet.