Skip to content

Grafana: Basic end-to-end example / integration test#1442

Merged
amotl merged 4 commits intomainfrom
grafana
Mar 24, 2026
Merged

Grafana: Basic end-to-end example / integration test#1442
amotl merged 4 commits intomainfrom
grafana

Conversation

@amotl
Copy link
Member

@amotl amotl commented Mar 3, 2026

About

we don't run integration tests against Grafana yet

Review

While CI is failing on some matrix slots, the patch can still be reviewed and merged, because it is valid, as demonstrated on CI slots of earlier Grafana releases. Thanks!

References

/cc @karynzv, @hammerhead, @goat-ssh, @zolbatar@WalBeh

@coderabbitai
Copy link

coderabbitai bot commented Mar 3, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds a Grafana integration: Dependabot entries, a new GitHub Actions "Grafana" workflow, a Docker Compose stack (CrateDB, Grafana, example task), a Python provisioning/validation script with requirements, and a test runner script for end‑to‑end verification.

Changes

Cohort / File(s) Summary
Dependabot
\.github/dependabot.yml
Added three Dependabot entries for application/grafana (package-ecosystems: pip, docker, docker-compose) scheduled daily.
CI Workflow
\.github/workflows/application-grafana.yml
New "Grafana" GitHub Actions workflow: triggers on PR/push/manual/schedule, concurrency cancel-in-progress, matrix over OS/CrateDB/Grafana, runs application/grafana/test.sh.
Compose & Docker
application/grafana/compose.yml
New Docker Compose defining cratedb, grafana, and example-weather (inline Dockerfile); healthchecks for CrateDB/Grafana; example-weather depends on healthy services and uses tasks profile.
Example App & Requirements
application/grafana/example-weather.py, application/grafana/requirements.txt
New Python example that loads sample weather data into CrateDB, provisions a Grafana datasource and dashboard via API, and validates datasource/dashboard queries; requirements added (cratedb-toolkit, grafana-client<6, grafanalib<0.8).
Test runner
application/grafana/test.sh
Shell script to bring up the compose stack (docker compose up --wait -d) and run the example-weather task via docker compose run.

Sequence Diagram

sequenceDiagram
    participant GHA as GitHub Actions
    participant Docker as Docker Compose
    participant CDB as CrateDB
    participant GF as Grafana
    participant Script as example-weather.py

    GHA->>Docker: docker compose up --wait -d
    Docker->>CDB: start CrateDB container
    Docker->>GF: start Grafana container
    CDB-->>Docker: healthcheck OK
    GF-->>Docker: healthcheck OK

    Docker->>Script: docker compose run example-weather
    Script->>CDB: insert sample weather data (SQL)
    CDB-->>Script: insert OK

    Script->>GF: create datasource via Grafana API
    GF-->>Script: datasource created / exists

    Script->>GF: upload/update dashboard
    GF-->>Script: dashboard created/ack

    Script->>GF: validate datasource health
    GF-->>Script: health OK

    Script->>GF: trigger dashboard queries (Grafana -> CrateDB)
    GF->>CDB: execute SQL queries
    CDB-->>GF: query results
    GF-->>Script: query responses (HTTP 200)

    Script-->>Docker: validation complete
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • seut
  • matriv
  • hammerhead

Poem

🐰 I hopped through compose and CI light,
I planted datasources late at night,
Weather rows in CrateDB sing,
Grafana panels bloom in spring,
A little rabbit cheers the flight. 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main change: adding a basic end-to-end example and integration test for Grafana, which aligns with the file additions and purpose evident in the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description check ✅ Passed The pull request description clearly relates to the changeset by explaining the purpose (adding Grafana integration tests) and acknowledging CI issues.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch grafana

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@amotl amotl requested review from matriv and seut March 3, 2026 05:15
@amotl amotl marked this pull request as ready for review March 3, 2026 05:17
coderabbitai[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as off-topic.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@application/grafana/example-weather.py`:
- Around line 143-147: The current hard assert on
grafana.datasource.health_inquiry(DATASOURCE_UID) can fail due to Grafana's
health-check SQL being incompatible with CrateDB; keep the two logger.info lines
for health.status and health.message but remove the assert and instead run an
explicit lightweight smoke query via the smartquery API (e.g., call smartquery
with a trivial SELECT/limit against DATASOURCE_UID) to determine pass/fail; if
the smartquery returns a successful result treat the datasource as healthy,
otherwise log the smartquery error and raise/assert failure. Ensure you
reference grafana.datasource.health_inquiry, DATASOURCE_UID, health, logger, and
smartquery when making the change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 985816ff-6a03-44ee-ab7d-29b15d1bde41

📥 Commits

Reviewing files that changed from the base of the PR and between 61e93bb and 09603e7.

📒 Files selected for processing (6)
  • .github/dependabot.yml
  • .github/workflows/application-grafana.yml
  • application/grafana/compose.yml
  • application/grafana/example-weather.py
  • application/grafana/requirements.txt
  • application/grafana/test.sh
🚧 Files skipped from review as they are similar to previous changes (3)
  • application/grafana/requirements.txt
  • application/grafana/test.sh
  • application/grafana/compose.yml

@amotl amotl changed the title Grafana: Weather data example (integration test) Grafana: Basic end-to-end example / integration test Mar 5, 2026
@amotl amotl force-pushed the grafana branch 5 times, most recently from c527bfc to d075101 Compare March 9, 2026 08:53
@amotl
Copy link
Member Author

amotl commented Mar 10, 2026

Is it possible that this test rig can also validate the column name autocompletion feature?

Copy link
Member

@seut seut left a comment

Choose a reason for hiding this comment

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

thanks! just some versions to check suggestions

- "11.6"
- "12.3.4"
- "12.4"
- "nightly"
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it makes sense to test a grafana nightly, it may fail for valid reasons which could never land in a release.

Suggested change
- "nightly"

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this would only be the case with main. Using nightly here is the right choice to receive "going south" signals early. If we see too much flakyness due to unrelated instabilities in Grafana nightly, we can always remove that label again. I don't expect many of such, because we are only testing a very minor surface of Grafana, and I don't expect them to ship any completely dysfunctional releases. Grafana nightly is well tested.

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to instead ensure that we update this once a new major or minor version is released (dependbot?). I'd like to avoid any possible effort caused by a nightly version which may never see the light. Anyhow, we can also test and remove it once it causes issues.

@amotl amotl requested a review from seut March 16, 2026 21:45
Comment on lines +106 to +113
SELECT
$__timeGroupAlias("timestamp", $__interval),
"location",
MEAN("{column_name}") AS "{column_name}"
FROM "example"."weather_data"
WHERE $__timeFilter("timestamp")
GROUP BY "time", "location"
ORDER BY "time"
Copy link
Member Author

@amotl amotl Mar 16, 2026

Choose a reason for hiding this comment

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

Please note this querying scheme+template is super important to follow, otherwise you will set the wire and the browser on fire when processing big data.

@hammerhead told me about this the other day, but I don't know if it is general knowledge across the board and is correctly applied by all our users and customers. 1

Footnotes

  1. I have a slight suspicion users can overload their clusters easily because nobody tells them about those details. In this spirit, a dedicated CrateDB datasource plugin for Grafana would be very sweet indeed, which would optimally guide the user appropriately, similarly like InfluxDB users, for example, who do not need to be concerned about relevant details at all?

Copy link
Member Author

Choose a reason for hiding this comment

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

However, in the following comment, you can see @matriv pinged me about the DATE_BIN() function that had been recommended by @seut.

If that works now, I would be happy to demonstrate the modern version here, even if it requires CrateDB 5.7. We can easily add the SQL template above to the documentation to inform users of older CrateDB versions.

Copy link
Member

Choose a reason for hiding this comment

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

Would be great to do that, 5.7 is already a bit old.

Copy link
Member Author

Choose a reason for hiding this comment

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

Did someone use the new variant successfully already, so we can copy from there?

/cc @hammerhead, @grbade, @zolbatar

Copy link
Member Author

Choose a reason for hiding this comment

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

@zolbatar will look into this detail. Thank you! 🙇

Copy link
Member Author

Choose a reason for hiding this comment

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

@coderabbitai: Please capture this into a dedicated issue, so we can improve later. Thank you.

Copy link

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

gh issue create \
  --repo crate/cratedb-examples \
  --title "Grafana: Improve SQL template to use DATE_BIN() (CrateDB 5.7+)" \
  --body "## Summary

In the Grafana end-to-end example ([application/grafana/example-weather.py](https://github.com/crate/cratedb-examples/blob/grafana/application/grafana/example-weather.py)), the SQL query template currently uses \`\$__timeGroupAlias(\"timestamp\", \$__interval)\` together with \`MEAN()\` for time-series aggregation.

It has been recommended to adopt the modern \`DATE_BIN()\` function (available since CrateDB 5.7) as the preferred SQL template for Grafana queries, since CrateDB 5.7 is already a reasonably old baseline.

Using the correct querying scheme and template is critical — as noted by `@hammerhead` — to avoid overloading the cluster or setting the wire and the browser on fire when processing large datasets. This is important knowledge that is not always applied correctly by users and customers.

## Background

- `@matriv` flagged the \`DATE_BIN()\` function (recommended by `@seut`) as the modern replacement in [this comment](https://github.com/daq-tools/kotori/pull/148#discussion_r1499035232).
- The original discussion about the querying scheme can be found [here](https://github.com/daq-tools/kotori/pull/148#discussion_r1237795785).
- `@zolbatar` has volunteered to look into the details of the new \`DATE_BIN()\` variant.

## Tasks

- [ ] Validate the \`DATE_BIN()\` SQL template for Grafana with CrateDB (coordinated with `@zolbatar`)
- [ ] Update the SQL query in \`application/grafana/example-weather.py\` to use the modern \`DATE_BIN()\` approach
- [ ] Add the legacy \`\$__timeGroupAlias\` SQL template to the documentation as a reference for users on older CrateDB versions

## References

- PR: https://github.com/crate/cratedb-examples/pull/1442
- Comment: https://github.com/crate/cratedb-examples/pull/1442#discussion_r2943186202
- Requested by: `@amotl`
" \
  --assignee amotl

Length of output: 53


@amotl, done! I've created issue #1508 to track this improvement:

👉 #1508

"Grafana: Improve SQL template to use DATE_BIN() (CrateDB 5.7+)"

The issue captures:

  • The motivation for adopting DATE_BIN() as the modern time-series aggregation approach (replacing $__timeGroupAlias + MEAN())
  • The importance of using the correct querying scheme to avoid overloading clusters
  • References to the original discussions with @hammerhead, @seut, and @matriv
  • @zolbatar's involvement in validating the new variant
  • Tasks for updating example-weather.py and documenting the legacy template for older CrateDB versions

(∩^ω^)⊃━☆゚.*・。゚

- directory: "/application/grafana"
package-ecosystem: "docker-compose"
schedule:
interval: "daily"
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't weekly be enough? (same for the other jobs)

Suggested change
interval: "daily"
interval: "weekly"

Copy link
Member Author

@amotl amotl Mar 17, 2026

Choose a reason for hiding this comment

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

We want to be informed in-time when something goes south, so we can start conversations with upstream authors earlier than later.

Comment on lines +106 to +113
SELECT
$__timeGroupAlias("timestamp", $__interval),
"location",
MEAN("{column_name}") AS "{column_name}"
FROM "example"."weather_data"
WHERE $__timeFilter("timestamp")
GROUP BY "time", "location"
ORDER BY "time"
Copy link
Member

Choose a reason for hiding this comment

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

Would be great to do that, 5.7 is already a bit old.

@amotl amotl merged commit b6b52cd into main Mar 24, 2026
8 checks passed
@amotl amotl deleted the grafana branch March 24, 2026 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants