signaltometricsconnector: Add sum.monotonic property for improved counter handling#45865
Conversation
bencehornak
force-pushed
the
signaltometrics-sum-monotonic
branch
from
cb5c5da to
14c6393
Compare
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Waiting for the review of @ChrsMark and @lahsivjar |
bencehornak
force-pushed
the
signaltometrics-sum-monotonic
branch
from
14c6393 to
e8ac38e
Compare
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
Signed-off-by: Bence Hornák <bence.hornak@gmail.com>
bencehornak
force-pushed
the
signaltometrics-sum-monotonic
branch
from
e8ac38e to
5bda2a7
Compare
lahsivjar
left a comment
lahsivjar
left a comment
There was a problem hiding this comment.
Apologies about the delay in the review. Mostly LGTM, just a comment on one of the test value change.
| startTimeUnixNano: "1581452772000000321" | ||
| timeUnixNano: "1581452773000000789" | ||
| - asInt: "456" | ||
| - asInt: "-456" |
There was a problem hiding this comment.
Shouldn't this break some other tests too? What's the rationale for this change of test value?
There was a problem hiding this comment.
Ah, I see what is happening now, this specific test data was only used for counting. LGTM!
There was a problem hiding this comment.
My intention was to make this test a generic 'sum' test case without specifying is_monotonic explicitly.
So I wanted to make sure that we are not testing only a 'special' case, in which all increments are positive, and the metric can go only up.
| startTimeUnixNano: "1581452772000000321" | ||
| timeUnixNano: "1581452773000000789" | ||
| - asInt: "456" | ||
| - asInt: "-456" |
There was a problem hiding this comment.
Ah, I see what is happening now, this specific test data was only used for counting. LGTM!
|
@mx-psi I think this PR is ready to be merged, or is there anything else we are waiting on? |
|
Thank you for your contribution @bencehornak! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey. If you are getting started contributing, you can also join the CNCF Slack channel #opentelemetry-new-contributors to ask for guidance and get help. |
4 participants
Description
So far there has been no way to set the
IsMonotonicproperty of metrics, although the main use-case based on the examples of the README is to generate counters, which are clearly monotonic.The incorrect monotonic=false setting causes some problems on my end, namely that the OTel -> Prometheus conversion is incorrect (
_totalisn't appended, if monotonic is false), additionally, theintervalprocessor doesn't work with non-monotonic sums.