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

There MUST NOT be any API for creating an instrument other than with a Meter #3454

Closed
MrAlias opened this issue Nov 8, 2022 · 2 comments · Fixed by #3530
Closed

There MUST NOT be any API for creating an instrument other than with a Meter #3454

MrAlias opened this issue Nov 8, 2022 · 2 comments · Fixed by #3530
Assignees
Labels
area:metrics Part of OpenTelemetry Metrics bug Something isn't working pkg:API Related to an API package

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Nov 8, 2022

Currently the OpenTelemetry specification states:

There MUST NOT be any API for creating a Counter other than with a Meter

There MUST NOT be any API for creating an Asynchronous Counter other than with a Meter

There MUST NOT be any API for creating a Histogram other than with a Meter

There MUST NOT be any API for creating an Asynchronous Gauge other than with a Meter

There MUST NOT be any API for creating an UpDownCounter other than with a Meter

There MUST NOT be any API for creating an Asynchronous UpDownCounter other than with a Meter

The opentelemetry-go project currently provides the asyncint64.InstrumentProvider, asyncfloat64.InstrumentProvider, syncint64.InstrumentProvider, and syncfloat64.InstrumentProvider APIs to create these instruments.

@MadVikingGod
Copy link
Contributor

Has there been any recorded feedback that the current API is non-compliant?

My issue with this interpretation is that it is stricter than other SDKs have taken. Java, python, and probably others use a builder pattern, where the meter's API returns an intermediate object that can be used to create the actual instrument.

We decided on the current format in march (#2526), what has materially changed to churn like this?

@MrAlias
Copy link
Contributor Author

MrAlias commented Dec 9, 2022

Has there been any recorded feedback that the current API is non-compliant?

This issue acts as a source of recorded feedback, it is stating the non-compliance of our implementation. Additionally, #2653 and #2653 (comment)

My issue with this interpretation is that it is stricter than other SDKs have taken. Java, python, and probably others use a builder pattern, where the meter's API returns an intermediate object that can be used to create the actual instrument.

To be clear, Java indeed does use a builder pattern, though Python does not. Also, javascript does not (I'm pretty sure .NET does, but cannot find the link as I get lost looking at that repo).

I agree that the builder pattern may not adheres to the normative requirements of the specification, and that these other language implementation are out of compliance. This was a reason I did not suggest moving to a builder pattern in this issue. That said, I would be open to exploring the builder pattern if there is evidence this was intended to be allowed by the specification (though this would run into style issues with our expected options pattern).

To be clear our instrument provider approach is not equivalent to the builder pattern other languages provide. The builder pattern is an alternative approach to building up the configuration of an instrument during its construction. This is both a common pattern for those languages and can be for Go. Our current approach provides configuration with option parameters, the instrument providers are solely intended as APIs to construct instruments from (as stated in the linked issue). This being the sole intention of the instrument providers, means they are an additional interface we export for users to create instruments other than with a Meter. Hence, they directly violate the specification requirement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics bug Something isn't working pkg:API Related to an API package
Projects
No open projects
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants