Skip to content

Clarify when bucket_counts array may be empty#619

Merged
tigrannajaryan merged 2 commits intoopen-telemetry:mainfrom
tigrannajaryan:tigran/bucketcounts
Feb 12, 2025
Merged

Clarify when bucket_counts array may be empty#619
tigrannajaryan merged 2 commits intoopen-telemetry:mainfrom
tigrannajaryan:tigran/bucketcounts

Conversation

@tigrannajaryan
Copy link
Copy Markdown
Member

@tigrannajaryan tigrannajaryan commented Feb 5, 2025

The current definition says:

"The number of elements in bucket_counts array must be by one
greater than the number of elements in explicit_bounds array."

However, it also says "bucket_counts is an optional field", which
means its length can be 0, which then in turn means the length of
explicit_bounds must be -1, which is impossible.

We clarify that it is a valid situation to have both bucket_counts and explicit_bounds arrays empty.

The current definition says:

"The number of elements in bucket_counts array must be by one
 greater than the number of elements in explicit_bounds array."

However, it also says "bucket_counts is an optional field",
which means its length can be 0, which than in turn means the
length of explicit_bounds must be -1, which is impossible.

We clarify that it is a valid situation to have both bucket_counts
and explicit_bounds arrays empty.
@tigrannajaryan tigrannajaryan requested a review from a team February 5, 2025 18:49
@tigrannajaryan tigrannajaryan requested a review from a team February 10, 2025 17:39
@mx-psi
Copy link
Copy Markdown
Member

mx-psi commented Feb 11, 2025

Does this resolve open-telemetry/opentelemetry-specification/issues/3848? Should it be solved first? I am always unsure about the exact relationship between the proto and the data model

@tigrannajaryan
Copy link
Copy Markdown
Member Author

Does this resolve open-telemetry/opentelemetry-specification/issues/3848? Should it be solved first? I am always unsure about the exact relationship between the proto and the data model

@mx-psi this PR is only about the regular Histogram. open-telemetry/opentelemetry-specification#3848 is about Exponential Histogram.

@mx-psi
Copy link
Copy Markdown
Member

mx-psi commented Feb 11, 2025

Ah, my bad, sorry

Comment thread opentelemetry/proto/metrics/v1/metrics.proto
@pellared
Copy link
Copy Markdown
Member

CC @open-telemetry/specs-metrics-approvers

@tigrannajaryan
Copy link
Copy Markdown
Member Author

This was open for several days and appears to match the data model spec. Merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants