Skip to content

Add CloudWatch reporting datastore#1088

Open
OVI3D0 wants to merge 23 commits into
opensearch-project:mainfrom
OVI3D0:feat/cloudwatch-datastore-clean
Open

Add CloudWatch reporting datastore#1088
OVI3D0 wants to merge 23 commits into
opensearch-project:mainfrom
OVI3D0:feat/cloudwatch-datastore-clean

Conversation

@OVI3D0

@OVI3D0 OVI3D0 commented Jul 15, 2026

Copy link
Copy Markdown
Member

Summary

Adds Amazon CloudWatch as an optional reporting datastore, alongside the
existing OpenSearch and in-memory stores. Follows RFC #1083.

  • Write path: samples and aggregated results are written as EMF
    (Embedded Metric Format) via PutLogEvents — one call produces both a
    queryable log line and a CloudWatch metric, no separate PutMetricData.
  • Read path: osb list test-runs, osb compare, and the aggregation
    flow pull results back via Logs Insights.
  • Auth: default boto3 credential chain (env vars → profile → IMDS).
    Optional datastore.role_arn triggers an STS auto-refreshing wrapper
    so long benchmarks survive token expiry.

Motivation

Today OSB requires either an OpenSearch cluster or the in-memory store for
reporting. Standing up a dedicated results cluster is overhead operators
often want to avoid when they already run in AWS. CloudWatch needs no infra
provisioning and has native dashboarding.

Config

[reporting]
datastore.type = cloudwatch
datastore.region = us-west-2
datastore.log_retention_days = 30       # optional
datastore.profile = my-profile          # optional
datastore.role_arn = arn:aws:iam::...   # optional; STS auto-refresh

What's in this PR

  • Registry refactor: `metrics_stores/{init, in_memory, opensearch}`
  • CloudWatch backend: `metrics_stores/cloudwatch/` — client, config, EMF
    builder, log-stream writer, Insights query, three stores (metrics /
    results / test-run)
  • `boto3>=1.34.0` added to `setup.py`
  • Docs: `docs/user-guides/cloudwatch-datastore.md`
  • Tests: read + write path, ~1,300 LOC

Test plan

  • Unit: `pytest tests/metrics_stores/cloudwatch/` — EMF formatting,
    batch chunking, retention validation, Insights parsing, error paths
  • Live end-to-end on AWS: `osb run --workload=geonames --test-mode`
    with `datastore.type=cloudwatch` → EMF landed in `benchmark-metrics`
    log group with valid `_aws` block; `osb list test-runs` and
    `osb compare` both round-tripped via Logs Insights

Related

@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit bc40375.

PathLineSeverityDescription
101highDependency version bump: boto3 minimum raised from >=1.28.62 to >=1.34.0. Per mandatory review policy, all dependency changes must be flagged regardless of apparent legitimacy. Maintainers should verify the new lower bound resolves to the intended artifact and that no transitive dependency changes introduce malicious behavior.
361mediumThe `sort_key` parameter passed to `get_one` is interpolated directly into a CloudWatch Logs Insights query string inside backtick delimiters without sanitization. `_escape_query_value` is applied to `safe_name` but never to `sort_field`. A sort_key value containing a newline, pipe, or other Insights control characters (e.g., `x\n| fields @message`) could alter the query structure and potentially leak unintended log events.
121lowThe `_assume_role` method sets `botocore_session._credentials` directly — a private/undocumented attribute — to inject a custom `DeferredRefreshableCredentials` object. The comment acknowledges this is a private API. If the botocore internal attribute is renamed or its semantics change, the assume-role path could silently fall back to the outer session's identity rather than the assumed role, causing metrics to be written under the wrong IAM principal without any error.

The table above displays the top 10 most important findings.

Total: 3 | Critical: 0 | High: 1 | Medium: 1 | Low: 1


Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.


⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

OVI3D0 added 22 commits July 15, 2026 13:25
Adds osbenchmark/metrics_stores/cloudwatch/ with empty modules whose
docstrings describe their planned responsibilities. No behavior change;
subsequent commits wire each module's implementation and register the
CloudWatch backend with the metrics-store dispatchers in osbenchmark/metrics.py.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
`metrics_store_class`, `test_run_store`, and `results_store` previously
branched on `cfg.opts("reporting", "datastore.type") == "opensearch"` to
choose between the OpenSearch-backed and the in-memory / file / no-op
implementations. Replace the three independent if/else chains with a
single `_DATASTORE_REGISTRY` mapping `datastore.type` values to factory
triples, plus a `_DATASTORE_DEFAULT` that preserves the previous
fall-through behavior. Unknown datastore types still resolve to the
defaults exactly as before.

This is a pure refactor with no behavior change for existing
configurations. It establishes the extension point that the upcoming
CloudWatch datastore will plug into.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
Adds osbenchmark/metrics_stores/cloudwatch/config.py with a `load(cfg)`
helper that reads the CloudWatch-specific keys from the existing
[reporting] section of benchmark.ini and returns a frozen CloudWatchConfig
dataclass.

Keys read:
- datastore.region (required)
- datastore.namespace (default: OSB)
- datastore.log_group.{metrics,test_runs,results} (defaults: benchmark-*)
- datastore.log_retention_days (validated against the fixed CloudWatch Logs
  retention enum: 1, 3, 5, 7, 14, 30, 60, 90, 120, 150, 180, 365, ...)
- datastore.profile, datastore.role_arn (optional AWS auth overrides)
- datastore.cloudwatch.spool.{enabled,dir,trigger_failures,recheck_seconds}

Pure config parsing — no boto3 import, no I/O. Callable from tests
without AWS credentials. Wired into the metrics dispatchers in a later
commit alongside the boto3 client factory.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
CloudWatchClientFactory builds boto3 Logs / CloudWatch clients via the
default credential provider chain, honoring the optional datastore.profile
(boto3 profile_name) and datastore.role_arn (refreshable AssumeRole
session) config keys.

`probe_caller_identity()` calls sts:GetCallerIdentity at datastore startup
and logs "writing to account X as Y in region Z" so users notice
wrong-account / wrong-identity mistakes before any data ships. Surfaces
friendly errors for the three common failure modes (NoCredentialsError,
NoRegionError, ClientError).

Bumps `boto3>=1.34.0` in setup.py — PutLogEvents sequence-token deprecation
is fully clean by 1.34, and the CloudWatch metrics store relies on parallel
writes to a single log stream from multiple workers.

Closes task opensearch-project#21 (boto3 version pin).

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
emf.build_event(doc, namespace) transforms an OSB metric document of the
shape produced by MetricsStore.put_value_*_level into an Embedded Metric
Format (EMF) log event. The function is a pure transform — no I/O, no
boto3 — so it can be unit-tested in isolation and exercised from the
batcher in commit opensearch-project#6 without AWS-stack mocking.

EMF requires the metric name to be a top-level JSON key holding the
numeric value, so the function pivots OSB's {name: "service_time",
value: 12.3} pair into {"service_time": 12.3, ...} and references the
key from the _aws.CloudWatchMetrics block.

Dimensions are restricted to a fixed, low-cardinality set
(Workload, Task, OperationType, SampleType) and only the subset whose
values are present in the document are declared. Run-identity fields
(TestRunId, NodeName via meta.*, user tags) are emitted as plain
top-level fields — queryable via Logs Insights but not promoted to
CloudWatch Metrics dimensions, which keeps custom-metric cardinality
bounded. See design.md §"Dimension cardinality" for the cost rationale.

Telemetry payloads (MetricsStore.put_doc) deliver flattened {prefix_field:
value} dicts rather than the {name, value} pair handled here; their
multi-directive grouping is added in commit opensearch-project#9.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
`log_streams.LogStreamWriter` ships pre-serialized JSON log events to a
single (log_group, log_stream) pair via PutLogEvents, handling all the
boilerplate CloudWatch demands:

* Sorts each batch chronologically (CloudWatch rejects out-of-order).
* Chunks to respect the per-call limits (≤10,000 events, ≤1 MiB payload,
  ≤1 MiB per event including the 26-byte per-event overhead).
* Drops single events that exceed the per-event byte limit rather than
  failing the surrounding batch (logged at WARNING).
