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

[prometheusreceiver] Enable proto negotiation #27030

Conversation

jaronoff97
Copy link
Contributor

Description:

Exposes the protobuf negotiation flag for the scrape manager.

Link to tracking Issue: issue

Testing: tested locally that setting this flag indeed set this flag.

Documentation: Documented the flag in the prom receiver docs

@github-actions github-actions bot added the receiver/prometheus Prometheus receiver label Sep 20, 2023
@jaronoff97 jaronoff97 changed the title Enable proto negotiation [prometheusreceiver] Enable proto negotiation Sep 20, 2023
@dashpole dashpole added the enhancement New feature or request label Sep 20, 2023
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

@dashpole dashpole added the ready to merge Code review completed; ready to merge by maintainers label Sep 20, 2023
@codeboten codeboten merged commit 135e41c into open-telemetry:main Sep 21, 2023
101 of 102 checks passed
@github-actions github-actions bot added this to the next release milestone Sep 21, 2023
jmsnll pushed a commit to jmsnll/opentelemetry-collector-contrib that referenced this pull request Nov 12, 2023
Exposes the protobuf negotiation flag for the scrape manager.
open-telemetry#27027

---------

Co-authored-by: David Ashpole <dashpole@google.com>
codeboten pushed a commit that referenced this pull request Nov 15, 2023
…negotiation (#29153)

The code needs some basic tests that can be later expanded with tests
for native histograms use cases.

Changes:
Refactored `testComponent` function to be easier to customize the
configuration of the scrape.
Expanded `compareHistogram` to assert on the explicit boundaries as
well.
Added function `prometheusMetricFamilyToProtoBuf` to helpers to be able
to turn serialize a Prometheus metric family
into Protobuf.
Added simple test of Protobuf based scrape of counters, gauges,
summaries and histograms.

#26555  

Followup to #27030 
Related to #28663 

**Testing:**

Adding simple e2e test for scraping over Protobuf. 

**Documentation:** 

Not applicable.

---------

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Co-authored-by: David Ashpole <dashpole@google.com>
RoryCrispin pushed a commit to ClickHouse/opentelemetry-collector-contrib that referenced this pull request Nov 24, 2023
…negotiation (open-telemetry#29153)

The code needs some basic tests that can be later expanded with tests
for native histograms use cases.

Changes:
Refactored `testComponent` function to be easier to customize the
configuration of the scrape.
Expanded `compareHistogram` to assert on the explicit boundaries as
well.
Added function `prometheusMetricFamilyToProtoBuf` to helpers to be able
to turn serialize a Prometheus metric family
into Protobuf.
Added simple test of Protobuf based scrape of counters, gauges,
summaries and histograms.

open-telemetry#26555  

Followup to open-telemetry#27030 
Related to open-telemetry#28663 

**Testing:**

Adding simple e2e test for scraping over Protobuf. 

**Documentation:** 

Not applicable.

---------

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Co-authored-by: David Ashpole <dashpole@google.com>
jpkrohling pushed a commit that referenced this pull request Apr 9, 2024
**Description:** 

Implement native histogram append MVP.
Very similar to appending a float sample.

Limitations:
- Only support integer counter histograms fully.
- In case a histogram has both classic and native buckets, we only store
one of them. Governed by scrape_classic_histograms scrape option. The
reason is that in the OTEL model the metric family is identified by the
normalized name (without _count, _sum, _bucket suffixes for the classic
histograms), meaning that the classic and native histograms would map to
the same metric family in OTEL model , but that cannot have both
Histogram and ExponentialHistogram types at the same time.
- Gauge histograms are dropped with warning as that temporality is
unsupported, see
open-telemetry/opentelemetry-specification#2714
- NoRecordedValue attribute might be unreliable. Prometheus scrape marks
all series with float NaN values when stale, but transactions in
prometheusreceiver are stateless, meaning that we have to use heuristics
to figure out if we need to add a NoRecordedValue data point to an
Exponential Histogram metric. (Need work in Prometheus.)



Additionally: 
- Created timestamp supported.
- Float counter histograms not fully tested and lose precision, but we
don't expect instrumentation to expose these anyway.

**Link to tracking Issue:**

Fixes: #26555 

**Testing:** 

Added unit tests and e2e tests.

**Documentation:**

TBD: will have to call out protobuf negotiation while no text format.
#27030

---------

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Co-authored-by: David Ashpole <dashpole@google.com>
@jaronoff97 jaronoff97 deleted the prometheus-receiver-proto-negotiation-option branch June 6, 2024 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready to merge Code review completed; ready to merge by maintainers receiver/prometheus Prometheus receiver
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants