Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stabilize synchronous gauge instrument API #3893

Closed
3 of 11 tasks
jack-berg opened this issue Feb 21, 2024 · 10 comments · Fixed by #4019
Closed
3 of 11 tasks

Stabilize synchronous gauge instrument API #3893

jack-berg opened this issue Feb 21, 2024 · 10 comments · Fixed by #4019
Assignees
Labels
spec:metrics Related to the specification/metrics directory triage:accepted:ready

Comments

@jack-berg
Copy link
Member

jack-berg commented Feb 21, 2024

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.

@jack-berg jack-berg added the spec:metrics Related to the specification/metrics directory label Feb 21, 2024
@joaopgrassi
Copy link
Member

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?

@reyang reyang added the triaged-accepted The issue is triaged and accepted by the OTel community, one can proceed with creating a PR proposal label Feb 28, 2024
@pichlermarc
Copy link
Member

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

@jack-berg
Copy link
Member Author

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.

jack-berg added a commit that referenced this issue Apr 5, 2024
Related to #3893. 

The table has entries for all the instrument types but I forgot to add
an entry for synchronous gauge in #3540. Fixing.
@trask
Copy link
Member

trask commented Apr 23, 2024

we have 3 prototypes now:

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

@jack-berg
Copy link
Member Author

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
Copy link
Member

reyang commented Apr 29, 2024

@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
Copy link
Member

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

No design issues. 🙂

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 (open-telemetry/opentelemetry-js#4648)

@MrAlias
Copy link
Contributor

MrAlias commented Apr 29, 2024

Go PoC: open-telemetry/opentelemetry-go#5280

@cijothomas
Copy link
Member

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

@cijothomas
Copy link
Member

This #2318 (comment) 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.

@austinlparker austinlparker added triage:accepted:ready and removed triaged-accepted The issue is triaged and accepted by the OTel community, one can proceed with creating a PR proposal labels Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:metrics Related to the specification/metrics directory triage:accepted:ready
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants