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.

Sign up for GitHub

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

Jump to bottom

Consider removing bound metric instrument support #2351

Closed
jmacd opened this issue Nov 5, 2021 · 0 comments · Fixed by #2399
Closed

Consider removing bound metric instrument support #2351

jmacd opened this issue Nov 5, 2021 · 0 comments · Fixed by #2399
Labels
enhancement New feature or request

Comments

@jmacd
Copy link
Contributor

jmacd commented Nov 5, 2021

Problem Statement

Bound metric instruments have a large API surface area and very few members of the OTel-Go community have defended keeping them. Although the corresponding Prometheus client library has this concept, it's not clearly needed in OTel-Go at this time.

Proposed Solution

Remove this feature until it has become part of the OTel specification. The Metrics SIG has discussed working on this after Metrics API and SDK reach 1.0.

Alternatives

This can be kept. I don't find myself wanting it.

Prior Art

See the Prometheus-Go client.

Additional Context

The SDK structure will remain. The existing concurrency mechanism and support for "forgetting" metric instruments when configured with delta temporality means that the mechanism behind bound instruments will stay in place. It will be easy to add this feature again.
@jmacd jmacd added the enhancement New feature or request label Nov 5, 2021
@jmacd jmacd mentioned this issue Nov 5, 2021
Closed
@MadVikingGod MadVikingGod mentioned this issue Nov 16, 2021
Merged
@vmihailenco vmihailenco mentioned this issue Dec 22, 2021
Merged
nicoche added a commit to koyeb/kreconciler that referenced this issue May 2, 2022
@nicoche
Upgrade observability dependencies
8ad4ae2 
* Bump go (1.16 -> 1.18)
* Bump otel (0.16.0 -> 1.7.0 / 0.30.0)

A lot of stuff we used have been moved around. Traces are now stable in
otel so we can expect this to stay still; metrics are still in alpha
though so compat may break again in the future.
Notables changes:
* metric.Must has been removed; some constructors now have their
  signature changed. We can introduce a custom Must so we don't
  change those signatures
* metric.Bind has been removed from the metrics package. That introduces
  a little more complexity to the code.  I understood that
  (open-telemetry/opentelemetry-go#2351) it
  might be reintroduced in the future
@nicoche nicoche mentioned this issue May 2, 2022
Merged
nicoche added a commit to koyeb/kreconciler that referenced this issue May 9, 2022
@nicoche
Bump observability stack (#6)
9510eb8 
* Fix typo in README.md

* Upgrade observability dependencies

* Bump go (1.16 -> 1.18)
* Bump otel (0.16.0 -> 1.7.0 / 0.30.0)

A lot of stuff we used have been moved around. Traces are now stable in
otel so we can expect this to stay still; metrics are still in alpha
though so compat may break again in the future.
Notables changes:
* metric.Must has been removed; some constructors now have their
  signature changed. We can introduce a custom Must so we don't
  change those signatures
* metric.Bind has been removed from the metrics package. That introduces
  a little more complexity to the code.  I understood that
  (open-telemetry/opentelemetry-go#2351) it
  might be reintroduced in the future
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
1 participant
@jmacd