Skip to content

Conversation

@sichenzhao
Copy link

What does this PR do?

#14199

Update clickhouse datadog agent client to have ability to specify a cluster name, and query system tables with clusterAllReplicas option.

Motivation

Datadog agent currently is not working with clickhouse clusters running across more than one node. This PR is an attempt to support monitoring multi-node clickhouse deployment by datadog agent.

Additional Notes

I am yet to build/test locally.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached
  • If the PR doesn't need to be tested during QA, please add a qa/skip-qa label.

@sichenzhao sichenzhao requested a review from a team as a code owner May 8, 2023 20:45
Copy link
Contributor

@yzhan289 yzhan289 left a comment

Choose a reason for hiding this comment

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

Thanks for making this PR!

I am yet to build/test locally.

You can build and test your changes using our CLI tool ddev. After installing ddev, you can spin up a test environment for clickhouse using ddev env start clickhouse <environment>. To see the possible ddev test environments we currently support, run ddev env ls clickhouse.

tls_verify: Optional[bool]
use_global_custom_queries: Optional[str]
username: Optional[str]
clustername: Optional[str]
Copy link
Contributor

Choose a reason for hiding this comment

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

This file is autogenerated, so if you want to add clustername as a new configuration option, you can add it here, and then run ddev validate config clickhouse -s and ddev validate models clickhouse -s to regenerate this file and the conf.yaml.example file.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, got it. let me try. Thank you!

Copy link
Author

Choose a reason for hiding this comment

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

I got bash-3.2$ ddev validate config clickhouse -s Validating default configuration files for 1 checks... Unsupported check: clickhouse or manifest_version: None

@sichenzhao sichenzhao requested a review from a team as a code owner July 3, 2023 22:23
@ghost ghost added the documentation label Jul 3, 2023
@sichenzhao
Copy link
Author

You can build and test your changes using our CLI tool ddev. After installing ddev, you can spin up a test environment for clickhouse using ddev env start clickhouse <environment>. To see the possible ddev test environments we currently support, run ddev env ls clickhouse.

Hi there, I tried ddev, but got bash-3.2$ ddev env ls clickhouse No testable checks found for: clickhouse instead.

How may I set up the test environment for ClickHouse first?

@sichenzhao sichenzhao requested a review from yzhan289 July 3, 2023 22:28
@github-actions
Copy link

github-actions bot commented Jul 3, 2023

Test Results

14 tests   3 ✔️  35s ⏱️
  1 suites  3 💤
  1 files    8

For more details on these failures, see this check.

Results for commit 682a59c.

Copy link
Contributor

@kayayarai kayayarai left a comment

Choose a reason for hiding this comment

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

👍 from docs

@alopezz
Copy link
Contributor

alopezz commented Jul 10, 2023

Hi there, I tried ddev, but got bash-3.2$ ddev env ls clickhouse No testable checks found for: clickhouse instead.

The most likely reason for that is that you didn't configure ddev to point at your copy of the repo. You can run ddev config edit to edit the config and make sure that it's pointing to the path where you cloned the integrations-core repo:

[repos]
core = "~/path/to/integrations-core"

@alopezz
Copy link
Contributor

alopezz commented Aug 21, 2023

Hey @sichenzhao, did you get to try my tip above? Let us know if there is anything we can do to help you towards getting this into shape.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants