[pull] master from DataDog:master#569
Merged
Merged
Conversation
* Add file-based YAML metrics loading for OpenMetrics V2 Add a new module `metrics_file.py` with: - MetricsPredicate protocol for conditional file loading - ConfigOptionTruthy, ConfigOptionEquals predicates - AllOf, AnyOf composable predicates - MetricsFile dataclass for declaring YAML metric files - MetricsConfig type alias Modify OpenMetricsBaseCheckV2 to: - Add METRICS_FILES class variable for declaring metric files - Load metrics from YAML files in get_config_with_defaults - Support convention-based discovery of metrics.yml - Add _load_file_based_metrics and _load_metrics_file helpers Include 39 unit tests covering all predicates, file loading, convention-based discovery, and config integration. * Add changelog entry for file-based metrics loading * Address PR review feedback - Add error handling in _load_metrics_file: catch yaml.YAMLError and validate loaded data is a dict, raising RuntimeError with descriptive messages for both cases - Document AllOf/AnyOf behavior with zero predicates (follows Python's all()/any() semantics) in docstrings - Refactor tests: convert from class-based to plain functions, use pytest fixtures for common setup, parametrize predicate tests - Add error scenario tests: malformed YAML, empty files, non-dict content * Address review feedback for file-based metrics loading Renames: - MetricsFile -> MetricsMapping (better describes the concept) - MetricsConfig -> RawMetricsConfig (clearer purpose) - metrics_file.py -> metrics_mapping.py - METRICS_FILES -> METRICS_MAP Improvements: - Add should_load() to MetricsMapping dataclass to encapsulate predicate evaluation - Extract _apply_file_metrics() helper so subclasses can build their own defaults dict without re-implementing file loading - Move yaml import inside _load_metrics_file to avoid loading pyyaml for integrations that don't use file-based metrics - Move typing imports behind TYPE_CHECKING - Use self.METRICS_MAP directly instead of local variable rename - Trim docstrings: shorter, newline after opening quotes, concise examples - Consolidate tests: plain functions, parametrize composites, extract _write_yaml helper, remove duplicate test patterns (370 -> 230 lines) * Fix file metrics mutation, add caching, and support metrics.yaml extension - Remove _apply_file_metrics in favour of inlining the logic in get_config_with_defaults, making the ownership of defaults explicit - Build a fresh list with list() + file_metrics instead of extend() so that any list returned by get_default_config is never mutated in place - Add instance-level _file_metrics cache on _load_file_based_metrics so YAML files are read only once per check instance regardless of how many times create_scraper is called (e.g. via refresh_scrapers overrides) - Support both metrics.yaml and metrics.yml for convention discovery, with metrics.yaml taking precedence when both are present - Document the contract on get_default_config: the returned dict can be mutated by the framework so subclasses must return a new dict each call * Apply ruff formatting * Improve type checking and tests * Update dependency resolution * Address review: ConfigurationError, public exports, fail-once cache - Export MetricsMapping, MetricsPredicate, ConfigOptionTruthy/Equals, AllOf, AnyOf from v2 package via lazy_loader stub. - Raise ConfigurationError (instead of RuntimeError) for malformed/unreadable YAML; catch OSError so FS errors surface as configuration errors. - Seal _file_metrics = [] before the load attempt so a malformed file fails once rather than re-raising on every scrape. - Document that overrides of get_config_with_defaults must call super() to keep file-based loading active. - Tests: vacuous AllOf()/AnyOf(), cache suppresses predicate re-evaluation on config change, permanent-failure-fails-once. * Harden METRICS_MAP defaults and metrics-file diagnostics - Use an immutable tuple as the METRICS_MAP class default to prevent subclasses from corrupting the base via .append; document yaml-then-yml convention discovery precedence. - Shallow-copy get_default_config() in get_config_with_defaults so a subclass that returns a cached top-level dict cannot accumulate file metrics across instances. - Reference the absolute file_path (not the relative path argument) in ConfigurationError messages so misconfigured metrics files name the directory being searched. - Patch _get_package_dir in test_load_file_based_metrics_no_files; add coverage for the cached-defaults pathway. * Tighten metrics_mapping internals and surface - Hoist yaml import to module top; yaml is a hard dependency of datadog_checks_base, not optional. - Chain ConfigurationError to the originating exception so YAML parse/IO context (line, column, errno) is preserved in stack traces. - Rename RawMetricsConfig to _RawMetricsConfig: it's the internal loader return type, not part of the integration-author surface. - Document the seal-on-any-error contract in _load_file_based_metrics: a single bad file in METRICS_MAP discards earlier successes; cache lands as []. - Document that ConfigOptionEquals treats a missing key as equal to None. - Add multi-file partial-success test and a lazy_loader stub smoke test for the v2 public re-exports. * Resolve metrics file paths in the caller and widen loader return type - Resolve METRICS_MAP file paths once in _load_file_based_metrics; _load_metrics_file now takes an absolute path. Removes the duplicate _get_package_dir lookup and makes the helper self-contained, so tests can pass a real path instead of patching the lookup. - Widen _RawMetricsConfig inner value to dict[str, Any] so the alias matches the transformer shapes the scraper consumes downstream. - Use tuple literals in the MetricsMapping docstring example and the inline test subclasses, matching the tuple[MetricsMapping, ...] annotation hardened in the previous commit. --------- Co-authored-by: dd-agent-integrations-bot[bot] <dd-agent-integrations-bot[bot]@users.noreply.github.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )