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

Investigate Refactor of simple Integrator Process function #862

Closed
MrAlias opened this issue Jun 23, 2020 · 1 comment · Fixed by #1024
Closed

Investigate Refactor of simple Integrator Process function #862

MrAlias opened this issue Jun 23, 2020 · 1 comment · Fixed by #1024
Labels
area:metrics Part of OpenTelemetry Metrics pkg:SDK Related to an SDK package prototype Feature to prototype a spec-level decision
Milestone

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Jun 23, 2020

TODO: as seen in lengthy comments below, both the current and delta fields have multiple uses depending on the specific configuration of instrument, exporter, and accumulator. It is possible to simplify this situation by declaring explicit fields that are not used with a dual purpose. Improve this situation?

  1. "delta" is used to combine deltas from multiple accumulators, and it is also used to store the output of subtraction when computing deltas of PrecomputedSum instruments.
  2. "current" either refers to the Aggregator passed to Process() by a single accumulator (when either there is just one Accumulator, or the instrument is Asynchronous), or it refers to "delta", depending on configuration.

Originally posted by @jmacd in #856 (comment)

@MrAlias MrAlias added pkg:SDK Related to an SDK package area:metrics Part of OpenTelemetry Metrics prototype Feature to prototype a spec-level decision labels Jun 23, 2020
@jmacd
Copy link
Contributor

jmacd commented Jul 6, 2020

Note also this related comment:

		// TODO: This code is organized to support multiple
		// accumulators which could theoretically produce the
		// data for the same instrument with the same
		// resources, and this code has logic to combine data
		// properly from multiple accumulators.  However, the
		// use of *metric.Descriptor in the stateKey makes
		// such combination impossible, because each
		// accumulator allocates its own instruments.  This
		// can be fixed by using the instrument name and kind
		// instead of the descriptor pointer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics pkg:SDK Related to an SDK package prototype Feature to prototype a spec-level decision
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants