-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Metric SDK: Do not export non-observed attribute sets for async instruments #4290
Conversation
5fdb1d3
to
271d230
Compare
Additionally, for delta async instruments, omitting an attribute set from a callback in one call makes the next call assume that it is starting over from zero, rather than the previously recorded value |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4290 +/- ##
=======================================
- Coverage 83.5% 83.5% -0.1%
=======================================
Files 183 183
Lines 14218 14217 -1
=======================================
- Hits 11886 11881 -5
- Misses 2105 2109 +4
Partials 227 227
|
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.
Looks good overall, just small questions.
Where do we plan to document the introduced behavior here and our recommendation to use these types of instruments for this feature? opentelemetry.io?
31931c9
to
d720051
Compare
d720051
to
380d875
Compare
I'm happy to document it here for now. If we can make sure this behavior is consistent across languages, then I think opentelemetry.io would be great. |
Would the SDK instrumentation constructors be an appropriate place to document the behavior? e.g. https://github.com/open-telemetry/opentelemetry-go/blob/main/sdk/metric/meter.go#L110 |
I think there and the |
Done |
2a01ee5
to
235bcda
Compare
235bcda
to
62b6e0e
Compare
Fixes #3605
It addresses this issue by calling delete(s.values, attr) in precomputed aggregators for the value of an attribute set when it is sent. This ensures a given observation is only exported if it is recorded in that collection cycle.
For a simple example of the impact, see the diff in sdk/metric/meter_test.go.