Enhance WeaverLiveCheck runner capabilities (#5526) - #5580
Conversation
…butes BoundedAttributes.__setitem__ and _set_items, when operating in extended_attributes mode, called _clean_extended_attribute and then stored the result unconditionally. The problem: _clean_extended_attribute returns None for BOTH: 1. A valid caller-supplied None value. 2. An invalid key (empty / non-string key). So an entry with a valid None value was silently skipped (never stored) because None was interpreted as 'invalid'. Fix: introduce an _INVALID_ATTRIBUTE sentinel object that _clean_extended_attribute returns when the key or value is genuinely invalid. Only the sentinel triggers the early-return / skip path. A true None value now flows through to _setitem_locked as intended. Also update _clean_extended_attribute return type and docstring, and add a regression test (test_extended_attributes_none_value_stored). Fixes open-telemetry#5432
|
Pull request dashboard statusWaiting on the author · refreshed 2026-08-24 05:52 UTC Resolve merge conflicts. Two things need attention:
Status above doesn't look right?
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9640e53edf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Sentinel object returned by _clean_extended_attribute to signal that the | ||
| # key/value was invalid and must not be stored in the attributes dict. | ||
| # A plain ``None`` cannot serve this purpose because ``None`` is a valid | ||
| # AnyValue and must be stored when explicitly set by the caller. | ||
| _INVALID_ATTRIBUTE = object() | ||
|
|
There was a problem hiding this comment.
Remove this attribute-cleaning behavior change from the WeaverLiveCheck commit. Storing explicit None values in BoundedAttributes is unrelated to the runner enhancements described by this commit, affects a separate public package, and introduces its own compatibility and regression surface; it should be reviewed and tested independently as a narrowly scoped change.
AGENTS.md reference: AGENTS.md:L17-L19
Useful? React with 👍 / 👎.
| except TypeError as exception: | ||
| _logger.warning("Attribute %s: %s", key, exception) | ||
| return None | ||
| return _INVALID_ATTRIBUTE |
There was a problem hiding this comment.
When an extended attribute is a mapping whose nested value cannot be stringified, the recursive _clean_extended_attribute call returns this sentinel, but _clean_extended_attribute_value assigns that result directly into cleaned_dict. The outer attribute is then accepted with a plain object embedded in it, violating the AnyValue contract and potentially causing serialization/export failures; the recursive mapping path must detect and omit the sentinel rather than store it.
Useful? React with 👍 / 👎.
| urllib_logger = logging.getLogger("urllib3.connectionpool") | ||
| prev_level = urllib_logger.level | ||
| urllib_logger.setLevel(logging.ERROR) |
There was a problem hiding this comment.
When two live checks start concurrently, each saves and changes the same process-global logger level: the second can save ERROR, the first can restore the original level, and the second then restores ERROR, permanently suppressing urllib3 warnings after both startups finish. This also suppresses unrelated requests made by other threads during polling, so readiness checks should silence only their own traffic rather than mutate the shared logger.
Useful? React with 👍 / 👎.
1 participant
Summary
Fixes #5526
Enhance
WeaverLiveCheckandLiveCheckReportfor programmatic test and conformance runner integration.Changes
.rawproperty and.to_dict()method returningself._report. Added typed properties.samplesand.statistics.stdoutandstderrattributes for failure diagnostics.config(--config) andadvice_data(--advice-data), and allowedpolicies_dirto be aSequence[str].startup_timeoutparameter and made readiness polling quiet by silencing transient connection warnings.stdoutandstderrproperties reading the captured output.end()andend_and_check()raiseRuntimeErrorif called when weaver was already stopped.tests/opentelemetry-test-utils/tests/test_weaver_live_check.py.