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

Changing OTLP exporters to export cumulative metrics by default #731

Closed
huyan0 opened this issue Jul 23, 2020 · 7 comments · Fixed by #2013
Closed

Changing OTLP exporters to export cumulative metrics by default #731

huyan0 opened this issue Jul 23, 2020 · 7 comments · Fixed by #2013
Assignees
Labels
area:sdk Related to the SDK priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:metrics Related to the specification/metrics directory

Comments

@huyan0
Copy link
Member

huyan0 commented Jul 23, 2020

What are you trying to achieve?
Currently, OTLP exporters in SDKs are pass-through and send delta metrics directly. This makes Prometheus Exporter in the collector exports delta counter values as "gauge". Because of this behavior of the SDK exporter, it is then up to the collector to perform aggregations before exporting to cumulative backends like Prometheus. This feature does not yet exist, but has been proposed in #1422 in the collector repository.

However, in the case of multiple collector instances behind a load balancer, each individual collector possibly cannot aggregate cumulative values correctly, as metric events could be distributed across collectors.

If OTLP exporters export cumulative by default, then it guarantees that events from the same metric or timeseries are aggregated correctly before sending to the collector. The collector could then correctly export the metric to cumulative backends.

Additional context
related issues: Collector #1422, #1255

cc @alolita @huyan0 @jmacd

@huyan0 huyan0 added the spec:metrics Related to the specification/metrics directory label Jul 23, 2020
@bogdandrutu bogdandrutu added area:sdk Related to the SDK priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA labels Jul 24, 2020
@jmacd
Copy link
Contributor

jmacd commented Jul 24, 2020

Following the discussion at yesterday's Metric SIG, I believe our best step forward is:

  1. State the OTLP can be configured for Cumulative, Passthrough, or Delta by default. The machinery for this is already demonstrated by the Metrics Processor component in Support cumulative, delta, and pass-through exporters opentelemetry-go#840. I see this as a requirement that OTLP support both delta and cumulative for all kinds of metric. See Replace Temporality with Kind; Type with ValueType opentelemetry-proto#168 for an OTLP proposal that meets this requirement.
  2. Declare an environment variable or configuration field that determines this setting for OTLP
  3. Set the default OTLP behavior to Cumulative, which gives status-quo behavior for Prometheus users
  4. Add support in the Collector, when configured as an Agent, to support converting Passthrough OTLP to Cumulative OTLP. The assumption that there is a single agent allows this to work.
  5. Collector exporters that wish to expose Deltas are impacted by this, as it's difficult to coordinate correctly in a horizontally scaled pool of collectors. These clients will have to be configured for DELTA OTLP.
  6. Future possibility: the OTLP client would contact the service for a dynamic configuration and use the best option at startup.

@bogdandrutu
Copy link
Member

@jmacd #725 this is related with this. For UpDownSum I haven't seen too many protocols supporting delta

@jmacd
Copy link
Contributor

jmacd commented Aug 6, 2020

@huyan0 will file an OTEP stating this in more detail.

@huyan0
Copy link
Member Author

huyan0 commented Aug 11, 2020

I created a OTEP PR #131 to describe our conclusion from Metrics SIG meetings

@jsuereth jsuereth removed the area:data-model For issues related to data model label Mar 30, 2021
@reyang reyang unassigned alolita and jmacd Mar 30, 2021
@reyang reyang added the area:sdk Related to the SDK label Mar 30, 2021
@reyang
Copy link
Member

reyang commented Mar 30, 2021

Discussed during the 03/30/2021 #128 Metrics Data Model SIG Mtg, moving this out of the Metrics Data Model, tracking this in the Metrics SDK project.

@jmacd
Copy link
Contributor

jmacd commented Oct 1, 2021

This is done, right?

@reyang
Copy link
Member

reyang commented Oct 13, 2021

This is done, right?

Not yet, I believe #2013 is going to nail it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:sdk Related to the SDK priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:metrics Related to the specification/metrics directory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants