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

Add array aggregation for raw measure metrics, improve testing #282

Merged
merged 15 commits into from
Nov 4, 2019

Conversation

jmacd
Copy link
Contributor

@jmacd jmacd commented Nov 3, 2019

This adds another aggregation type for exporters that wish to see a raw list of values, as opposed to the maxsumcount and ddsketch aggregators, which cannot provide the raw values.

Improves the testing framework, adds more DDSketch testing, and adds error values for the Quantile/Min/Max interfaces.

Copy link
Contributor

@rghetia rghetia left a comment

Choose a reason for hiding this comment

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

one minor question on median formula change.

}
panic("unknown number kind")
func (n *Numbers) Median() core.Number {
return n.numbers[len(n.numbers)/2]
Copy link
Contributor

Choose a reason for hiding this comment

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

why is media formula changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm using Median() here to mean Quantile(0.5), and there are too many definitions for Quantile to list. (I found http://mathworld.wolfram.com/Quantile.html informative)

After some research in the metrics space, I concluded that the most frequent definition for a quantile returns the smallest value from the original data set which is >= the given quantile. This means we always return a value in the set, not an interpolated value.

Interpolation has risks -- it makes sense when you have a data model, but we don't in this case. If we know the data is uniform or gaussian, then interpolation could make sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added "// Median() is an alias for Quantile(0.5)." and

	// Note that len(n.numbers) is 1 greater than the max element
	// index, so dividing by two rounds up.  This gives the
	// intended definition for Quantile() in tests, which is to
	// return the smallest element that is at or above the
	// specified quantile.

@jmacd jmacd merged commit 9040d82 into open-telemetry:master Nov 4, 2019
@jmacd jmacd deleted the jmacd/array_dist branch November 4, 2019 22:24
hstan referenced this pull request in hstan/opentelemetry-go Oct 15, 2020
* Fix non-portable sed invocation in pre_release.sh

* Fixes due to API changes

* Update CHANGELOG.md
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.

2 participants