Repository navigation
fix(collector): apply the cardinality guardrail to metric data points, not just spans (#172) - #201
Merged
Merged
Conversation
…, not just spans (#172) The agent config's transform processor dropped user.id / session.id / request.id / order.id from span attributes only. semconv-discipline ranks a metric dimension as the worst position for an identifier (one retained series per value, untouched by sampling, persisting in the backend after the code is fixed), so the guardrail covered the least damaging case and missed the most damaging one. The repo-specific keys from derived.highCardinalityAttributes (#37) had the same gap: they were only ever emitted as trace_statements. - collector-topology template and the three agent goldens: a metric_statements list (context: datapoint) with the same drop-list, and transform in the metrics pipeline between memory_limiter and batch. The generated file says that dropping a dimension merges the series that differed only by it. - /otel-collector adds each derived.highCardinalityAttributes key to both lists. - semconv-discipline said the Collector guardrail "operates on spans only" and "cannot reach a metric dimension", and every subagent that reads the skill was told the case must be fixed in code. Corrected: it covers spans and metric data points for the keys on its list, as a backstop that does not replace fixing the instrumentation. Log records are deliberately NOT guarded, which departs from the issue's proposal. semconv-discipline ranks a log-record attribute as the prescribed home for a per-request identifier, a log record is not a time series, and deleting user.id there would remove debugging data with no cardinality benefit. The template says so and when a log_statements list would be justified. Verified at runtime against otelcol-contrib 0.161.0 using the golden's own processor block: a metric data point and a span carrying user.id lose it and keep http.route, while a log record keeps both. The goldens still validate (tests/collector-validate.sh). collector-cardinality-contract.test.sh gains 17 checks: per golden, the two drop-lists are identical, the datapoint context is used, and traces and metrics run memory_limiter, transform, batch while logs stay unguarded. Bumps to 0.8.20.
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.
Summary
The agent config's cardinality guardrail cleaned span attributes only, while
semconv-disciplineranks a metric dimension as the worst position for an identifier. Fixes the metric half of #172; log records are deliberately left unguarded (below).What changed
collector-topologytemplate and the three agent goldens: ametric_statementslist (context: datapoint) with the same drop-list, andtransformin themetricspipeline betweenmemory_limiterandbatch. A comment notes that dropping a dimension merges the series that differed only by it/otel-collectoradds eachderived.highCardinalityAttributeskey to both listssemconv-disciplinesaid the Collector guardrail "operates on spans only" and "cannot reach a metric dimension"; corrected to spans and metric data points, as a backstop for the listed keyscollector-cardinality-contract.test.sh: 17 new checks. Bumps to 0.8.20Departs from the issue: log records are NOT guarded.
semconv-disciplineranks a log-record attribute as the prescribed home for a per-request identifier, and a log record is not a time series, so deletinguser.idthere would remove debugging data with no cardinality benefit. The template explains when alog_statementslist would be justified.Test plan
All hook, snapshot, drift and e2e unit tests pass locally;
claude plugin validate . --strictpasses;tests/collector-validate.shpasses against otelcol-contrib 0.161.0Runtime, with the golden's own processor block on Collector 0.161.0: a metric data point and a span carrying
user.idlose it and keephttp.route; a log record keeps bothTeeth: removing one key from the metric list fails the contract test; 3 checks fail against the previous skill and command text
Vendor, SDK and semantic-convention facts were verified against the source, not recalled — linked where it isn't obvious.
Nothing hardcoded that has a single source of truth (
SEMCONV_VERSION,backends.txt,hooks/otel-paths.sh).