-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Update OM 2.0 with proposal#43 Created Timestamp syntax #2636
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
base: main
Are you sure you want to change the base?
Conversation
ee584fd
to
f24c2ab
Compare
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
77a1036
to
8d8e083
Compare
Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com> Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com> Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with some small corrections. Also as per WG decision, you'll need another approval.
Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com> Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
Thanks for clarifying about exemplars :) |
acme_http_router_request_seconds_sum{path="/api/v1",method="GET"} 9036.32 ct@1605281325.0 | ||
acme_http_router_request_seconds_count{path="/api/v1",method="GET"} 807283.0 ct@1605281325.0 | ||
acme_http_router_request_seconds_sum{path="/api/v2",method="POST"} 479.3 ct@1605281325.0 | ||
acme_http_router_request_seconds_count{path="/api/v2",method="POST"} 34.0 ct@1605281325.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Should we make the created timestamps different in the example? IIRC, the CR is the timestamp of the first observation for the series, so they will almost never match. We don't want to give the wrong impression that they will always be the same.
``` | ||
|
||
Exemplars MAY be attached to the MetricPoint's Total sample. | ||
|
||
An example with a Metric with no labels, and a MetricPoint with a timestamp and a created and an exemplar: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be "Created Value" or "created timestamp"
``` | ||
|
||
An example of a Metric with no labels and a MetricPoint with two quantiles: | ||
An example of a Metric with no labels and a MetricPoint with two quantiles and Created values: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An example of a Metric with no labels and a MetricPoint with two quantiles and Created values: | |
An example of a Metric with no labels and a MetricPoint with two quantiles and Created Values: |
nit: consistently capitalize or lowercase "Created Value"
foo_count 17 | ||
foo_sum 324789.3 | ||
foo_created 1520430000.123 | ||
foo_bucket{le="0.0"} 0 ct@1520430000.123 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about created timestamps all being different. Count and sum should match the lowest CT
Updates OpenMetrics 2.0 with our discussions in the working group and prometheus/proposals#43
cc @Maniktherana