feat(providers): add GreptimeDB metric provider#10
feat(providers): add GreptimeDB metric provider#10shilicqupt wants to merge 4 commits intoalibaba:mainfrom
Conversation
yaohe-wzh
left a comment
There was a problem hiding this comment.
Thanks for adding the GreptimeDB provider. I tested this PR locally against GreptimeDB v1.0.1 and reviewed the provider wiring. The overall direction looks good — reusing the shared PromQL provider with the /v1/prometheus prefix is the right approach — but I think there are a few issues to fix before merge.
- Please do not declare
metric infoas supported for GreptimeDB.
GreptimeDB v1.0.1 does not expose the Prometheus metadata endpoint used by obz metric info:
GET /v1/prometheus/api/v1/metadata
Local verification:
/v1/health -> 200
/v1/prometheus/api/v1/query -> 200
/v1/prometheus/api/v1/labels -> 200
/v1/prometheus/api/v1/series -> 200
/v1/prometheus/api/v1/label/{name}/values -> 200
/v1/prometheus/api/v1/metadata -> 404
GreptimeDB also has an upstream issue for this exact missing API: GreptimeTeam/greptimedb#4824 (/v1/prometheus/api/v1/metadata not supported).
So SupportedCommands should be changed to:
metric_query: true,
metric_list: true,
metric_info: false,
metric_labels: true,
metric_label_values: true,
metric_series: true,Please also update README.md, README.zh-CN.md, and skills/obz-greptimedb/SKILL.md so they no longer say that all six metric commands work or that GreptimeDB supports metric info.
- The new
obz-greptimedbskill is not registered in the embedded skill registry.
This PR adds:
skills/obz-greptimedb/SKILL.md
but it is not added to crates/obz/src/skills.rs::SKILLS. As a result, the built binary cannot show the skill:
$ ./target/debug/obz skills show obz-greptimedb
unknown skill 'obz-greptimedb' — run `obz skills list` to see available skills
Please add the corresponding SkillEntry and update the skill registry tests so obz skills list/show/install can see the GreptimeDB skill.
- Please fix the
provider checkexample in the skill document.
The current skill doc says:
obz provider check -p greptimedb --endpoint http://localhost:4000However, provider check does not accept -p or --endpoint; it checks a configured provider name. The example should instead configure a provider first, then run something like:
obz provider check greptimedbI also ran the usual Rust checks on this branch:
cargo fmt --check passed
cargo clippy --workspace --all-targets -- -D warnings passed
cargo test passed
cargo build passed
cargo deny check licenses bans sources passed, with existing unmatched-license warnings only
Once metric_info is disabled for GreptimeDB and the skill registry/docs are fixed, this provider should be in much better shape for merge.
yuanmao16
left a comment
There was a problem hiding this comment.
Overall this PR follows the established provider pattern well — reusing PromqlMetricProvider with the /v1/prometheus path prefix is the right approach. A few issues around documentation accuracy and a minor lint concern below.
| providers: | ||
| greptime-prod: | ||
| endpoint: http://localhost:4000 | ||
| auth: | ||
| username: ${env:GREPTIME_USER} | ||
| password: ${env:GREPTIME_PASS} |
There was a problem hiding this comment.
The config key greptime-prod differs from the provider's internal name greptimedb, so a provider: greptimedb field is needed here — otherwise obz won't know which provider this config entry maps to.
Same issue on line 81 for the greptime-local example.
Compare with README.md examples for sls, dd, es-prod which all include the provider: field.
| | Signal | Metric | | ||
| | Query language | PromQL | | ||
| | Default port | 4000 (HTTP) | | ||
| | Auth | Basic auth or no auth | |
There was a problem hiding this comment.
The Quick Reference says Auth is Basic auth or no auth, but the code in mod.rs line 63 calls config.bearer_token(), meaning bearer token auth is also supported. This should read something like Bearer token, Basic auth, or no auth to reflect the actual capability.
| //! payloads to standard Prometheus, so we reuse the shared conversion | ||
| //! logic from the `promql` module. | ||
|
|
||
| #[allow(unused_imports)] |
There was a problem hiding this comment.
#[allow(unused_imports)] is used here without a justification comment. Per project guidelines (AGENTS.md), suppressing warnings requires justification. Since these re-export files exist for structural consistency with other PromQL-based providers (Mimir, Prometheus), consider adding a brief comment above the attribute, e.g.:
// Structural consistency with other PromQL-based providers;
// imports are used by tests and future provider-specific overrides.Same applies to response.rs.
| //! `GreptimeDB` routes queries to the `public` database by default. | ||
| //! To target a different database, append `?db=<name>` directly in | ||
| //! your queries or set a custom `db` query parameter via the `db` | ||
| //! config key (passed as an extra header workaround is not needed — | ||
| //! the default works for most deployments). |
There was a problem hiding this comment.
This paragraph is a bit hard to follow — "passed as an extra header workaround is not needed" reads as a parenthetical that references a workaround the reader has no context for. Consider simplifying to:
To target a different database, pass the
dbquery parameter in your PromQL request. The default database ispublic, which suits most deployments.
| ## Authentication | ||
|
|
||
| GreptimeDB supports basic auth (username + password). Configure credentials | ||
| in `~/.config/obz/config.yaml`: | ||
|
|
||
| ```yaml | ||
| providers: | ||
| greptime-prod: | ||
| endpoint: http://localhost:4000 | ||
| auth: | ||
| username: ${env:GREPTIME_USER} | ||
| password: ${env:GREPTIME_PASS} | ||
| ``` |
There was a problem hiding this comment.
Since the code supports bearer token auth (via config.bearer_token()), it would be useful to add a token-based config example here alongside the basic auth and no-auth examples, e.g.:
providers:
greptime-cloud:
provider: greptimedb
endpoint: https://greptime.example.com
auth:
token: ${env:GREPTIME_TOKEN}| [](https://github.com/alibaba/obz-cli/actions/workflows/ci.yml) | ||
| [](https://github.com/alibaba/obz-cli/releases) | ||
| [](https://crates.io/crates/obz) | ||
| [](https://github.com/alibaba/obz-cli/releases) | ||
| [](LICENSE) | ||
| [](Cargo.toml) |
There was a problem hiding this comment.
These badge additions are unrelated to the GreptimeDB provider feature. Mixing unrelated changes into a feature PR makes it harder to review and revert independently. Consider splitting them into a separate commit or PR.
GreptimeDB exposes a Prometheus-compatible HTTP API under /v1/prometheus, so we reuse the shared PromqlMetricProvider with that path prefix. All six metric commands (query, list, info, labels, label-values, series) work out of the box. Provider aliases: greptimedb, greptime. Health check uses /v1/health. Auth: basic auth or unauthenticated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wrap GreptimeDB in backticks in all doc comments to satisfy clippy::doc_markdown, and collapse the check closure to a single line to match rustfmt output. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
9ccb409 to
4c3b0c8
Compare
Summary
-p greptimedb/-p greptime).PromqlMetricProviderwith path prefix/v1/prometheusfor query, list, labels, label-values, and series.metric infosupport because GreptimeDB does not currently expose the Prometheus metadata endpoint.metric infolocally with anunsupportederror instead of calling the known-missing backend endpoint.obz-greptimedbskill in the embedded skill registry.Changes
crates/providers/src/greptimedb/mod.rsmeta()andbuild(); delegates supported PromQL commands and rejectsmetric infolocallycrates/providers/src/greptimedb/convert.rscrates/providers/src/greptimedb/response.rscrates/providers/src/lib.rsgreptimedb::meta()crates/obz/src/skills.rsobz-greptimedbskill and add testsskills/obz-greptimedb/SKILL.mdREADME.md/README.zh-CN.mdUsage
Test plan
cargo fmt --checkcargo test -p obz-providers greptimedb -- --nocapturecargo clippy --workspace --all-targets -- -D warningscargo test --workspacecargo deny check licenses bans sources(passes with existing unmatched-license warnings)cargo build./target/debug/obz metric info -p greptimedb --endpoint http://localhost:4000 upreturnsunsupportedwithout a backend request./target/debug/obz skills show obz-greptimedb./target/debug/obz skills list | rg 'obz-greptimedb|GreptimeDB'./target/debug/obz provider listshows thegreptimedbentry