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

Support ruok command in zookeeper receiver #21481

Closed
akats7 opened this issue May 3, 2023 · 6 comments · Fixed by #22726
Closed

Support ruok command in zookeeper receiver #21481

akats7 opened this issue May 3, 2023 · 6 comments · Fixed by #22726
Labels
enhancement New feature or request receiver/zookeeper

Comments

@akats7
Copy link
Contributor

akats7 commented May 3, 2023

Component(s)

receiver/zookeeper

Is your feature request related to a problem? Please describe.

Other zookeeper agents (e.g., datadog) support the ruok command along with the metric set from mntr, I wanted to inquire regarding adding a scraper to get the ruok response, and potentially adding it as an attribute as it likely doesn't belong in a metric table as a standalone.

Describe the solution you'd like

We would like the ruok response to be scraped along with the mntr metric set.

Describe alternatives you've considered

No response

Additional context

No response

@akats7 akats7 added enhancement New feature or request needs triage New item requiring triage labels May 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 3, 2023

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@atoulme atoulme removed the needs triage New item requiring triage label May 6, 2023
@akats7
Copy link
Contributor Author

akats7 commented May 8, 2023

Hi @djaglowski, wanted to follow up

@djaglowski
Copy link
Member

Hi @akats7. Thanks for the proposal.

Am I correct in understanding that this command is essentially a boolean health check? If so, my suggestion would be to model this as an uptime metric. Here are several examples. What do you think about this approach?

Otherwise, can you clarify where you would add this as an attribute?

@akats7
Copy link
Contributor Author

akats7 commented May 8, 2023

Hey @djaglowski, thanks for the quick response.

Yep it is a boolean health check, after looking at the example set you sent, I stumbled upon a mongodb health metric (here) that seems to be a analogous to what we're trying to see with zookeeper. It's of kind gauge with a numeric 1 or 0 to represent health. I think this would be a good fit.

@djaglowski
Copy link
Member

@akats7, that makes sense. Sounds good to me.

@akats7
Copy link
Contributor Author

akats7 commented May 24, 2023

Hey @djaglowski, the zookeeper server closes the socket after running each command, so to run the ruok command in the same scraper, we have to use multiple connections. I wanted to run it by you before I update the existing tests.

I'll open and link a preliminary PR.

djaglowski pushed a commit that referenced this issue Jun 21, 2023
**Description:** <Describe what has changed.>
Add zookeeper ruok health check metric based on response from ruok 4lw
command
**NOTE**: preliminary PR not ready to be merged

**Link to tracking Issue:** resolves #21481 

**Testing:**: Modified existing unit tests to account for additional
metric from additional command. Made minor change to test structure to
account for separate text responses from separate commands. Added unit
test to handle following cases, valid "imok" return from server, invalid
response from server, and empty response from server.
fchikwekwe pushed a commit to fchikwekwe/opentelemetry-collector-contrib that referenced this issue Jun 23, 2023
**Description:** <Describe what has changed.>
Add zookeeper ruok health check metric based on response from ruok 4lw
command
**NOTE**: preliminary PR not ready to be merged

**Link to tracking Issue:** resolves open-telemetry#21481 

**Testing:**: Modified existing unit tests to account for additional
metric from additional command. Made minor change to test structure to
account for separate text responses from separate commands. Added unit
test to handle following cases, valid "imok" return from server, invalid
response from server, and empty response from server.
Caleb-Hurshman pushed a commit to observIQ/opentelemetry-collector-contrib that referenced this issue Jul 6, 2023
**Description:** <Describe what has changed.>
Add zookeeper ruok health check metric based on response from ruok 4lw
command
**NOTE**: preliminary PR not ready to be merged

**Link to tracking Issue:** resolves open-telemetry#21481 

**Testing:**: Modified existing unit tests to account for additional
metric from additional command. Made minor change to test structure to
account for separate text responses from separate commands. Added unit
test to handle following cases, valid "imok" return from server, invalid
response from server, and empty response from server.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request receiver/zookeeper
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants