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

Modify health check extension to add new metrics health check functions #5643

Merged
merged 1 commit into from
Nov 1, 2021

Conversation

skyduo
Copy link
Contributor

@skyduo skyduo commented Oct 5, 2021

Description:
This is a new health check method which allow customers to monitor the health status of containers based on the exporter functionally failures. It will return a 200/500 status when call the endpoint based on if the components can send data to the destination properly. Currently it only supports monitoring the exporter health status, will also support receivers and processors in the future.

Link to tracking Issue:
open-telemetry/opentelemetry-collector#2573

Testing:
make otelcontribcol and run executable file

Documentation:
https://docs.google.com/document/d/1SpUMsWA2DeaoVazeQ8uEc1Wvu5LphmQU_TjzLmuJ4QM/edit#heading=h.rs1luwizct2w

original pr was creating a new health check extension, but some community members think we should add the new feature in the existing one. Had discussion with @bogdandrutu and @Aneurysm9 and create this new pr.
previous pr:
#5144

@skyduo skyduo requested review from a team and jpkrohling October 5, 2021 00:13
extension/healthcheckextension/README.md Outdated Show resolved Hide resolved
extension/healthcheckextension/README.md Outdated Show resolved Hide resolved
extension/healthcheckextension/config.go Outdated Show resolved Hide resolved
extension/healthcheckextension/healthcheckextension.go Outdated Show resolved Hide resolved
@jpkrohling
Copy link
Member

Tests started.

@Aneurysm9
Copy link
Member

@skyduo please address this lint failure (and any others reported by running make golint locally):

healthcheckextension.go:75:7: shadow: declaration of "err" shadows declaration at line 48 (govet)
			if err := hc.server.Serve(ln); err != http.ErrServerClosed && err != nil {
			   ^

@skyduo
Copy link
Contributor Author

skyduo commented Oct 8, 2021

not sure why make golint not work on my workspace even after i make install-tools and download the golint, but just resolve this issue it indicate

@skyduo
Copy link
Contributor Author

skyduo commented Oct 13, 2021

hi @bogdandrutu ! could you help check and approve this pr if possible? Anthony and Juraci has approved, just wait for your final approval.

@jpkrohling jpkrohling added the ready to merge Code review completed; ready to merge by maintainers label Oct 18, 2021
@skyduo
Copy link
Contributor Author

skyduo commented Oct 20, 2021

Hey @bogdandrutu, since this pr is ready to merge, could you help approve it? Thanks!

@alolita
Copy link
Member

alolita commented Oct 25, 2021

Hi @bogdandrutu gentle reminder for a final review and merge from you for this PR. We would like to be able to make this feature available for OTEL users in the next release. Ty in advance!

@skyduo
Copy link
Contributor Author

skyduo commented Oct 28, 2021

hi @bogdandrutu! i just resolve all the issues and merge it to a single commit, could you help approve it now?

@jpkrohling
Copy link
Member

Could you please fix the merge conflict?

@skyduo
Copy link
Contributor Author

skyduo commented Oct 29, 2021

hi @bogdandrutu! i just rebase and resolve the conflict, all tests pass now, could you help approve and merge it today? Thanks!

@jpkrohling jpkrohling merged commit 12b27b7 into open-telemetry:main Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants