Part of https://github.com/open-telemetry/opentelemetry-go/issues/8110
Related to
https://github.com/open-telemetry/opentelemetry-go/issues/5882.
I'm hoping to find a better way to support experimental Options types in
our API packages. This is one approach to consider.
This contains no public API changes. It introduces a type:
`ExperimentalOption` in `/metric/internal/x`, which can be used by our
experimental options defined outside of the module. Options that embed
this interface are ignored by `New*Config` builder functions in the
metrics API to prevent them from panicing when used. Only SDKs that
explicitly support the experimental option in question will respect it.
Alternative SDKs will ignore the experimental options. We would still
need to treat ExperimentalOption as a stable artifact, since the SDK
will indirectly depend on it.
See
https://github.com/open-telemetry/opentelemetry-go/compare/main...dashpole:opentelemetry-go:attributes_advisory
for how this would be used to support the advisory attributes parameter.
The
[specifications](https://opentelemetry.io/docs/specs/otel/metrics/api/#asynchronous-instrument-api)
about callbacks say:
> Callback functions MUST be documented as follows for the end user:
> - **Callback functions SHOULD be reentrant safe.** The SDK expects to
evaluate callbacks for each MetricReader independently.
> - **Callback functions SHOULD NOT take an indefinite amount of time.**
> - Callback functions SHOULD NOT make duplicate observations (more than
one Measurement with the same attributes) across all registered
callbacks.
>
> The resulting behavior when a callback violates any of these
RECOMMENDATIONS is explicitly not specified at the API level.
Currently the Go API mentions that callbacks "needs to complete in a
finite amount of time" and "needs to be concurrent safe", but does not
mention reentrancy.
And while close in meaning, there is a nuance between reentrant and
concurrent safe, and a function locking a plain (non-recursive) mutex is
an example of a function that is concurrent safe while not being
reentrant.
Furthermore, in the Go implementation provided in this repo, the metrics
pipeline locks a mutex (in `produce`) then calls all callbacks, which
means that locking a mutex from within a callback would mean holding 2
mutexes at the same time, which opens the door to a deadlock.
In conclusion, users should be made aware in no uncertain terms that
locking a mutex from callbacks is discouraged and not a supported
behavior
(Relates to the discussion in #7755)
Co-authored-by: Damien Mathieu <42@dmathieu.com>
David Ashpole
Antoine Gagniere
Tyler Yahn
Nathan Baulch
Robert Pająk