Skip to content

Migrate the Grafana plugin build from grafana-toolkit to create-plugin - #118

Open
PDGGK wants to merge 2 commits into
apache:masterfrom
PDGGK:feature/grafana-create-plugin
Open

Migrate the Grafana plugin build from grafana-toolkit to create-plugin#118
PDGGK wants to merge 2 commits into
apache:masterfrom
PDGGK:feature/grafana-create-plugin

Conversation

@PDGGK

@PDGGK PDGGK commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Why

@grafana/toolkit was archived by Grafana and no longer receives updates, so npm run build was pinned to an unmaintained toolchain. This migrates the plugin's frontend build to @grafana/create-plugin, which is the supported replacement.

What changed

  • Build toolchain. grafana-toolkit plugin:build/test/dev are replaced by webpack, SWC/Jest, the flat ESLint config and Playwright, all driven from the generated .config/ directory. .config/ is checked in as create-plugin intends, carries its own "do not edit" notice, and can be refreshed later with npx @grafana/create-plugin update.

  • Lockfile. yarn.lock is removed in favour of package-lock.json, matching the npm-based scripts the new toolchain generates.

  • grafanaDependency moves from >=9.3.0 to >=12.3.0. This is the one compatibility decision in the change, so to be explicit about it: the plugin builds and is tested against @grafana/data, @grafana/ui and @grafana/runtime 13.1.0. Those packages are webpack externals (.config/bundler/externals.ts), resolved from the host Grafana at runtime rather than bundled — so an API that exists in the 13.1.0 types but not on an older host does not fail the build, it fails at runtime in the user's Grafana with no bundled fallback. Declaring >=12.3.0, the floor create-plugin declares for this SDK generation, states the range we can actually stand behind. If keeping a lower floor matters more, that is worth saying now — it would mean pinning the @grafana/* packages down instead.

  • Two ESLint errors fixed. The stricter config flags react-hooks/immutability in the tree-model query editors, where an array is mutated in place and the same reference is then passed to onChange — so a consumer comparing by reference sees no change. Both now pass a new array instead.

    To be plain about what this leaves behind: FromValue.tsx and SelectValue.tsx still contain the identical append (prefixPath[prefixPath.length] = ''; onChange(prefixPath)) a few lines below the corrected one, where the rule does not flag it, plus two splice-based removals with the same shape. I left them because these components have no test coverage and a toolchain migration is the wrong place to change untested UI behaviour — but I am happy to fix the remaining sites in a separate change with tests.

  • CI workflow updated in the same change. .github/workflows/grafana-plugin.yml triggers on connectors/grafana-plugin/**, pins node-version: "14.x", installs with yarn install --frozen-lockfile and keys both caches on hashFiles('**/yarn.lock'). Since this change removes yarn.lock, that workflow would fail at install, and Node 14 cannot run the new toolchain in any case. It now takes the Node version from the plugin's .nvmrc — so it cannot drift away from the toolchain again — uses setup-node's npm cache keyed on package-lock.json, and runs npm ci && npm run build. The Go/backend step is unchanged.

  • Development environment. docker-compose.yaml now also starts a standalone IoTDB next to Grafana, and provisioning/ wires the datasource to it (REST on 18080 for the tree-model modes, native Thrift on 6667 for the table-model mode). npm run server therefore brings up a Grafana with the plugin loaded and a server to query, instead of an empty instance.

    One known rough edge in the generated config: .config/supervisord/supervisord.conf waits for a backend binary matching gpx_iotdb*, derived from the plugin name, while plugin.json declares executable: gpx_apache_iotdb_datasource. That only affects DEVELOPMENT=true (the delve path); with the default DEVELOPMENT=false the entrypoint runs Grafana directly, which is the path verified below. I have not hand-edited it, since .config/ is generated and regenerated by create-plugin update.

Verification

Toolchain: typecheck, lint (0 errors; 9 remaining warnings are pre-existing deprecation notices), test:ci (13 tests) and build all pass. The exact command chain the updated workflow runs — npm ci && npm run build — was run from a clean install to confirm package-lock.json and package.json are in sync, since npm ci fails hard if they are not.

End to end against real servers, not just a green build: the plugin was loaded into Grafana 13.1.0 from docker compose up, with a real apache/iotdb:2.0.8 behind it. Grafana registers the plugin (Plugin registered pluginId=apache-iotdb-datasource), the provisioned datasource connects, and a table-model query

SELECT time, device_id, temperature FROM sensors WHERE $__timeFilter(time) ORDER BY time

renders in a panel with the row-major result correctly pivoted into one series per tag value, and the same data shown in the table view.

@grafana/toolkit was archived by Grafana and no longer receives updates,
so the plugin could not be built with a supported toolchain. This moves
the frontend build to @grafana/create-plugin: webpack, SWC/Jest, the flat
ESLint config, Playwright for e2e, and a package-lock.json replacing the
yarn.lock. The generated .config directory is checked in as create-plugin
intends and is not hand-edited; it can be refreshed with
'npx @grafana/create-plugin update'.

