Skip to content

Update Tests - #147

Merged
ecbadeaux merged 1 commit into
mainfrom
Enable-Meter-Test
Oct 20, 2026
Merged

ecbadeaux merged 1 commit into
mainfrom
Enable-Meter-Test

Conversation

@ecbadeaux

@ecbadeaux ecbadeaux commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Fix MeterId hash, to_string ordering, and blank tags on the wire; run MeterID-test in CI

Summary

MeterID-test was never registered with CTest, so CI did not run it. Four of its tests fail on main:

  • HashSameTags and LookupTags: std::hash<MeterId> depends on tag iteration order, but operator== does not, so equal ids can hash differently and unordered_map<MeterId, ...> lookups miss.
  • ToString: to_string() prints tags in unordered-map order, which differs between standard libraries.
  • SpectatordId: the test asserts one specific tag order from an unordered map.

This PR fixes the code or the test for each, registers MeterID-test with add_test, and fixes one more bug found along the way: tags that validation drops still reached spectatord.

Changes

  • Hash: each tag pair is mixed on its own and the results are summed, so the hash no longer depends on iteration order. The pair hash is not symmetric in key and value, and mixing each pair keeps a key tied to its own value ({a: 1, b: 2} and {a: 2, b: 1} hash differently). The declaration in meter_id.h is unchanged.
  • to_string(): tags are sorted, so the output is deterministic.
  • Blank tags: GetTags() already dropped tags with an empty or whitespace-only key or value, but the spectatord id still rendered them, publishing series like foo,a=,b=2. The id now skips the same tags. One shared IsValidTag is used by both, so they cannot drift apart.
  • Tag order on the wire is unchanged for valid input. The id is still rendered from the caller's map in its own iteration order. When validation dropped nothing, which is the common case, no extra checks run.
  • Tests: MeterID-test is registered with add_test. The SpectatordId test accepts either tag order. New tests cover hash equality across construction paths and insertion order, distinct ids hashing differently, blank tags being omitted from the id, and WithTag/WithTags removing a tag set to blank.

No public API changes: helpers are in an anonymous namespace, no signatures change, and WithStat is untouched. This is suitable for a patch release.

Behavior change

A caller that passed a blank key or value used to publish a bogus series (for example k=) alongside the valid tags. That series is no longer published. For valid tags the output is byte-identical to main; I

Meter creation and emit paths are unchanged. Nothing in the library calls std::hash<MeterId>.

@ecbadeaux
ecbadeaux merged commit ff30bf4 into main Oct 20, 2026
1 check passed
@ecbadeaux
ecbadeaux deleted the Enable-Meter-Test branch October 20, 2026 18:17
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.

1 participant