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.

Sign up for GitHub

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

Jump to bottom

Incorporate histogram min and max fields into histogram language, add… #1983

Merged
merged 15 commits into from
Oct 21, 2021
Merged

Incorporate histogram min and max fields into histogram language, add… #1983

merged 15 commits into from
Oct 21, 2021

Conversation

jack-berg
Copy link
Member

… zero bucket histogram aggregation

Resolves #1902.

Changes

  • Update explicit bucket histogram language to describe capturing min and max
  • Add new Zero Bucket Histogram aggregation, a convenience method for specifying an explicit bucket histogram with zero buckets which produces histogram data points that capture summary data (min, max, sum, count)

@jack-berg jack-berg requested review from a team as code owners October 1, 2021 19:43
@reyang reyang added area:sdk Related to the SDK spec:metrics Related to the specification/metrics directory labels Oct 2, 2021
@reyang reyang added this to the Metrics API/SDK Feature Freeze milestone Oct 2, 2021
reyang
reyang approved these changes Oct 2, 2021
View reviewed changes
Copy link
Member

@reyang reyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM besides the naming, which is subjective though 🤷

bogdandrutu
bogdandrutu previously requested changes Oct 5, 2021
View reviewed changes
specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
jsuereth
jsuereth approved these changes Oct 8, 2021
View reviewed changes
Copy link
Contributor

@jsuereth jsuereth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving the overall call-out to users that a 1-bucket histogram = Min/Max/Sum/Count and is useful.

Naming - I'm sure there will be debates on specific name. I made some comments/possible suggestions but I don't feel strongly about any particular name for this one.

As a side comment, we use this degenerate case of a single-bucket explicit histogram in the Java SDK metric tests.

@bogdandrutu bogdandrutu dismissed their stale review October 12, 2021 15:10

Monotonicity was removed.

@jmacd jmacd enabled auto-merge (squash) October 21, 2021 18:15
@jmacd jmacd merged commit f3fe34c into open-telemetry:main Oct 21, 2021
Spec - Metrics API/SDK Feature Freeze automation moved this from In progress to Done Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hdost hdost hdost left review comments

@jmacd jmacd jmacd approved these changes

@jsuereth jsuereth jsuereth approved these changes

@reyang reyang reyang approved these changes

@bogdandrutu bogdandrutu bogdandrutu left review comments

Assignees

@bogdandrutu bogdandrutu

Labels
area:sdk Related to the SDK spec:metrics Related to the specification/metrics directory
Projects
No open projects
Spec - Metrics API/SDK Feature Freeze
Done
Milestone
Metrics API/SDK Feature Freeze
Development

Successfully merging this pull request may close these issues.

Support min/max from Metrics data model and SDK
6 participants
@jack-berg @jsuereth @hdost @bogdandrutu @jmacd @reyang