maintenance: define VictoriaMetrics label collisions

[zqr10159]

Summary

• define an explicit collision policy shared by the single-node and cluster VictoriaMetrics writers
• keep job and instance as ordinary Prometheus custom labels, preserving the exact label set and series identity used before this PR
• reject a metrics batch when monitor custom labels use HertzBeat-managed keys: __name__, __monitor_id__, __metrics__, or __metric__
• report conflicting key names without logging their values; do not silently discard or rename user labels
• document the upgrade check and clarify that existing VictoriaMetrics series are not rewritten

Upgrade behavior

• monitors using job, instance, or ordinary custom labels require no migration and continue writing the same label set
• before upgrade, rename any monitor custom label using one of the four HertzBeat-managed keys
• a remaining managed-key collision is observable and fail-closed: that metrics batch is not sent, and the application log lists only the conflicting key names

Validation

• regression contracts first failed on the previous head: the writer changed existing job/instance values and silently accepted managed-key collisions
• ./mvnw -pl hertzbeat-warehouse -Dtest=VictoriaMetricsDataStorageTest test -DskipITs -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false
  • 6 tests passed; Checkstyle passed
• ./mvnw -pl hertzbeat-warehouse -am test -DskipITs
  • complete reactor passed; hertzbeat-warehouse ran 61 tests with 0 failures and 0 errors
• git diff --cached --check

AI assistance: used for draft implementation and test iteration.
Human validation: the serialized write payload retains existing custom job and instance values, managed-key collisions produce a value-free diagnostic and no HTTP write, and the complete warehouse reactor passed locally.
Risk notes: monitors that currently use a HertzBeat-managed custom-label key stop writing VictoriaMetrics batches until the key is renamed. This is intentional and documented so the upgrade cannot silently split or relabel an existing series.

[zqr10159]

Author remediation update:

The collision policy is now explicit and shared by both VictoriaMetrics writers. Existing Prometheus-style job, instance, and ordinary custom labels are preserved exactly. HertzBeat-managed keys are rejected with a key-only diagnostic and no write; user labels are not silently discarded or renamed. The upgrade note identifies the four managed keys that must be renamed.

Payload-continuity and no-write diagnostic tests passed (6 focused tests), and the full warehouse reactor passed (61 tests). Backend, E2E, license, and label checks are green. DOC CI failed only because an unchanged historical Gitee link returned HTTP 405; that failed job has been rerun. Maintainer review remains required.

[zqr10159]

CI follow-up: the DOC CI rerun passed, including Dead Link Check and the documentation build. Backend, Maven E2E, image E2E, license, and label checks are also green; all current checks have completed successfully.

[Duansg]

The problem is real — labels.putAll(customizedLabels) runs after name, monitor_id, metrics and metric are set, so a monitor custom label using one of those keys silently rewrites the series identity (a custom name in particular redirects samples into an arbitrary metric name). Defining an explicit collision policy is the right call, and keeping job/instance as ordinary labels avoids churning existing series. Two things I'd like to raise.

addCustomizedLabels re-runs the collision check per sample

saveData() already rejects the batch up front, so by the time the write loop runs the label map is known to be conflict-free. But addCustomizedLabels now sits where the old putAll was — inside the per-row × per-numeric-field loop (VictoriaMetricsDataStorage.java:217-244) — and each call re-executes:

Set collisions = new TreeSet<>(customizedLabels.keySet());
collisions.retainAll(MANAGED_LABEL_KEYS);

That's a fresh TreeSet built from every custom label key, plus a retainAll, for every sample. A monitor emitting 1,000 rows × 20 numeric fields does 20,000 redundant allocations per batch, on the hottest path in the write pipeline. The IllegalArgumentException inside it is unreachable for the same reason.

The cluster variant is worse: there the for (Map.Entry<String, Double> ...) loop is nested inside the cellStream().forEach(cell -> {...}) lambda (VictoriaMetricsClusterDataStorage.java:218-262), so the call count is rows × cells × fields. That nesting looks like a pre-existing bug — it also appears to emit duplicate VictoriaMetricsContent entries — and it's out of scope here, but it does multiply the cost of this change.

Suggestion: keep the single entry-point validation and let the inner loop do a plain labels.putAll(customizedLabels).

Rejecting the whole batch is itself silent data loss

The PR description says the goal is to "not silently discard or rename user labels", but dropping the entire metrics batch discards strictly more than renaming one label would. From an operator's point of view a single mistyped label key makes all metrics for that monitor disappear, with only an ERROR line in the log to explain it.

Fail-closed is defensible, but it would help to make the failure observable beyond a log line — a counter, or surfacing it on the monitor itself — since the affected monitor otherwise just looks like it stopped reporting. Alternatively, dropping only the conflicting keys and keeping the sample preserves the data while still protecting series identity.

Coverage

GreptimeDbDataStorage.java:96 declares the same LABEL_KEY_NAME = "name" and follows the same "set managed labels, then putAll custom labels" pattern, so it has the identical collision. Worth either covering it here or noting it as a follow-up so it doesn't get lost.

Minor

OutputCaptureExtension assertions on log text are sensitive to logging configuration; asserting on a returned/collected value would be more robust if there's a natural seam for it. Not blocking.

[zqr10159]

Addressed the review findings in commit b87ff44df7.

Changes:

• VictoriaMetrics collision validation now runs once per incoming batch instead of allocating/revalidating for every emitted sample;
• single-node and cluster VictoriaMetrics storage count rejected collision batches and include the cumulative count in value-free diagnostics;
• Greptime now uses the same explicit reject policy for custom labels that collide with instance, ts, or metric fields instead of silently skipping them;
• non-conflicting Greptime custom tags remain supported.

Human validation:

• full VictoriaMetricsDataStorageTest,GreptimeDbDataStorageTest suites passed;
• focused collision and allowed-label contracts passed;
• git diff --check.

The new-head backend, Maven E2E, license, docs, and label checks are currently queued by GitHub and have not started yet.

AI assistance: used for draft implementation and test iteration.
Risk notes: collision rejection is intentionally observable and batch-scoped; it does not rewrite or namespace user labels.

@Duansg, please re-review this head when convenient.