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

Verify compliant metric API specification implementation: Instrument Asynchronous Counter #3375

Closed
2 tasks done
Tracked by #3383
MrAlias opened this issue Oct 20, 2022 · 11 comments
Closed
2 tasks done
Tracked by #3383
Assignees
Labels
area:metrics Part of OpenTelemetry Metrics pkg:API Related to an API package

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Oct 20, 2022

  • Identify all the normative requirements, recommendations, and options the specification defines as comments to this issue
  • Ensure the current metric API implementation is compliant with these normative requirements, recommendations, and options in those comments.
@MrAlias MrAlias added area:metrics Part of OpenTelemetry Metrics pkg:API Related to an API package labels Oct 20, 2022
@MrAlias
Copy link
Contributor Author

MrAlias commented Nov 8, 2022

#3453

@MrAlias
Copy link
Contributor Author

MrAlias commented Nov 8, 2022

#3454

@MrAlias MrAlias self-assigned this Jan 5, 2023
@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 5, 2023

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

With #3454 now complete, our implementation is compliant.

@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 5, 2023

This MAY be called CreateObservableCounter. If strong type is desired, OpenTelemetry API authors MAY decide the language idiomatic name(s), for example CreateUInt64ObservableCounter, CreateDoubleObservableCounter, CreateObservableCounter<UInt64>, CreateObservableCounter<double>.

We do not follow this optional criteria. We instead follow more common Go method naming and use Int64ObservableCounter and Float64ObservableCounter.

@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 5, 2023

It is highly recommended that implementations use the name ObservableCounter (or any language idiomatic variation, e.g. observable_counter) unless there is a strong reason not to do so.

I'm going to assume the authors intended "recommended" to be the normative key word "RECOMMENDED".

We do not follow this recommendation. #3453 is tracking this issue.

@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 5, 2023

OpenTelemetry API authors MAY decide what is the idiomatic approach. Here are some examples:

  • Return a list (or tuple, generator, enumerator, etc.) of Measurements.
  • Use an observable result argument to allow individual Measurements to be reported.

This looks to be an inappropriate use of the normative key word "MAY" here. For what it's worth, we do choose the option to design an idiomatic approach.

The single callback design is being added in #3507, and the multi-callback is being defined in #3564.

@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 5, 2023

User code is recommended not to provide more than one Measurement with the same attributes in a single callback. If it happens, OpenTelemetry SDK authors MAY decide how to handle it in the SDK. For example, during the callback invocation if two measurements value=1, attributes={pid:4, bitness:64} and value=2, attributes={pid:4, bitness:64} are reported, OpenTelemetry SDK authors MAY decide to simply let them pass through (so the downstream consumer can handle duplication), drop the entire data, pick the last one, or something else.

This is all not relevant to the API.

@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 5, 2023

The API MUST treat observations from a single callback as logically taking place at a single instant, such that when recorded, observations from a single callback MUST be reported with identical timestamps.

The API does not handle collection cycles or the timestamping of observations, that is done by implementations receiving data from the API. Our implementation seems to comply with this as it treats observations agnostically regarding the time an observation occurs.

@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 5, 2023

The API SHOULD provide some way to pass state to the callback. OpenTelemetry API authors MAY decide what is the idiomatic approach (e.g. it could be an additional parameter to the callback function, or captured by the lambda closure, or something else).

Similar to discussed here we pass a context.Context.

@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 5, 2023

@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 12, 2023

Every normative clause from the Asynchronous Counter section of the v1.16.0 specification has been identified and our API has been validated that it complies. Closing.

@MrAlias MrAlias closed this as completed Jan 12, 2023
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 pkg:API Related to an API package
Projects
No open projects
Status: Done
Development

No branches or pull requests

1 participant