Skip to content

feat: metrics schema version #12732

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

vittoriopolverino
Copy link
Member

@vittoriopolverino vittoriopolverino commented Jun 9, 2025

Motivation

This PR introduces explicit schema versioning for metrics definitions.
The goal is to make telemetry changes safer and more maintainable:

  • enabling easier debugging and auditability of metric changes over time
  • makes it easier to analyze data across schema versions by maintaining backward compatibility

For now:.

  • No persistent file or central registry to verify schema consistency
  • No strict validation

However, having the schema_version available in Tinybird helps with:

  • Managing backward compatibility.
  • Reconciling metrics whose schema may have evolved over time.
    • For example, suppose an initial version of a counter uses label_1_value to store the status of a request, with possible values like "success" and "error". Later, the counter is updated: a new label label_2 is introduced to hold the status instead:
multiIf(
      schema_version = 1, label_1_value,
      schema_version = 2, label_6_value,
      schema_version = 3, label_*_value,
      ...
  ) AS status

A more structured approach would require:

  • A schema registry.
  • Explicit contracts for each metric.
  • Validation at build time, CI, or runtime to ensure consistency between contracts and the metric definitions in code.

Changes

  • Introduced a naive approach for handling schema_version in metrics.

@vittoriopolverino vittoriopolverino self-assigned this Jun 9, 2025
@vittoriopolverino vittoriopolverino added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Jun 9, 2025
@vittoriopolverino vittoriopolverino added this to the 4.6 milestone Jun 9, 2025
@vittoriopolverino vittoriopolverino force-pushed the DAT-156-metrics-schema-version branch from 5025e2a to 8fdc9e8 Compare June 12, 2025 13:35
Copy link

github-actions bot commented Jun 12, 2025

Test Results - Preflight, Unit

21 617 tests  +4   19 962 ✅ +4   6m 19s ⏱️ -34s
     1 suites ±0    1 655 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit f276526. ± Comparison against base commit 3bbf944.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jun 12, 2025

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 6s ⏱️ -32s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit f276526. ± Comparison against base commit 3bbf944.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jun 12, 2025

Test Results - Alternative Providers

987 tests   589 ✅  30m 27s ⏱️
  4 suites  398 💤
  4 files      0 ❌

Results for commit f276526.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jun 12, 2025

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 22m 21s ⏱️
5 230 tests 4 301 ✅ 929 💤 0 ❌
5 236 runs  4 301 ✅ 935 💤 0 ❌

Results for commit f276526.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jun 12, 2025

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 45m 19s ⏱️ + 2m 14s
4 873 tests ±0  4 096 ✅ ±0  777 💤 ±0  0 ❌ ±0 
4 875 runs  ±0  4 096 ✅ ±0  779 💤 ±0  0 ❌ ±0 

Results for commit f276526. ± Comparison against base commit 3bbf944.

♻️ This comment has been updated with latest results.

@@ -132,15 +151,25 @@ class LabeledCounter(Metric):
"""

_type: str
_schema_version: int
Copy link
Member Author

@vittoriopolverino vittoriopolverino Jun 12, 2025

Choose a reason for hiding this comment

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

note: for now, I’m adding schema_version only to the counter implementations. If it proves to be relevant for other metric types as well, we’ll move its definition up to the base Metric class.


def __init__(self, namespace: str, name: str):
def __init__(self, namespace: str, name: str, schema_version: int = 1):
Copy link
Member Author

Choose a reason for hiding this comment

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

note: default is set to 1 and automatically applied to all existing counters.

Comment on lines 111 to 112
if schema_version is None:
raise ValueError("Labeled metrics require an explicit schema version.")
Copy link
Member Author

@vittoriopolverino vittoriopolverino Jun 12, 2025

Choose a reason for hiding this comment

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

thought: It shouldn’t ever happen given the default is set to 1,

question: should we remove the check, or simply keep it as an extra safeguard?

Comment on lines +113 to +118

if not isinstance(schema_version, int):
raise TypeError("Schema version must be an integer.")

if schema_version <= 0:
raise ValueError("Schema version must be greater than zero.")
Copy link
Member Author

@vittoriopolverino vittoriopolverino Jun 12, 2025

Choose a reason for hiding this comment

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

note: naive approach. At the moment, we can't verify whether the schema_version is being incremented sequentially. However, once approved, we’ll explicitly document how schema_version should be used going forward.

@vittoriopolverino vittoriopolverino marked this pull request as ready for review June 12, 2025 15:22
@vittoriopolverino vittoriopolverino requested a review from thrau as a code owner June 12, 2025 15:22
@vittoriopolverino vittoriopolverino marked this pull request as draft June 12, 2025 15:23
@vittoriopolverino vittoriopolverino marked this pull request as ready for review June 12, 2025 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant