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

Add Prometheus Receiver tests for honor_labels configuration #6369

Merged

Conversation

mustafain117
Copy link
Contributor

@mustafain117 mustafain117 commented Nov 18, 2021

Description:

This PR depends on #6150, the latest commit in this PR is relevant to the honor_labels configuration test
Adds TestHonorLabelsFalseConfig to test honor_labels : false configuration
Adds TestHonorLabelsTrueConfig to test honor_labels : true configuration

Details:
Both tests use the following test data:

# HELP test_gauge0 This is my gauge
# TYPE test_gauge0 gauge
test_gauge0{instance="hostname:8080",job="honor_labels_test",testLabel="value1"} 1

TestHonorLabelsFalseConfig:
Sets honor_labels : false for the scrape configuration
Validates that the label conflicts are resolved by prefixing job and instance labels with "exported_" for the test_gauge0 metric

TestHonorLabelsTrueConfig:
Sets honor_labels : true for the scrape configuration
Validates that the label conflicts are resolved by keeping job and instance label values from the scraped data

Link to tracking Issue:
Closes #5995

@mustafain117 mustafain117 requested review from Aneurysm9 and a team as code owners November 18, 2021 04:03
@mustafain117 mustafain117 marked this pull request as draft November 18, 2021 04:07
@mustafain117 mustafain117 marked this pull request as ready for review November 18, 2021 17:53
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.

Please rebase @mustafain117

@mustafain117
Copy link
Contributor Author

@codeboten Done!

@mustafain117
Copy link
Contributor Author

@dashpole @vishiy can you please provide a review? Thank you.

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.

Tests look good to me, pinging @Aneurysm9 as a codeowner for a review.

func verifyHonorLabelsTrue(t *testing.T, td *testData, rms []*pdata.ResourceMetrics) {
//Test for honor_labels: true is skipped. Currently, the Prometheus receiver is unable to support this config
//See: https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/5757
t.Skip("skipping test for honor_labels true configuration")
Copy link
Contributor

Choose a reason for hiding this comment

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

Skipped tests tend to drift in my experience. I'd prefer to omit the tests, and leave a TODO with a link to the issue to add them.

Copy link
Member

Choose a reason for hiding this comment

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

Since we've got an implementation that should work if the appender were able to handle this case I'd like to get this into the history so we can easily resurrect it. Because we squash PRs to a single commit that means that we'd need to land this and then remove it in a subsequent PR. I'm fine with leaving it in with a skip but we can follow up to remove this if you think it needs to go.

@codeboten codeboten merged commit 90c223e into open-telemetry:main Nov 29, 2021
jamesmoessis pushed a commit to atlassian-forks/opentelemetry-collector-contrib that referenced this pull request Dec 8, 2021
…lemetry#6369)

* refactor existing external labels test

* Add test for label_limit configuration

* shutdown mock prometheus http test server within testHelper

* fix spelling

* fix lint error

* rename testHelper to testMetricsReceiver

* fix: lint issue in metrics_receiver_helper_test.go

* Add Prometheus receiver test for honor_labels configuration

* update TestHonorLabelsFalseConfig to use testComponent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add test for honor_labels configuration for Prometheus Receiver
6 participants