* Retries ThrottlingException / ServiceUnavailableException with bounded
  exponential backoff + jitter so multiple worker processes don't
  synchronise their retries.
* Retries ExpiredTokenException / InvalidSignatureException once to
  absorb credential-rotation races (boto3 owns the refresh) before
  bubbling up for the upcoming disk-spool path to handle.
* Recreates the log stream once if CloudWatch returns
  ResourceNotFoundException (e.g. operator deleted it mid-run).

Sequence tokens are intentionally NOT tracked — they were deprecated in
2023 and boto3 >= 1.34 accepts PutLogEvents without one.

Also exposes idempotent provisioning helpers `ensure_log_group` (with
optional retention) and `ensure_log_stream` so the metrics store can
create both lazily on first write.

Caller-supplied events use the canonical CloudWatch shape
`{"timestamp": <epoch_ms>, "message": <json_str>}`; the EMF transform in
emf.build_event produces dicts that the caller will json.dumps into that
message field in commit opensearch-project#7.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
Creates ``osbenchmark/metrics_stores/opensearch/`` and
``osbenchmark/metrics_stores/in_memory/`` as siblings of the existing
``cloudwatch/`` subpackage. Both new ``__init__.py`` files are shims that
re-export the classes currently defined in ``osbenchmark.metrics``
(OsMetricsStore, OsTestRunStore, OsResultsStore, InMemoryMetricsStore,
NoopResultsStore, plus the OS client / template helpers).

Resolves the per-backend layout asymmetry called out by the OSB
Conventions reviewer on commit opensearch-project#1: the new ``metrics_stores/`` parent now
contains one subpackage per built-in backend, matching the established
``osbenchmark/database/clients/<engine>/`` pattern.

A future refactor can move the source-of-truth definitions from
``osbenchmark/metrics.py`` into these subpackages without touching
callers. Until then, this is a zero-behavior-change reorg.

Closes task opensearch-project#19.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
Wires the EMF transform (emf.build_event) and the PutLogEvents writer
(log_streams.LogStreamWriter) into a concrete MetricsStore subclass and
registers it under datastore.type=cloudwatch.

Behavior:
* open() probes sts:GetCallerIdentity (logging account / role / region),
  provisions the metrics log group (with optional retention), and creates
  a per-worker log stream named "<workload>/<test-run-id>/<pid>" so
  parallel workers ship without coordination.
* _add(doc) transforms each OSB metric document through emf.build_event,
  JSON-serialises it, and buffers as {timestamp, message}. Non-numeric
  values are dropped with a warning (matching emf.build_event's contract).
* Auto-flushes when the buffer reaches CloudWatch's per-call caps
  (10,000 events or ~1 MiB payload) so a single flush always fits a
  single PutLogEvents call.
* close() / to_externalizable() force a flush to avoid losing buffered
  data across shutdown / actor-boundary paths.

Read methods (get_one, get_stats, get_percentiles, get_error_rate) raise
NotImplementedError; the Logs Insights wiring lands in commit opensearch-project#11. The
TestRunStore + ResultsStore wiring lands in commit opensearch-project#8 — until then,
datastore.type=cloudwatch falls back to FileTestRunStore + NoopResultsStore,
matching the InMemoryMetricsStore defaults.

Registration is deferred behind a function (lazy boto3 import) so users
who don't opt into the CloudWatch backend never pay the import cost.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
Replaces the placeholder TestRunStore + ResultsStore that
datastore.type=cloudwatch fell back to in commit opensearch-project#7 (FileTestRunStore +
NoopResultsStore) with real CloudWatch-backed implementations.

CloudWatchTestRunStore.store_test_run serialises the test-run document
via TestRun.as_dict() and ships it as a single PutLogEvents call to a
shared "test-runs" stream under the configured test-runs log group
(default benchmark-test-runs).

CloudWatchResultsStore.store_results explodes
TestRun.to_result_dicts() into one log event per result record and ships
them as a single PutLogEvents batch to a shared "results" stream under
the configured results log group (default benchmark-results). Writes
are low-frequency (once per benchmark) so no batching beyond what the
LogStreamWriter already provides is required.

Both stores defer log-group/stream provisioning until the first actual
write so that read-only paths (`osbenchmark list test-runs`, `compare`)
don't need write permissions or pay the AWS round trip.