The plugin's own CI workflow is updated in the same change, because it is
what the migration breaks: it pinned Node 14, installed with
'yarn install --frozen-lockfile' and keyed both caches on yarn.lock, which
this change removes. It now reads the Node version from the plugin's
.nvmrc, caches on package-lock.json and runs 'npm ci && npm run build'.
The Go backend step is untouched.

grafanaDependency moves from >=9.3.0 to >=12.3.0. The plugin builds and is
tested against the @grafana/data, @grafana/ui and @grafana/runtime 13.1.0
packages the new toolchain pulls in. Those are webpack externals resolved
from the host Grafana at runtime, so an API present in the 13.1.0 types
but absent on an older host fails at runtime rather than at build time,
which is exactly why the declared floor should be the one we build and
test against.

The stricter ESLint config reports two errors in the tree-model query
editors, where an array is mutated in place and the same reference is then
handed to onChange. Both are changed to pass a new array instead. The same
append, and two splice-based removals, remain a few lines away where the
rule does not flag them; those are left for a separate change with tests
rather than altering untested UI behaviour here.

docker-compose.yaml also starts a standalone IoTDB alongside Grafana and
./provisioning wires the datasource to it, so 'npm run server' brings up a
working plugin against a real server rather than an empty Grafana.

Signed-off-by: Zihan Dai <99155080+PDGGK@users.noreply.github.com>
@CritasWang

Copy link
Copy Markdown
Contributor

Thanks for taking this on — moving off the archived @grafana/toolkit is the right direction, and the PR description is unusually thorough about its own trade-offs (the externals/grafanaDependency reasoning and the self-disclosed unfixed mutation sites are exactly the kind of thing that makes a migration reviewable).

Three things below. The first is blocking; the second is a suggestion that would have caught the first automatically; the third needs a call from the PMC rather than from you or me.


1. Blocking: connectors/grafana-plugin/pom.xml needs to change in this same PR

The PR description explains why .github/workflows/grafana-plugin.yml had to be updated here — "it is what the migration breaks". That reasoning is correct, but it applies equally to a second build path that this PR leaves untouched.

connectors/grafana-plugin/pom.xml drives the frontend build through frontend-maven-plugin, pinned to yarn:

<goal>install-node-and-yarn</goal>
<nodeVersion>v16.13.1</nodeVersion>
<yarnVersion>v1.22.17</yarnVersion>
...
<goal>yarn</goal>  <arguments>install</arguments>
<goal>yarn</goal>  <arguments>build</arguments>

After this PR that invocation runs Node 16 + Yarn 1.22 against a project that:

  • no longer has a yarn.lock (removed here),
  • declares "engines": { "node": ">=22" } and ships .nvmrc = 22,
  • builds via webpack -c ./.config/webpack/webpack.config.ts, which needs ts-node / @swc/core from the new toolchain.

So mvn -Pwith-grafana-plugin package — which CLAUDE.md documents as the canonical entrypoint for this plugin, with an explicit "do not invoke npm directly unless you know what the Maven wrapper is doing" — breaks on master right after this merges.

Roughly what it needs:

<goal>install-node-and-npm</goal>
<nodeVersion>v22.x.x</nodeVersion>   <!-- keep in sync with .nvmrc -->
...
<goal>npm</goal>  <arguments>ci</arguments>
<goal>npm</goal>  <arguments>run build</arguments>

Worth also dropping the ${basedir}/node_modules / ${basedir}/node clean filesets a second look, and noting that .npmrc's ignore-scripts=true (a good default, but an undocumented behaviour change in this PR — please call it out in the description) applies to the Maven-driven install too.

2. Why CI did not catch this, and a suggestion

All 12 checks are green, which is misleading here: no CI job builds this module through Maven at all.

  • compile-check.yml runs -P with-all-connectors -P with-examples, and with-all-connectors in connectors/pom.xml does not include grafana-plugin — it lives in the separate with-grafana / with-grafana-plugin profiles.
  • Jenkinsfile runs a bare mvn clean install, which also excludes those profiles.
  • The green build check is grafana-plugin.yml, i.e. the npm path this PR just rewrote — it is verifying its own new path.

The Maven path is a genuine blind spot, not a one-off oversight, so it will be missed again the same way. Suggest adding a job (either a step in compile-check.yml or a second job in grafana-plugin.yml) that runs:

mvn clean package -Pwith-grafana-plugin -DskipTests -ntp

That is cheap, and it is what turns item 1 from "someone has to remember" into "CI says so".

3. Two decisions that belong on dev@

Neither of these is something I think you should just decide in the PR:

grafanaDependency >=9.3.0>=12.3.0. Your reasoning is sound and well stated — the @grafana/* packages are webpack externals resolved from the host at runtime, so a 13.1.0-only API does not fail the build, it fails in the user's Grafana with no bundled fallback, and declaring a floor we can actually stand behind is more honest than a nominal >=9.3.0. But the practical effect is dropping support for Grafana 9/10/11 users of a released Apache plugin, and that is a project-level compatibility call. Please raise it on dev@ and let the PMC settle it; if the answer is "keep a lower floor", your own alternative (pinning the @grafana/* packages down instead) is the path.

The generated .config/AGENTS/ directory (4 files, ~316 lines). create-plugin now emits AI-agent instruction files (instructions.md, e2e-testing.md, skills/*.md) that tell an agent to fetch grafana.com at runtime and state "Do not modify anything inside the .config folder". Checking agent-directed instructions into an ASF repository is new ground, and that last rule sits somewhat awkwardly with ASF expectations about governance of code in the tree. Not objecting — just flagging that it should be a conscious PMC decision rather than something that arrives as a side effect of a toolchain bump. Also worth confirming whether connectors/grafana-plugin/LICENSE / NOTICE need a pass given the dependency set changed wholesale, even though these are devDependencies and grafana-plugin/** is RAT-excluded.


Item 1 is the only thing that has to change before this can merge. Items 2 and 3 can be follow-ups if the PMC prefers, though item 2 is small enough that folding it in here is probably easier than tracking it separately.

On the two remaining in-place array mutations in FromValue.tsx / SelectValue.tsx: agree with your call to leave them. A toolchain migration is the wrong place to change untested UI behaviour, and you flagged them explicitly rather than quietly — a separate PR with tests is the right shape.

The migration removed yarn.lock and moved the plugin to Node 22 and a webpack
config loaded through ts-node, but connectors/grafana-plugin/pom.xml still drove
frontend-maven-plugin with install-node-and-yarn, Node 16.13.1 and Yarn 1.22.17.
That is a second consumer of the toolchain this change replaces, alongside the
workflow already updated here, so mvn -Pwith-grafana-plugin package would have
broken on master straight after the merge.

The Maven executions now install Node from the same version the plugin declares
and run npm ci and npm run build. ci rather than install so the lockfile stays
authoritative and a drifted dependency set fails the build instead of resolving
silently. Note that .npmrc sets ignore-scripts=true, which applies to the
Maven-driven install as well.

The reason this was not caught is that no CI job builds this module through
Maven at all: grafana-plugin is not part of the with-all-connectors profile that
compile-check.yml uses, and the Jenkinsfile's plain mvn clean install does not
activate with-grafana-plugin either. The green build check is this workflow,
which exercises the npm path directly, so it was verifying its own new path. A
second job now runs the Maven entrypoint, which also covers the antrun execution
bound to the package phase that invokes backend-compile.sh -- that one is not
skipped by -DskipTests despite its execution id.

Signed-off-by: Zihan Dai <99155080+PDGGK@users.noreply.github.com>
@PDGGK

PDGGK commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — item 1 is a real break and I had missed it in exactly the way you describe. Pushed as c244362.

The pom. install-node-and-yarn / Node 16.13.1 / Yarn 1.22.17 → install-node-and-npm / Node 22.11.0, and yarn install / yarn buildnpm ci / npm run build. ci rather than install so the lockfile stays authoritative and a drifted dependency set fails the build instead of resolving silently. I left a comment on the node version pointing at .nvmrc so the two do not drift apart again, and called out in the description that .npmrc's ignore-scripts=true applies to the Maven-driven install as well — you were right that it was an undocumented behaviour change.

Verified locally: mvn clean package -Pwith-grafana-plugin -DskipTests -ntp is green end to end, 45s, with antrun:3.1.0:run (test) executing and six backend binaries plus module.js carrying build-time timestamps. One caveat on that run: I needed GOFLAGS=-buildvcs=false, which is a workaround for a broken .git directory in my own home path, not something CI needs — I deliberately did not put it in the job.

Item 2. Added as a second job in grafana-plugin.yml rather than a step in compile-check.yml, so it stays with the module it covers. I checked your diagnosis and it holds: with-all-connectors lists nine modules and grafana-plugin is not among them, and the Jenkinsfile's plain mvn clean install does not activate with-grafana-plugin either.

One thing worth recording from getting that job green: the antrun execution has <id>test</id> but is bound to <phase>package</phase>, so -DskipTests does not skip it — the Maven job really does run backend-compile.sh. That is fine, and it is why the job needs setup-go, but the id is misleading and I would not have caught it by reading the id alone.

Item 3. Agreed that both belong to the PMC rather than to this PR, and I will raise them on dev@ as one thread with two sections — the grafanaDependency floor as a compatibility decision, and the .config/AGENTS/ directory together with the LICENSE/NOTICE question you raised. I will link the thread here.

One observation I am deliberately not folding in, since it is pre-existing and this PR does not touch the file: backend-compile.sh runs go get -u github.com/grafana/grafana-plugin-sdk-go@v0.250.0 and go mod tidy at build time, so a build can mutate go.mod. Mine bumped magefile/mage from v1.15.0 to v1.17.2, which I reverted before committing. Happy to open that as its own issue once this lands.

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