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 Counter #3374

Closed
2 tasks done
Tracked by #3383
MrAlias opened this issue Oct 20, 2022 · 8 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

#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 a Counter other than with a Meter.

Post #3454, our implementation is compliant with this.

@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 5, 2023

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

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

@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 5, 2023

This API SHOULD NOT return a value (it MAY return a dummy value if required by certain programming languages or systems, for example null, undefined).

We comply with this recommendation:

Add(ctx context.Context, incr int64, attrs ...attribute.KeyValue)
Add(ctx context.Context, incr float64, attrs ...attribute.KeyValue)

@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 5, 2023

Required parameters:

  • Optional attributes.
  • The increment amount, which MUST be a non-negative numeric value.

This assumes the term "Required" in the specification means the key word "REQUIRED".

Both parameters are accepted for the Add methods of counters:

Add(ctx context.Context, incr int64, attrs ...attribute.KeyValue)
Add(ctx context.Context, incr float64, attrs ...attribute.KeyValue)

The increment amount, which MUST be a non-negative numeric value.

This is tricky. It could be interpreted as though the API needs to ensure non-negative numeric values are the only values the API accepts. However, as is seen in many other parts of the specification, this may be intended for a user of the API. Meaning, the user is required to not provide non-negative numeric values.

Tracking this question in open-telemetry/opentelemetry-specification#3081

@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 5, 2023

The OpenTelemetry API authors MAY decide to allow flexible attributes to be passed in as arguments. If the attribute names and types are provided during the counter creation, the OpenTelemetry API authors MAY allow attribute values to be passed in using a more efficient way (e.g. strong typed struct allocated on the callstack, tuple). The API MUST allow callers to provide flexible attributes at invocation time rather than having to register all the possible attribute names during the instrument creation.

We do not allow attribute name/type registration at instrument creation time, meaning we do not support this optional feature. We do comply with the requirement to allow users to provide flexible attributes at invocation (of the Add API) time.

@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 5, 2023

@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 30, 2023

@MrAlias MrAlias closed this as completed Jan 30, 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