refactor(rc): drop in-tracer RC tags normalization shim [stacked on #227]#242
Closed
bm1549 wants to merge 1 commit into
Closed
Conversation
Removes `normalize_rc_tags` (and its 3 unit tests + call site), the
temporary shim that converted RC's list-shape sampling-rule tags
(`[{key, value_glob}]`) into the map shape (`{key: value}`) that
libdd-sampling's deserializer currently requires.
This shim exists only because libdd-sampling does not yet parse the RC
wire shape natively. Once a libdd-sampling release lands that accepts
list-shape tags directly, this normalization is dead weight.
Stacked on #227. **Intentionally premature — DO NOT MERGE yet.** This
deletion is only safe once the workspace libdd-sampling dependency is
bumped to the release that parses list-shape tags natively. Merging the
shim removal alone regresses RC tag-qualified sampling in production
(the tracer would advertise RC sampling support but reject any rule with
tags). CI is RED until that bump.
The gate is `test_handler_rc_rules_with_list_tags_applied`: it fails
today with `invalid type: sequence, expected a map` and flips green once
the bump lands. Left red (not #[ignore]) deliberately, as a merge
interlock that prevents shipping the regression.
Kept (post-bump contract, behavior-level, no ref to the removed fn):
- test_handler_rc_rules_with_list_tags_applied (the green-light gate)
- test_handler_malformed_tags_rejects_update (re-validate post-bump:
libdd-sampling must still reject malformed list entries, not silently
drop them — the fail-closed invariant the shim used to guarantee; its
comment attributing rejection to the shim is now stale and should be
updated by the bump PR)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Contributor
Author
|
Folding this into the base PR #227 instead of keeping it stacked. libdd-sampling 2.1.0 shipped with native RC list-shape tag parsing, so the shim removal is no longer "premature" — #227 now bumps libdd-sampling 1.0.0 → 2.1.0 and drops the Closing as superseded — no separate cleanup PR needed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Deletes
normalize_rc_tagsfromdatadog-opentelemetry/src/core/configuration/remote_config.rs— the temporary shim added in #227 that converts Remote Config's list-shape sampling-rule tags ([{"key": "...", "value_glob": "..."}]) into the map shape ({"key": "value"}) thatlibdd-sampling'sSamplingRuleConfigdeserializer currently requires.Also removes the shim's 3 direct unit tests (
test_normalize_rc_tags_*) and its single call site. Net: -106 / +10 in one file.Motivation
The shim only exists because
libdd-sampling(currently pinned at1.0.0) deserializes ruletagsas a{key: value}map and rejects the RC wire shape. Once alibdd-samplingrelease lands that parses the list shape[{key, value_glob}]natively, the in-tracer normalization is dead weight — the RC JSON can flow straight through. This PR stages that cleanup so it's ready to land the moment the dependency supports it.Merge gate — read before merging
This change is only safe once
libdd-samplingis bumped to the release with native list-shape tag parsing. Merging the shim removal alone regresses production: the tracer would advertise RC sampling support but reject any rule carrying tags (invalid type: sequence, expected a map).The interlock is the kept test
test_handler_rc_rules_with_list_tags_applied:libdd-sampling 1.0.0(verified locally:invalid type: sequence, expected a map at line 1 column 43).libdd-samplingis bumped to the release that accepts list-shape tags.#[ignore]d) so CI cannot go green — and the PR cannot be cleanly merged — until the parser actually supports the wire shape.To land this PR: in this branch (or a precursor), bump
libdd-sampling(workspaceCargo.toml+Cargo.lock) to the release with native list-shape support, confirm the gate test goes green, then merge.Additional Notes
worktree-brian.marks+adaptive-sampling-fixes). Rebase ontomainonce fix(rc): accept RC list-shape tags and honor tracing_sampling_rate #227 merges.test_handler_malformed_tags_rejects_updatestill asserts that a rule with a malformed list-shape tag entry (missingvalue_glob) is rejected wholesale. Today that holds because libdd-sampling rejects all list-shape tags. Post-bump, the fail-closed invariant (don't silently drop a bad entry and broaden a tag-constrained rule) must be re-validated against the new parser, and that test's comment (which still attributes rejection to the removed shim) should be refreshed by the bump PR.🤖 Generated with Claude Code