Read paths (`list`, `find_by_test_run_id`) return safe empty / NotFound
values for now; Logs Insights wiring lands in commit opensearch-project#12.

The dispatcher now hands datastore.type=cloudwatch a
CompositeTestRunStore(CloudWatchTestRunStore, FileTestRunStore) so
local file persistence is preserved alongside the CloudWatch ship,
matching the OpenSearch backend pattern.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
OSB telemetry devices (NodeStats, ShardStats, RecoveryStats, IndexStats,
SegmentStats, CcrStats, TransformStats) call MetricsStore.put_doc(doc)
with a flattened dict carrying many numeric fields keyed by
underscore-prefixed paths — e.g. {"indices_segments_count": 42,
"jvm_mem_heap_used_percent": 73, "os_mem_used_percent": 41, ...}.

The single-metric build_event from commit opensearch-project#5 cannot represent these
because EMF requires one Metric.Name per declaration and the docs lack
a singular name/value pair. Adds emf.build_telemetry_event which:

* Scans the doc for numeric fields (excluding bools and the well-known
  identity fields injected by put_doc) as metric candidates.
* Groups candidates by their first underscore-delimited prefix
  (indices_, jvm_, os_, thread_pool_, process_, breakers_, network_) and
  emits one CloudWatchMetrics directive per group.
* Splits any group exceeding EMF's 100-metrics-per-directive cap into
  additional directives within the same _aws.CloudWatchMetrics[] list of
  the same log event, so a single put_doc still corresponds to a single
  PutLogEvents record regardless of NodeStats payload width.
* Emits all the same identity/meta fields the single-metric path emits
  (Workload, Task, TestRunId, meta.*, etc.) so Logs Insights can filter
  across both event shapes uniformly.

CloudWatchMetricsStore._add now routes between build_event (when the doc
has a `value` field — single-metric path) and build_telemetry_event
(when it doesn't — the flattened-dict telemetry path).

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
E2E install on a clean Python 3.11 venv surfaced a real circular import
that earlier unit tests masked. When something other than
osbenchmark.metrics is the first import to pull in
osbenchmark.metrics_stores.cloudwatch.metrics_store, that module's top-
level `from osbenchmark.metrics import MetricsStore` triggers a full
load of osbenchmark.metrics — which then hit the eager
`from ...cloudwatch.metrics_store import CloudWatchMetricsStore` at the
bottom of the file, re-entering the still-partially-loaded module and
failing with "cannot import name 'CloudWatchMetricsStore' from
partially initialized module".

The previous try/except wrapped this failure silently, leaving
datastore.type=cloudwatch unavailable at runtime — exactly what would
break a user trying to use the new backend.

Fix: defer all cloudwatch imports to first-use callables. The registry
already accepts callables for test_run_store / results_store; this
commit extends metrics_store_class to also accept a 0-arg callable that
returns the class, so the import only happens when the user actually
configures datastore.type=cloudwatch. By then all modules have finished
loading and there's no cycle.

Verified by reproducing both import orders (metrics-first and
cloudwatch-first) end-to-end: both resolve to the same
CloudWatchMetricsStore class with no warnings.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
insights.run_query wraps boto3's logs.start_query + poll get_query_results
into a single synchronous call that returns rows as plain Python dicts.

Insights returns each row as a list of {"field": ..., "value": ...}
pairs where every value is a string. run_query flattens that to
{field: value}, drops the synthetic @ptr column, and leaves type
coercion to the caller (the metrics store knows which fields are
numeric). Includes a `to_float` best-effort helper for percentile /
stats results.

Terminal states are mapped:
* Complete -> return rows
* Failed / Cancelled / Timeout / Unknown -> InsightsQueryError
* Client-side poll timeout (default 120s, well under Insights's 60-min
  server cap) -> best-effort stop_query then InsightsQueryError

This helper is the foundation for the read-side wiring in the next
commit; the metrics store will use it to translate get_one /
get_stats / get_percentiles / get_error_rate calls into Insights
queries against the configured metrics log group.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
Replaces the empty stubs from commit opensearch-project#7 (which returned None/0.0/empty
dicts so calculate_results would run without crashing) with real
CloudWatch Logs Insights queries that match the OS metrics store's
semantics:

