Skip to content

Tighten up block builder encoding validation, fix tests to actually run#6037

Merged
mdisibio merged 3 commits intografana:mainfrom
mdisibio:bb-validation-test-fixes
Dec 8, 2025
Merged

Tighten up block builder encoding validation, fix tests to actually run#6037
mdisibio merged 3 commits intografana:mainfrom
mdisibio:bb-validation-test-fixes

Conversation

@mdisibio
Copy link
Copy Markdown
Contributor

@mdisibio mdisibio commented Dec 8, 2025

What this PR does:

(1) The block builder validates the encoding on startup, but it's not restrictive enough. Encodings like "vParquet5-preview1" wouldn't be caught on startup, but much later only when it attempted to flush a block. This tightens the check to only allow encodings that can be used to write new blocks, so deprecated and other preview versions will cause a startup error.

(2) Block-builder tests have been accidentally not running because we were missing the call to m.Run() in TestMain(). Whoops. This updates it so they run again, but also removes the flood of log statements which made running and understanding the tests quite cumbersome. We can substitute the console logger back in as needed when a regression is introduced.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@mdisibio
Copy link
Copy Markdown
Contributor Author

mdisibio commented Dec 8, 2025

BB tests are flakey in CI, so going to look for a few more fixes.

Copy link
Copy Markdown
Contributor

@mattdurham mattdurham left a comment

Choose a reason for hiding this comment

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

lgtm

@mdisibio mdisibio merged commit 02528de into grafana:main Dec 8, 2025
21 checks passed
oleg-kozlyuk-grafana pushed a commit to oleg-kozlyuk-grafana/tempo that referenced this pull request Dec 10, 2025
…un (grafana#6037)

* Tighten up block builder encoding validation, fix tests to actually run

* changelog

* Better shutdown checks?
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