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

[chore] [reveiver/prometheusreceiver] Add unit tests around protobuf negotiation #29153

Merged

Conversation

krajorama
Copy link
Contributor

@krajorama krajorama commented Nov 13, 2023

Description:

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.

Link to tracking Issue:

#26555

Followup to #27030
Related to #28663

Testing:

Adding simple e2e test for scraping over Protobuf.

Documentation:

Not applicable.

I want to get access to EnableProtobufNegotiation

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
@krajorama krajorama requested a review from a team November 13, 2023 15:05
@krajorama krajorama marked this pull request as draft November 13, 2023 15:06
@github-actions github-actions bot added the receiver/prometheus Prometheus receiver label Nov 13, 2023
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
@krajorama krajorama changed the title [reveiver/prometheusreceiver] Add unit tests around protobuf negotiation [chore] [reveiver/prometheusreceiver] Add unit tests around protobuf negotiation Nov 13, 2023
krajorama and others added 5 commits November 13, 2023 20:54
Move prom metric family to protobuf encoding to its own function.
Reuse assertMetricPresent

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Also assert on explicit bounds.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
krajorama and others added 2 commits November 14, 2023 07:53
Code should be clear enough now.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Copy link
Contributor

@jesusvazquez jesusvazquez left a comment

Choose a reason for hiding this comment

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

This looks good to me 👍

I'm not 100% familiar with how the rest of the test files are structured in this repo but the modifications on testComponent and compareHistogram you added look legit to me.

@dashpole dashpole added the chore label Nov 14, 2023
@dashpole dashpole added the ready to merge Code review completed; ready to merge by maintainers label Nov 15, 2023
@codeboten codeboten merged commit bc25618 into open-telemetry:main Nov 15, 2023
83 checks passed
@github-actions github-actions bot added this to the next release milestone Nov 15, 2023
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore 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.

6 participants