* get_one — `filter ... | sort <sort_key> <asc|desc> | limit 1`,
  returning the mapped value or None.
* get_stats — `stats min, max, avg, sum, count` returning the same
  {count, min, max, avg, sum} dict OS produces.
* get_percentiles — `stats pct(<metric>, N) as p_N` for each requested
  percentile, falling back to `max(<metric>)` for p100 (Insights's pct
  function rejects 100). Returns an OrderedDict sorted by percentile, or
  None when there are no samples — matching OsMetricsStore.
* get_error_rate — `stats count(*) by meta.success` then computes
  errors / (errors + successes) Python-side.
* _get — full-row fetch for the parent's `get` / `get_raw` accessors,
  reconstructing OSB's `{value, task, operation-type, sample-type,
  meta.*}` doc shape from Insights's flat row format.

Every query is scoped to the current TestRunId so reads never bleed
across runs. SampleType is lower-cased to match the EMF write path
(emf.build_event emits `doc["sample-type"]` which is already lower-case).
The time window spans the test-run timestamp ±60s for clock skew.

The dispatcher swap from FileBackedCompositeTestRunStore (file-backed
reads) to the standard CompositeTestRunStore (CloudWatch-backed reads)
will happen alongside the test-run listing implementation in commit opensearch-project#12,
so this commit only touches CloudWatchMetricsStore — the test-run /
results stores still read from the local file store for now.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
Replaces the stubs from commit opensearch-project#8 (return [] / raise NotFound) with
Logs Insights queries against the test-runs log group. Stored JSON
messages are deserialised back into TestRun objects via the same
TestRun.from_dict consumers already expect from OsTestRunStore.

* list() — filters by environment_name, sorts by @timestamp desc,
  honors `system.list.test_runs.max_results`. Default 90-day window
  covers anything still in CloudWatch under the typical 30-day
  retention while keeping the query cheap.
* find_by_test_run_id — tighter 7-day initial window for the common
  "find a recent run" case, then falls through to the full window on
  miss so older runs are still discoverable. Raises NotFound on full
  miss (matches OsTestRunStore semantics).

Both inputs are escaped to prevent backticks/quotes in env names or
test-run-ids from breaking out of the Insights query string literal.
Malformed log events (bad JSON or shape) are skipped with a warning so
one rogue entry can't crash a list call.

FileBackedCompositeTestRunStore now reads from both stores rather than
file-only:

* find_by_test_run_id consults file first (fresh writes are not yet
  visible to Logs Insights — CW Logs has a several-second ingest lag),
  falls back to CloudWatch on miss.
* list merges file + cloudwatch, deduped on test_run_id with file
  winning. Gracefully degrades to file-only on InsightsQueryError so
  CloudWatch outages or permission gaps don't break local CLI commands.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
35 new tests covering the four write-side modules:

* tests/metrics_stores/cloudwatch/conftest.py — shared fixtures: a
  hand-rolled boto3 Logs client double, a default CloudWatchConfig,
  and autouse sleep-disabling so retry/poll loops don't burn time.
* test_write_path.py
  - TestConfigLoad: defaults, overrides, retention enum (incl 1096
    regression), bool/int validation, minimums.
  - TestEmfBuildEvent: full doc pivot, no-dimension case emits
    [[]] not [], partial dimensions, non-numeric value drop
    (incl bool), pivot-wins-on-collision, missing-timestamp
    fallback, unit case insensitivity.
  - TestEmfBuildTelemetryEvent: flattened-doc multi-directive
    grouping, >100-metric overflow chunking, nested-dict
    log-only fallback (RecoveryStats shape), bool fields kept
    as log fields.
  - TestLogStreamWriter: chronological sort, count chunking,
    oversized-event drop with warning, throttle retry,
    auth-error retry-once, stream-recreate on RNF.
  - TestCloudWatchMetricsStoreWrite: open() provisioning + STS
    probe, read-only-open skips AWS, put_value buffer/flush,
    non-numeric drop, telemetry put_doc routing, flush re-buffers
    on failure, close() flushes + clears.
  - TestCloudWatchTestRunStoreWrite / ResultsStoreWrite: single-
    event store_test_run, store_results record explosion,
    empty-results doesn't provision the log group.

All 98 metrics-store tests pass (63 existing OS path + 35 new CW).

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
45 new tests covering the read-side modules:

* TestInsightsHelper — happy-path flatten, polling through Running /
  Scheduled, Unknown-is-transient (commit-10 regression), terminal
  failure statuses raise InsightsQueryError, start_query ClientError
  wrapped, get_query_results throttle retried, float epoch coerced
  to int, InsightsQueryError extends BenchmarkError.
* TestFlattenRows / TestToFloat — primitive helpers.
* TestMetricsStoreReads — get_stats / get_percentiles (incl
  pct(field, 100) → max(field) substitution and "no hits → None"),
  get_one returns mapped value, relative-time-ms coerced to float
  (commit-11 regression), get_unit via parent's _get
  (commit-11 regression), get_error_rate all three branches, get
  returns raw value list, TestRunId scope filter on every query,
  SampleType enum lower-cased, UTC timezone window
  (commit-11 regression), backtick/quote injection escaped.
* TestCloudWatchTestRunStoreReads — list round-trips JSON to
  TestRun, list skips malformed events, find_by_test_run_id round-
  trips and raises NotFound with the exact OS wording on miss.
* TestFileBackedComposite — find prefers file, falls back to CW;
  list merges + dedupes; graceful degradation on CW error
  (catches Exception, not just InsightsQueryError); writes fan out
  to both stores.

All 143 metrics-store tests pass (63 existing + 35 write + 45 read).

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
Single-page guide at docs/user-guides/cloudwatch-datastore.md covering:

* Quick start (3-line benchmark.ini) + expected console output.
* Full config-key reference table (regions, log groups, retention,
  profile/role, spool tunables).
* Minimum write-only + read-extended IAM policies.
* How it works: EMF per-sample, multi-directive telemetry grouping,
  plain-log test-run/results docs, Logs-Insights reads.
* Long-running benchmarks: prefer renewable creds (EC2 instance role)
  but trust the disk spool when SSO sessions expire.
* Troubleshooting matrix for the common failure modes.
* Limitations vs the OpenSearch backend (read latency, no per-run
  dimension, CW Unit enum fidelity).

Front matter uses the same Jekyll layout / OSB Use Cases parent /
nav_order as docs/user-guides/create-pipeline.md so the page slots
into the existing docs site.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
E2E run on a clean AWS account surfaced a real-world gap: the IAM role
had logs:PutLogEvents (write) but not logs:StartQuery (read), so the
orchestrator's post-run calculate_results path crashed when it tried to
compute percentiles via Insights.

Fix: CloudWatchMetricsStore._run_insights now catches
InsightsQueryError and returns [], degrading each read method to the
same empty/zero values the commit-7 stubs returned. The benchmark
itself ran successfully (data is in CW Logs / Metrics) — only the
post-run percentile / stats reports are blank when the role lacks
read permissions.

This matches the fail-soft contract FileBackedCompositeTestRunStore
established in commit opensearch-project#12 for list / find. The user-facing
cloudwatch-datastore docs already split the IAM policy into "minimum
write-only" and "additional for read operations" — this commit makes
the runtime behavior match the docs: write-only IAM works, just with
empty post-run reports.

Also bumps a flaky-on-slow-CI spool test's recovery-thread wait from
5s to 15s.

179 tests pass.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
Without StorageResolution, CloudWatch buckets every EMF-extracted metric
into standard-resolution 60s bins. For short benchmark windows that means
the console shows a single dot instead of a line. Setting
StorageResolution=1 publishes high-resolution metrics, unlocking 1s / 5s
/ 10s / 30s period choices in the Metrics console so a sub-minute test
window can be charted as a real line.

Applies to both the single-metric (build_event) and telemetry
(build_telemetry_event) paths.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
OpenSearch NodeStats produces flattened field names like
`jvm_gc_collectors_G1 Concurrent GC_collection_count` and
`jvm_buffer_pools_mapped - 'non-volatile memory'_count`. Spaces and
apostrophes aren't in CloudWatch's allowed set for metric names
([A-Za-z0-9_.-]), and CW silently rejects an entire MetricDirective if
any Name in it is invalid — so a single bad name in the jvm_* group
prevents extraction of every metric grouped alongside it.

Sanitize both the directive Name and the top-level key that the
directive references so they still match, letting CloudWatch extract
the metric successfully.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
CloudWatch's EMF extractor enforces an undocumented per-log-event cap on
the total number of distinct metric definitions across ALL directives
(~100). NodeStats produces ~600 numeric fields — packed into a single
log event via multiple ≤100-metric directives, the extractor silently
drops every metric even though the log line itself lands correctly and
the event is otherwise spec-valid.

Verified empirically against a personal AWS account: 60 metrics in 12
directives → 60/60 extracted; 596 metrics in 12 directives → 0/596
extracted. The public spec only documents the 100/directive limit, but
CW enforces both.

Split telemetry documents into multiple log events (identical
identity/dimension fields, same Timestamp) each under the per-event cap.
Every event carries only the top-level numeric fields its own
directives reference, so we don't ship the full 600-field payload N
times.

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
Per RFC feedback (issue opensearch-project#1083, open question §6), the disk-spool +
cw-replay durability path was determined to be out of scope for the
initial CloudWatch datastore contribution. The scenario it addresses
(long laptop-SSO runs where AWS credentials expire mid-benchmark) is
narrow, adds ~40% of the surface area, and the OpenSearch backend has
no equivalent. If users request it later, it can be revisited as a
follow-up.

Removed:
- osbenchmark/metrics_stores/cloudwatch/spool.py
- datastore.cloudwatch.spool.* config keys
- Spool wiring in LogStreamWriter, CloudWatchMetricsStore, config
- Spool test fixtures and user-guide sections

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
@OVI3D0
OVI3D0 force-pushed the feat/cloudwatch-datastore-clean branch from 425ecc8 to c525a42 Compare July 15, 2026 20:30
Fixes ~30 lint issues surfaced by the CI job that were not caught locally
because the dev env runs a newer pylint. All pure lint fixes, no behavior
changes.

- osbenchmark/metrics.py:
  - W0108 unnecessary-lambda: use bare OsResultsStore constructor. The
    default's FileTestRunStore lambda is kept (with pragma) because the
    class is defined later in the file, so a forward reference would fail
    at import.
  - C0415 import-outside-toplevel: pragma on the deferred cloudwatch
    imports (deferral is intentional — imports at module top trigger a
    circular between metrics.py and the cloudwatch submodules).

- osbenchmark/metrics_stores/cloudwatch/metrics_store.py:
  - R1711 useless-return at end of to_externalizable.
  - W1309 pointless f-string in Insights query builder.

- osbenchmark/metrics_stores/cloudwatch/client.py:
  - C0415 pragma on deferred botocore.credentials imports (deferred to
    keep the assume-role branch cost-free when unused).

- tests/metrics_stores/cloudwatch/conftest.py:
  - E0702 raising-bad-type: bind fail_with to a local so type-narrowing
    is preserved.
  - Move botocore.exceptions import to the top of the module.

- tests/metrics_stores/cloudwatch/test_read_path.py,
  tests/metrics_stores/cloudwatch/test_write_path.py:
  - C0321 multi-statement lines expanded across ~10 sites in fake
    classes and one-line def bodies.
  - C0415/C0411 in-function conftest imports moved to module top and
    consolidated in first-party-then-local order.
  - W0611 unused imports removed.
  - test_write_path.py adds a module-level `# pylint: disable=protected-
    access` because the tests deliberately assert on `_buffered_events`
    and `_client_factory` to verify internal state (test-only, expected).

Signed-off-by: Michael Oviedo <mikeovi@amazon.com>
@OVI3D0
OVI3D0 force-pushed the feat/cloudwatch-datastore-clean branch from c525a42 to bc40375 Compare July 15, 2026 20:36
@OVI3D0 OVI3D0 added the skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis. label Jul 15, 2026
@OVI3D0
OVI3D0 marked this pull request as ready for review July 15, 2026 21:07
@OVI3D0
OVI3D0 requested review from IanHoang and gkamat as code owners July 15, 2026 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant