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.

Sign up for GitHub

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

Jump to bottom

Prometheus export from OTLP receiver not functional #1255

Closed
jmacd opened this issue Jul 2, 2020 · 12 comments
Closed

Prometheus export from OTLP receiver not functional #1255

jmacd opened this issue Jul 2, 2020 · 12 comments
Assignees
@alolita
Labels
bug Something isn't working help wanted Good issue for contributors to OpenTelemetry Service to pick up priority:p2 Medium spec:metrics
Milestone
Backlog

Comments

@jmacd
Copy link
Contributor

jmacd commented Jul 2, 2020

Describe the bug
Discussed under open-telemetry/opentelemetry-go#875, we have evidence that OTLP metrics data are being transformed incorrectly, such that the Prometheus exporter will:

  • report Counter data as "gauge"
  • drop Summary data (which the OTel-Go SDK is exporting for ValueRecorder, ValueObsever)

Steps to reproduce
The steps are described in open-telemetry/opentelemetry-go#875.

What did you expect to see?
Counter data should report as counter, ValueRecorder data should report as gauge.

What did you see instead?
Counters are labeled gauges, and no gauges appear.

What version did you use?
0.4

What config did you use?
This is also linked in open-telemetry/opentelemetry-go#875

Environment
Dockerized alpine

Additional context
I will self-assign this. I am working on it, just want it recorder here that Prometheus exporter is not functional with OTLP data due to a translation problem that I am investigating.
@jmacd jmacd added the bug Something isn't working label Jul 2, 2020
@jmacd jmacd changed the title Prometheus export from OTLP receiver Prometheus export from OTLP receiver not functional Jul 2, 2020
This was referenced Jul 2, 2020
Closed
Closed
@huyan0
Copy link
Member

huyan0 commented Jul 4, 2020 •
edited

I noticed the underlying prometheus-go-metric-exporter that the Collector's Prometheus exporter uses drop Summary when converting to Prometheus type. For some reason I thought it was a feature. Also, it doesn't care about the semantic difference and reports instantaneous adding scalar as a gauge.

Seems like the original prometheus-go-metric-exporter is moved to a different repo and support for Summary type is in progress .

@jmacd
Copy link
Contributor Author

jmacd commented Jul 5, 2020

The big issue with prometheus-go-metric-exporter is that it uses the OpenCensus format, and we're migrating to OTLP. I am not certain what the best option is. A bigger question is which library will support Prometheus push: see #1150.

@jmacd jmacd mentioned this issue Jul 8, 2020
Closed
@huyan0 huyan0 mentioned this issue Jul 9, 2020
Closed
6 tasks
@huyan0 huyan0 mentioned this issue Jul 23, 2020
Closed
@bogdandrutu bogdandrutu added this to the GA 1.0 milestone Jul 30, 2020
@jmacd
Copy link
Contributor Author

jmacd commented Jul 30, 2020

I don't think we can sort this out until OTLP is settled, but it's fine to assign this to me.

@dabangarang dabangarang mentioned this issue Aug 11, 2020
Closed
This was referenced Aug 11, 2020
Closed
Merged
This was referenced Aug 25, 2020
Closed
Merged
Closed
@tigrannajaryan tigrannajaryan added the help wanted Good issue for contributors to OpenTelemetry Service to pick up label Sep 2, 2020
@tigrannajaryan
Copy link
Member

This does not appear to be a dealbreaker for 1.0, I am removing from the milestone, feel free to object.

@tigrannajaryan tigrannajaryan modified the milestones: GA 1.0, Backlog Oct 19, 2020
@alolita
Copy link
Member

alolita commented Oct 22, 2020

Hi @tigrannajaryan Support of the summary metric type in OTLP is essential for supporting Prometheus metric types. The Prometheus pipeline is not complete without this support. Currently the default OTLP behavior is to just drop the metric if it is a summary type and the information gets lost at this point. See https://github.com/open-telemetry/opentelemetry-collector/blob/master/translator/internaldata/oc_to_metrics.go#L227-L228 Also please see https://prometheus.io/docs/concepts/metric_types/#summary
Since summary is not supported in OTLP, any summary type metric gets dropped - see code example below
https://github.com/open-telemetry/opentelemetry-collector/blob/master/exporter/prometheusremotewriteexporter/exporter.go#L126

This is a blocker for the Prometheus remote write exporter and the Prometheus exporter. I would consider it a show-stopper for OTLP.

@jmacd
Copy link
Contributor Author

jmacd commented Oct 22, 2020

I think we can solve this. We do not have a single data point type that represents summaries, but we can generate several gauges and counters to represent a summary so that the Prometheus remote-write output data will be correct. Prometheus uses a metric name suffix ("_count" an integer, "_sum" a floating point) for two counters, and it uses a gauge for point each quantile (e.g., "quantile=0.5", "quantile=0.9"). This representation is not efficient compared with a compact Summary data point, but no matter how we represent this data in OTLP, two counters and one gauge per quantile are what a Prometheus user expects to receive.

@mtwo
Copy link
Member

mtwo commented Oct 22, 2020 •
edited

Following up from the specs SIG meeting. @alolita from AWS will create a proposal for this.

@jmacd can you re-assign to @alolita?

@tigrannajaryan tigrannajaryan assigned alolita and unassigned jmacd Oct 26, 2020
@tigrannajaryan
Copy link
Member

@alolita as per @mtwo 's comment I assigned this to you.

@alolita alolita mentioned this issue Oct 26, 2020
Closed
@alolita
Copy link
Member

alolita commented Oct 27, 2020

@tigrannajaryan Will follow up with an implementation proposal shortly.

@amanbrar1999
Copy link
Contributor

@jmacd Resolving the summary part of this issue is currently happening, but does the issue of counter data being reported as gauge still exist?

@amanbrar1999
Copy link
Contributor

My understanding of this is that counter data being reported as gauge is a direct consequence of prometheus exporter exporting delta metrics, when it currently only functions correctly when exporting cumulative metrics. This specific part of this issue should be partially resolved when this spec issue is resolved: open-telemetry/opentelemetry-specification#731

It will only be completely resolved when there is functionality to convert delta metrics to cumulative metrics: https://github.com/open-telemetry/opentelemetry-collector/issues/1422

@andrewhsu
Copy link
Member

from the metrics sig mtg today, discussed this issue and decided to close

@huyan0 huyan0 mentioned this issue Aug 30, 2021
Open
MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this issue Nov 11, 2021
@XSAM @MrAlias
Add SpanContextFromContext() (open-telemetry#1255)
230bdd1 
* SpanFromContext returns nil if span not exists

* Add tests for SpanContextFromContext

* Update CHANGELOG

* Update trace.go

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

* SpanFromContext() continue to return a noopSpan{}

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this issue Apr 27, 2023
@pmcollins
zc: read jar file path and service name from a config file (open-tele…
911d125 
…metry#1255)

* Read jar file path and service name from a config file
* zc: rename config example file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alolita alolita

Labels
bug Something isn't working help wanted Good issue for contributors to OpenTelemetry Service to pick up priority:p2 Medium spec:metrics
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
8 participants
@andrewhsu @mtwo @bogdandrutu @alolita @jmacd @tigrannajaryan @huyan0 @amanbrar1999