-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Added support for endpoints in httpcheckreceiver #37265
Added support for endpoints in httpcheckreceiver #37265
Conversation
…adi/opentelemetry-collector-contrib into update-httpcheckreceiver
…adi/opentelemetry-collector-contrib into update-httpcheckreceiver
Hi @codeboten and @mx-psi , I hope you're doing well. When you have some time, could you kindly review and share your feedback on this PR? Your input would be greatly appreciated. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, some nits.
Comment removed Co-authored-by: Antoine Toulme <[email protected]>
removed new line Co-authored-by: Antoine Toulme <[email protected]>
Next step is a review by the codeowners. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @VenuEmmadi, could the endpoint
be marked as deprecated? seems unnecessary to have both, this could be done in a separate PR, not necessary to do it here
Hi @mx-psi, We now have the codeowner's approval also. Could you please proceed to merge the code? |
@VenuEmmadi Before we merge this, could you either file an issue for this or open a PR for this? |
Hi @codeboten and @mx-psi Thank you for the suggestion regarding marking endpoint as deprecated. However, since endpoint is part of the confighttp configuration, it cannot be deprecated. The httpcheck receiver relies on confighttp, and the endpoint is used indirectly within the configuration structure. Removing or deprecating it would disrupt the functionality and standard configuration pattern provided by confighttp. As such, we should continue supporting endpoint alongside the new endpoints feature for now. |
Captured an issue around adding a validation check to prevent users from setting the |
#### Description This PR enhances the `httpcheckreceiver` by adding support for multiple endpoints (`endpoints`). Users can now specify a list of endpoints in addition to a single `endpoint` for each target. This improves flexibility and reduces redundancy when monitoring multiple similar endpoints. Additional changes include: - Updates to `config.go` to handle `endpoints`. - Updates to `scraper.go` to iterate over and scrape all specified endpoints. - Added unit tests for the new functionality in `config_test.go` and `scraper_test.go`. - Updated documentation (`README.md`) to reflect the changes. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to Tracking Issue Fixes open-telemetry#37121 <!-- Describe what testing was performed and which tests were added. --> #### Testing - All existing and new tests pass. - Tested the `httpcheckreceiver` manually using the following configuration: ```yaml receivers: httpcheck: collection_interval: 30s targets: - method: "GET" endpoints: - "https://opentelemetry.io" - method: "GET" endpoints: - "http://localhost:8080/hello" - "http://localhost:8080/hello" headers: Authorization: "Bearer eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiJqYXZhaW51c2UiLCJleHAiOjE3MzcwMzMzMTcsImlhdCI6MTczNzAxNTMxN30.qNb_hckvlqfWmnnaw2xP9ie2AKGO6ljzGxcMotoFZg3CwcYSTGu7VE6ERsvX_nHlcZOYZHgPc7_9WSBlCZ9M_w" - method: "GET" endpoint: "http://localhost:8080/hello" headers: Authorization: "Bearer eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiJqYXZhaW51c2UiLCJleHAiOjE3MzcwMzMzMTcsImlhdCI6MTczNzAxNTMxN30.qNb_hckvlqfWmnnaw2xP9ie2AKGO6ljzGxcMotoFZg3CwcYSTGu7VE6ERsvX_nHlcZOYZHgPc7_9WSBlCZ9M_w" processors: batch: send_batch_max_size: 1000 send_batch_size: 100 timeout: 10s exporters: debug: verbosity: detailed service: pipelines: metrics: receivers: [httpcheck] processors: [batch] exporters: [debug] ``` #### **Documentation** Describe any documentation changes or additions: ```markdown <!-- Describe the documentation added. --> #### Documentation - Updated the `README.md` to include examples for `endpoints`. - Verified `documentation.md` for metric output consistency. --------- Co-authored-by: Antoine Toulme <[email protected]>
Description
This PR enhances the
httpcheckreceiver
by adding support for multiple endpoints (endpoints
). Users can now specify a list of endpoints in addition to a singleendpoint
for each target. This improves flexibility and reduces redundancy when monitoring multiple similar endpoints.Additional changes include:
config.go
to handleendpoints
.scraper.go
to iterate over and scrape all specified endpoints.config_test.go
andscraper_test.go
.README.md
) to reflect the changes.Link to Tracking Issue
Fixes #37121
Testing
httpcheckreceiver
manually using the following configuration:Documentation
Describe any documentation changes or additions: