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>
* Add measurement docs to metric API
Based on user feedback, how measurements are made with observable
instruments is not immediately clear to new users. This adds
documentation intended to help guide these users.
* Apply feedback
* Apply feedback
* Remove backticks
* Apply suggestions from code review
Co-authored-by: Robert Pająk <pellared@hotmail.com>
* Update metric/meter.go
Co-authored-by: Robert Pająk <pellared@hotmail.com>
---------
Co-authored-by: Robert Pająk <pellared@hotmail.com>
Co-authored-by: Chester Cheung <cheung.zhy.csu@gmail.com>
* docs(typos): Run codespell to fix typos
There were a lot of typos through the repository, so I ran
[codespell][], a tool for automatically fixing typos, to fix them.
```console
make codespell
```
There's already a tool called [misspell][] that's supposed to take care
of this, but misspell hasn't been updated for 6 years, and it doesn't
seem to be catching any of the typos that codespell can.
[codespell]: https://github.com/codespell-project/codespell
[misspell]: https://github.com/client9/misspell
* Revert and ignore spelling for Consequentially
* Add GH workflow for codespell
* Revert GH Workflow and Makefile for codespell
Per @pellared, since there's no instructions for setting up codespell,
it was suggested that the changes for setting up a workflow and section
in Makefile include instructions for setting up codespell as well.
* Revert spelling on consequently
---------
Co-authored-by: Chester Cheung <cheung.zhy.csu@gmail.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
* PoC of embedded private method ifaces
* Rename embed to embedded
* Add an embedded iface for all instruments
* Fix metric/instrument tests
* Fix global and otel
* Fix SDK
* Comment the embedded pkg types
* Update the embedded pkg docs
* Update otel/metric docs about impls
* Update otel/metric type docs on impl
* Update docs in otel/metric/instrument on default
* Add changes to changelog
* Apply suggestions from code review
Co-authored-by: Robert Pająk <pellared@hotmail.com>
* Apply feedback on URLs
* Reword based on feedback
* Make it clear we only recommended embedding noop
* Ignore links with godot linter
---------
Co-authored-by: Robert Pająk <pellared@hotmail.com>
* Update metric API documentation
Remove the warning on otel/metric not being GA.
Document the otel/metric and otel/metric/instrument package for
instrumenters.
* Remove unrendered links in MeterProvider.Meter doc
* Clarify synchronous and asynchronous
* Fix misspelling
* Update metric/instrument/doc.go
Co-authored-by: Robert Pająk <pellared@hotmail.com>
* Update metric/instrument/doc.go
Co-authored-by: Robert Pająk <pellared@hotmail.com>
* Update metric/instrument/doc.go
Co-authored-by: Robert Pająk <pellared@hotmail.com>
* Update metric/instrument/doc.go
Co-authored-by: Robert Pająk <pellared@hotmail.com>
* Apply feedback
* Apply suggestions from code review
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
---------
Co-authored-by: Chester Cheung <cheung.zhy.csu@gmail.com>
Co-authored-by: Robert Pająk <pellared@hotmail.com>
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
* Update RegisterCallback and Callback decls
RegisterCallback accept variadic Asynchronous instruments instead of a
slice.
Callback accept an observation result recorder to ensure instruments
that are observed by a callback.
* Update global impl
* Update noop impl
* Update SDK impl
* Fix prometheus example
* Fix metric API example_test
* Remove unused registerabler
* Rename ObservationRecorder to MultiObserver
* Update Callback documentation about MultiObserver
* Remove the Observe method from async inst
* Revert to iface for Observers
* Fix async inst docs
* Update global async delegate race test
* Restore removed observe doc
* Remove TODO
* Remove stale comment
* Update changelog
* Update RegisterCallback and Callback declerations
RegisterCallback accepts variadic Asynchronous instruments instead of a
slice.
Callback accepts an observation result recorder to ensure instruments
that are observed by a callback.
* Update global, noop, SDK implementations
* Fix examples
* Add changes to changelog
* Test RegisterCallback for invalid observers
* Test callbacks from foreign sources not collected
* Support registering delegating instruments
* Restructure RegisterCallback method
Instead of accepting instruments to register the callback with as a
slice, accept them as variadic arguments.
* Add changes to changelog
* Add PR number to changes
* Split metric inst config
Instead of having the same configuration for both the Synchronous and
Asynchronous instruments, use specific options for both.
* Use Async/Sync opt for appropriate inst
* Update noop inst providers
* Update internal global impl
* Update sdk
* Remove unused method for callbackOption
* Test instrument configuration
* Lint imports
* Add changes to changelog
* Refactor callbacks and further split opts
Define callbacks to return the value observed. Because of the different
types returned for different observables, the callbacks and options are
move to the sync/async packages.
* Update noop impl
* Fix example_test.go
* Fix internal impl
* Update Callbacks
Return observations for distinct attr sets.
* Refactor common code in sdk/metric inst provider
* Update examples and prom exporter
* Generalize callback
* Update changelog
* Add unit tests for callback
* Add meter tests for cbacks on creation
* Rename Observations to Measurements
* Update Callback to accept an Observer
* Update SDK impl
* Move conf to instrument pkg
* Apply suggestions from code review
* Create metric API Callback type
Document the type according the OTel specification requirements.
* Update all impls of the metric API with new type
* Add changes to changelog
* Update PR number in changelog entry
* Update Meter RegisterCallback method
Return a Registration from the method that can be used by the caller to
unregister their callback.
Update documentation of the method to better explain expectations of
use and implementation.
* Update noop impl
* Update global impl
* Test global Unregister concurrent safe
* Use a map to track reg in global impl
* Update sdk impl
* Use a list for global impl
* Fix prom example
* Lint metric/meter.go
* Fix metric example
* Placeholder for changelog
* Update PR number in changelog
* Update sdk/metric/pipeline.go
Co-authored-by: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com>
* Add test unregistered callback is not called
Co-authored-by: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com>
* docs(metric): document public interfaces with versioning policy
* docs(metric): remove versioning policy warning from interfaces without public method(s)
* Add WithScopeAttributes MeterOption to metric pkg
* Add MeterConfig unit tests
* Add changes to changelog
* Fix import linting
* Update MeterProvider documentation
Include information about how to use WithScopeAttributes.
* Empty All of the metrics dir
* Add instrument api with documentation
* Add a NoOp implementation.
* Updated to the new config standard
* Address PR comments
* This change moves components to sdk/metrics
The Moved components are:
- metric/metrictest
- metric/number
- metric/internal/registry
- metric/sdkapi
* The SDK changes necessary to satasify the new api
* This fixes the remaing tests.
* Update changelog
* refactor the Noop meter and instruments into one package.
* Renamed pkg.Instruments to pkg.InstrumentProvider
Co-authored-by: Aaron Clawson <MadVikingGod@users.noreply.github.com>
David Ashpole
Antoine Gagniere
Tyler Yahn
Robert Pająk
Charlie Le
Wisdom Matthew
Aaron Clawson
Aaron Clawson