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

[receiver/httpcheck] add metrics check whether substring contains in response body #27015

Closed
wants to merge 5 commits into from

Conversation

sakulali
Copy link
Contributor

@sakulali sakulali commented Sep 20, 2023

Description:
Part of #24913, add metrics check if substring contains in http check response body or not. When set body option and emit httpcheck.body, testing substring contains or not in the response of HTTP Check.

Link to tracking Issue:
#24913

Testing:
make generate
make chlog-validate
go test for httpcheckreceiver

Documentation:
receiver/httpcheckreceiver/README.md

@sakulali sakulali requested a review from codeboten as a code owner September 20, 2023 06:42
@sakulali sakulali requested a review from a team September 20, 2023 06:42
@github-actions github-actions bot added the receiver/httpcheck HTTP Check receiver label Sep 20, 2023
@sakulali
Copy link
Contributor Author

Thanks for careful reviews. I really appreciate it @atoulme!

@sakulali sakulali changed the title [receiver/httpcheck] add metrics check if content contains or not [receiver/httpcheck] add metrics check whether substring contains in resopnse body of http check Sep 28, 2023
@sakulali sakulali changed the title [receiver/httpcheck] add metrics check whether substring contains in resopnse body of http check [receiver/httpcheck] add metrics check whether substring contains in resopnse body Sep 28, 2023
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Oct 13, 2023
@sakulali
Copy link
Contributor Author

Could this PR hold on?

@github-actions github-actions bot removed the Stale label Oct 14, 2023
@lpegoraro
Copy link

Hi @sakulali, if you need help with this, let me know, I can work with you, since this is aligned with what I need for the receiver.

@sakulali
Copy link
Contributor Author

Hi @sakulali, if you need help with this, let me know, I can work with you, since this is aligned with what I need for the receiver.

Sorry for delay reply @lpegoraro. Could you please help review the code? I'm unsure if my implementation aligns with your issue, and I would also like to collaborate in completing this valuable feature you've proposed.

@sakulali
Copy link
Contributor Author

Hello @atoulme, cloud you mind help reviews again? I want to work with @lpegoraro to implement the feature, thanks!

@atoulme atoulme changed the title [receiver/httpcheck] add metrics check whether substring contains in resopnse body [receiver/httpcheck] add metrics check whether substring contains in response body Oct 24, 2023
@atoulme
Copy link
Contributor

atoulme commented Oct 24, 2023

Some English and a couple touches, but it's not far.

@sakulali
Copy link
Contributor Author

Some English and a couple touches, but it's not far.

Thanks very much for taking the time to reviews @atoulme!

@lpegoraro
Copy link

I also think we are very close!
I only have a suggestion about the encoding of the string of the check, if we either get the encoding from the configuration of the target, or do we fetch it from the Header, which I believe is safer, what do you guys think?

@sakulali
Copy link
Contributor Author

if we either get the encoding from the configuration of the target, or do we fetch it from the Header

Great suggestion! I think it's a good idea to create a separate issue to follow up. I'm not very familiar with encoding best practices either, i think we can retrieve the encoding format from a custom field in the HTTP header, such as X-Encoding: my-custom-encoding, or through URL parameters. We can then use the configuration file to select the appropriate decoding method for processing?

@github-actions github-actions bot added the Stale label Nov 14, 2023
@lpegoraro
Copy link

hi there folks, what is the hold up? can we do something about it?

@sakulali sakulali requested a review from codeboten November 16, 2023 16:44
@sakulali
Copy link
Contributor Author

sakulali commented Nov 16, 2023

hi there folks, what is the hold up? can we do something about it?

Thanks for reminder. Could you mind help to reviews again? @codeboten

@github-actions github-actions bot removed the Stale label Nov 17, 2023
@sakulali sakulali force-pushed the httpcheck-test-content branch from 16a6dfc to 5993665 Compare November 21, 2023 04:18
@lpegoraro
Copy link

Hi there! Great work on getting 2 approvals, can we merge this for the next release?

@atoulme atoulme added the ready to merge Code review completed; ready to merge by maintainers label Nov 30, 2023
@lpegoraro
Copy link

Hi there fellas, @codeboten @atoulme , would this be in the next release?

@codeboten
Copy link
Contributor

Sorry for the delay @lpegoraro, I would like to resolve #27015 (comment) before merging this

@codeboten codeboten removed the ready to merge Code review completed; ready to merge by maintainers label Dec 8, 2023
@lpegoraro
Copy link

Count me in for anything @codeboten

@sakulali sakulali force-pushed the httpcheck-test-content branch from 2653032 to 6a7a5bb Compare December 21, 2023 13:55
Signed-off-by: sakulali <sakulali@126.com>
Copy link
Contributor

github-actions bot commented Jan 5, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jan 5, 2024
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Jan 19, 2024
@@ -37,6 +37,8 @@ Each target has the following properties:

- `endpoint` (required): the URL to be monitored
- `method` (optional, default: `GET`): The HTTP method used to call the endpoint
- `body` (optional, default: ""): If set, the receiver will emit metrics based on whether the response body exact matches this string.

Choose a reason for hiding this comment

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

This could carry a different meaning when method is POST where the body content will be sent as part of the request.

@AwsmAshraf
Copy link

Any one working on this? I actually have a requirement for this. Can we do it in a way in which we can specify the status code to be expected? I actually agree with @andrzej-stencel on this. A single data point would be great, with any additional info in the form of attributes. Also, modifying the name of the metric to something other than httpcheck.status would make identifying the result of multiple http checks a lot easier. What I'm trying to say is different http endpoint checks would be populated as different metrics with the name of the metric specified for that endpoint when configuring this receiver.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
receiver/httpcheck HTTP Check receiver Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants