-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Audit the addition of the synchronous gauge #5369
Comments
From the exported API, the only way to create a (non-trivial opentelemetry-go/metric/meter.go Lines 61 to 64 in ebd0ade
opentelemetry-go/metric/meter.go Lines 111 to 114 in ebd0ade
|
We do not follow this optional. Our names are Given the optional nature, we remain compliant. |
Our interface type declarations require the type to not return a value: opentelemetry-go/metric/syncfloat64.go Lines 187 to 191 in ebd0ade
opentelemetry-go/metric/syncint64.go Lines 187 to 191 in ebd0ade
|
Our interface type declarations require a value parameter and can accept options: opentelemetry-go/metric/syncfloat64.go Lines 187 to 191 in ebd0ade
opentelemetry-go/metric/syncint64.go Lines 187 to 191 in ebd0ade
The opentelemetry-go/metric/instrument.go Lines 339 to 347 in ebd0ade
These opentelemetry-go/metric/instrument.go Lines 283 to 288 in ebd0ade
We comply with this requirement from the specification. |
We do not provide the optional ability to accept 'flexible attributes' during the gauge creation. We do comply with the requirement to accept attributes at "invocation time" as outlined here. |
See the general requirements for synchronous instruments. https://opentelemetry.io/docs/specs/otel/metrics/api/#synchronous-instrument-api
opentelemetry-go/metric/meter.go Lines 61 to 64 in ebd0ade
opentelemetry-go/metric/meter.go Lines 111 to 114 in ebd0ade
We comply with the required structure of accepting a The recommendation to document what the name should be is not present. Nor is is present on any other method. We need to track this as a fix for all methods. It is not blocking for this issue. We do not validate the name in the API.
We do not validate the unit.
There are no stable advisory parameters that apply to this instrument kind: https://opentelemetry.io/docs/specs/otel/metrics/api/#instrument-advisory-parameters When applicable advisory parameters are added or stabilized they can be added as options. |
The aggregation selection in the SDK: opentelemetry-go/sdk/metric/pipeline.go Lines 449 to 452 in ebd0ade
For delta temporality: opentelemetry-go/sdk/metric/internal/aggregate/aggregate.go Lines 80 to 81 in ebd0ade
For cumulative temporality: opentelemetry-go/sdk/metric/internal/aggregate/aggregate.go Lines 82 to 83 in ebd0ade
We comply with this requirement. |
delta: opentelemetry-go/sdk/metric/internal/aggregate/lastvalue.go Lines 63 to 80 in ebd0ade
cumulative: opentelemetry-go/sdk/metric/internal/aggregate/lastvalue.go Lines 82 to 98 in ebd0ade
With opentelemetry-go/sdk/metric/internal/aggregate/lastvalue.go Lines 24 to 31 in ebd0ade
|
@dashpole (and all other @open-telemetry/go-approvers) I believe I've completed the audit. If you concur, please go ahead and close this issue. |
From #5369 (comment)
Tracking in #5377 |
Thanks @MrAlias. LGTM |
#5304
Added to the spec here
The text was updated successfully, but these errors were encountered: