Skip to content

Commit f0987e3

Browse files
lucia-sbclaudeburaizu
authored
Fix Kueue metric metadata and add log collection (DataDog#24700)
* Fix Kueue metric metadata and add log collection - Correct metadata.csv for counter/histogram metrics: they are submitted as .count/.bucket/.sum (monotonic_count), not base-name gauges, so dashboards querying the base names showed no data. Corrects ~30 entries. - Rebuild tests/fixtures/metrics.txt to the real endpoint format for the affected families so unit tests reflect what Kueue actually exposes. - Strengthen tests so this can't regress: test_check runs the check twice so monotonic counters flush and the strict metadata assertion validates them; test_e2e uses rate=True so the e2e metadata check validates counters too. - Add Kueue log collection support (logs template in spec.yaml, regenerated conf.yaml.example, README section). - Expand the E2E environment to exercise cohort/fair-sharing, GPU, a custom resource, a not-active ClusterQueue, and preemption/eviction metrics. - Make the E2E env robust locally: choose non-colliding kind subnets and disable Kueue's visibility server, whose cert bootstrap crashloops some clusters. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Add changelog entries for DataDog#24700 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Remove non-existent metric and fix preemption_skips type in Kueue metadata Verified every metric in metadata.csv against Kueue v0.18 source (pkg/metrics/metrics.go) and the live endpoint: - kueue.resource_flavor.quota_reserved_workloads does not exist in Kueue v0.18 (no resource_flavor metric is defined). Removed it from the metadata, metric map, test fixture, and expected-metrics list. - kueue.admission_cycle.preemption_skips is a Gauge, not a counter. Reverted it to the base gauge name. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Collect kueue.finished_workloads.count via native_dynamic kueue_finished_workloads is a dual-form metric: Kueue exposes both a gauge (kueue_finished_workloads) and a counter (kueue_finished_workloads_total). The standard metric map collapsed the counter onto the gauge's name, so the counter was submitted as a gauge — polluting kueue.finished_workloads and never emitting a .count. Map it as native_dynamic (the same mechanism used for go.memstats.alloc_bytes and the local_queue transformer) so the gauge and the cumulative .count are both submitted correctly. Added the kueue.finished_workloads.count metadata row and fixture coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Restore original description quoting in Kueue metadata.csv 41 rows carried over from master differed only by having the surrounding double quotes stripped from the description column, with no change to the description text. Restoring the original quoting shrinks the diff for this file from 135 lines to 53, leaving only the substantive changes: the 22 base-name gauge rows replaced by 31 .count/.bucket/.sum count rows and the resource_flavor.quota_reserved_workloads removal. Parsing the file before and after with csv.reader yields identical rows, so this is a formatting-only change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Drop dead local_queue finished_workloads mapping and cover its counter The parser strips '_total' from counter families, so the kueue_local_queue_finished_workloads_total key was never looked up. Both forms already resolve correctly because the local-queue custom transformer wraps get_native_dynamic_transformer; add the resulting .count metric to the fixture assertions so that stays covered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Request a 24h metrics-reader token so --dev sessions survive Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Document why both Kueue cert-bootstrap mitigations are needed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Correct the stale reason for excluding counters from e2e assertions Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Make the fixture histogram families well-formed The four families converted from gauges to histograms had no le="+Inf" bucket and had bucket lines on only the first series, a shape no real endpoint produces. Emit the full bucket set plus +Inf for every series so the +Inf submission path is exercised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Harden kind subnet selection and clean up its temp file Derive the prefix from the octet count so macOS netstat's abbreviated network routes (10 meaning 10.0.0.0/8) are not narrowed to /32 and missed. Log the chosen pair, including the fallback, since the subnets are the first thing to check when the controller crashloops. Turn the builder into a context manager so the rendered config is unlinked on teardown. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Select the Kueue manager container by name when patching feature gates Patching containers/0 blind succeeds even if the flag lands on a sidecar, turning an upstream manifest change into a crashloop with an unrelated error. Resolve the index by container name and fail loudly if it is absent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Right-size the preemption scenario to 200m of quota Both preempt Jobs asked for a full core against a 1-core quota, so the admitted pod oversubscribed a 2-vCPU runner on top of kube-system and the existing workloads. Preemption is quota accounting, so the metrics are identical at a fifth of the resources and no pods are left Pending. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Derive the e2e env coordinates instead of hardcoding them The ddev config path and kind cluster name both encoded py3.13-v0.18.0 as a literal, so bumping python or KUEUE_VERSION broke the workload-events test with FileNotFoundError. Build both from get_active_env(). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Tighten remaining Kueue env hygiene Side-load the workload image once instead of pulling it from Docker Hub for every Job pod, explain why min_collection_interval dropped to 30, narrow the enabled integration frameworks to the one the tests exercise, and drive the workload-events check through run() so the test stops calling a private initialization directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Fix preempted_by tagging and rework the Kueue e2e test env Only derive `kueue_preempted_by` from evictions whose reason is `Preempted`. A `FlavorMigration` eviction carries a lookalike message ("Evicted to accommodate a workload (UID: ...)") that was being mined for a preemptor UID that does not exist. Also correct the log collection section of the README, extract the kubectl helpers into tests/kube.py, and extend the e2e coverage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Remove stale comment on the local queue finished_workloads mapping Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Remove comment on the preempting workload UID pattern Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Remove comment on the preemption reason gate Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Remove explanatory comments from the Kueue test metric lists Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Remove explanatory comments from the Kueue e2e tests Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Remove explanatory comments from the Kueue unit tests Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Use the built-in tag assertion for the e2e metric tags `assert_metric_has_tags` covers every metric in `EXPECTED_METRIC_TAGS`. The custom helper is now reserved for the status families, whose tags carry no information on their own: a series exists for every state of every queue, and only the value says which state is current. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Derive the Kueue version and workload images instead of duplicating them The version literal lived in both hatch.toml and a conftest fallback, and the workload image lived in a constant kept in sync with the Job manifests by a comment. Read both from their single source instead, so a bump has one place to change and a manifest using a different image is still preloaded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Drop the custom series assertion in favour of assert_metric The status families needed a value paired with tags, which the custom helper provided by matching a tag subset. `assert_metric` covers it by matching the exact tag set, with the per-run endpoint tag taken from the saved instance config, so the tests now use only built-in assertions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Remove explanatory comments from the Kueue test constants Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Gate the Kueue workload-events e2e test through the dd_agent_check fixture Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Remove explanatory comments from the Kueue test conftest Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Simplify kind subnet selection in the Kueue test conftest Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Let the Kueue Workload discovery loop retry instead of raising on an empty result Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Restore Kueue metric-surface changelog coverage and harden the test env Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Clarify the two kueue_preempted_by changelog entries Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Make kind image side-loading best effort in the Kueue test env kind load docker-image fails against node images whose containerd config version the installed kind does not support. The Job pods pull the image themselves in that case, so a failed side-load must not abort env start. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Tolerate a missing cluster when minting the Kueue metrics reader token The dd_environment body re-runs on the tear-down pass, where set_up_env is false so no cluster is available. A strict token mint aborted teardown. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Update kueue/README.md Co-authored-by: Bryce Eadie <bryce.eadie@datadoghq.com> * Update kueue/README.md Co-authored-by: Bryce Eadie <bryce.eadie@datadoghq.com> * Update kueue/README.md Co-authored-by: Bryce Eadie <bryce.eadie@datadoghq.com> * Reconcile the metric tag helper rename after merging master PR DataDog#24760 added a third `_get_metric_tags` call site on master while this branch renamed the helper to `get_metric_tags`. The two changes touched different regions, so the merge was textually clean but left the new call site referencing the old name, failing lint with F821. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Bryce Eadie <bryce.eadie@datadoghq.com>
1 parent 12f1439 commit f0987e3

23 files changed

Lines changed: 1258 additions & 505 deletions

kueue/README.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,24 @@ Kueue is a cluster-level service. Configure this integration as a Cluster Agent
5454

5555
See the [sample kueue.d/conf.yaml][4] for all available configuration options.
5656

57+
### Log collection
58+
59+
The Kueue controller manager writes logs to its container output, which Kubernetes captures as container logs. Collecting logs is disabled by default in the Datadog Agent. To enable it, see [Kubernetes Log Collection][12]. Logs are collected by the node Agent running on the node that hosts the Kueue controller manager, not by the Cluster Agent that runs this cluster check.
60+
61+
After log collection has been enabled, set the Kueue log configuration as an Autodiscovery annotation on the controller manager's pod template. This allows it to persist despite pod restarts. Add it under `spec.template.metadata.annotations` of the `kueue-controller-manager` deployment, or set `controllerManager.manager.podAnnotations` if you install Kueue with the Helm chart:
62+
63+
```yaml
64+
ad.datadoghq.com/manager.logs: |
65+
[
66+
{
67+
"source": "kueue",
68+
"service": "<SERVICE>"
69+
}
70+
]
71+
```
72+
73+
This annotation targets the container named `manager`, which is the container name used by both the Kueue release manifests and the Helm chart. Replace `manager` with the name (`.spec.containers[i].name`) of your Kueue container if you use a different name.
74+
5775
### Validation
5876

5977
[Run the Cluster Agent's `clusterchecks` subcommand][11] and look for `kueue` under the Checks section.
@@ -96,3 +114,4 @@ Need help? Contact [Datadog support][8].
96114
[8]: https://docs.datadoghq.com/help/
97115
[10]: https://docs.datadoghq.com/containers/cluster_agent/clusterchecks/?tab=helm#configuration-from-configuration-files
98116
[11]: https://docs.datadoghq.com/containers/troubleshooting/cluster-and-endpoint-checks/#dispatching-logic-in-the-cluster-agent
117+
[12]: https://docs.datadoghq.com/containers/kubernetes/log/

kueue/assets/configuration/spec.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,3 +61,8 @@ files:
6161
type: object
6262
example:
6363
example.com/fpga: fpga
64+
- template: logs
65+
example:
66+
- type: docker
67+
source: kueue
68+
service: <SERVICE>

kueue/changelog.d/24700.added

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Add support for Kueue log collection.
2+
Collect the cumulative ``kueue.finished_workloads.count`` counter.

kueue/changelog.d/24700.fixed

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Correct metric metadata so counters, histograms, and gauges resolve in dashboards, and remove a metric that Kueue does not emit.
2+
Fix the ``kueue_preempted_by`` workload event tag value, which appended the preempting Job UID to the Workload UID.
3+
Stop emitting the ``kueue_preempted_by`` workload event tag on evictions caused by resource flavor migration rather than preemption.

kueue/datadog_checks/kueue/check.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
RESOURCE_METRIC_PATTERN = '^(' + '|'.join(re.escape(k) for k in RESOURCE_METRIC_MAP) + ')$'
1717
LOCAL_QUEUE_METRIC_PATTERN = '^(' + '|'.join(re.escape(k) for k in LOCAL_QUEUE_METRIC_MAP) + ')$'
18-
PREEMPTING_WORKLOAD_UID_PATTERN = re.compile(r'\bworkload \(UID: ([^)]+)\)', re.IGNORECASE)
18+
PREEMPTING_WORKLOAD_UID_PATTERN = re.compile(r'\bworkload \(UID: ([^),\s]+)', re.IGNORECASE)
1919

2020
RESOURCE_NAME_MAP = {
2121
'cpu': 'cpu',
@@ -366,11 +366,12 @@ def workload_event_tags(
366366
if transition == 'evicted' and condition:
367367
if reason := condition.get('reason'):
368368
tags.append(f'kueue_eviction_reason:{reason}')
369-
preempted_condition = self.get_condition(workload, 'Preempted')
370-
if reason == 'Preempted' and preempted_condition and preempted_condition.get('reason'):
371-
tags.append(f'kueue_preemption_reason:{preempted_condition["reason"]}')
372-
if preempted_by := self.preempting_workload_uid(condition, preempted_condition):
373-
tags.append(f'kueue_preempted_by:{preempted_by}')
369+
if reason == 'Preempted':
370+
preempted_condition = self.get_condition(workload, 'Preempted')
371+
if preempted_condition and preempted_condition.get('reason'):
372+
tags.append(f'kueue_preemption_reason:{preempted_condition["reason"]}')
373+
if preempted_by := self.preempting_workload_uid(condition, preempted_condition):
374+
tags.append(f'kueue_preempted_by:{preempted_by}')
374375

375376
return tags
376377

kueue/datadog_checks/kueue/data/conf.yaml.example

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -680,3 +680,23 @@ instances:
680680
#
681681
# resource_name_map:
682682
# example.com/fpga: fpga
683+
684+
## Log Section
685+
##
686+
## type - required - Type of log input source (tcp / udp / file / windows_event).
687+
## port / path / channel_path - required - Set port if type is tcp or udp.
688+
## Set path if type is file.
689+
## Set channel_path if type is windows_event.
690+
## source - required - Attribute that defines which integration sent the logs.
691+
## encoding - optional - For file specifies the file encoding. Default is utf-8. Other
692+
## possible values are utf-16-le and utf-16-be.
693+
## service - optional - The name of the service that generates the log.
694+
## Overrides any `service` defined in the `init_config` section.
695+
## tags - optional - Add tags to the collected logs.
696+
##
697+
## Discover Datadog log collection: https://docs.datadoghq.com/logs/log_collection/
698+
#
699+
# logs:
700+
# - type: docker
701+
# source: kueue
702+
# service: <SERVICE>

kueue/datadog_checks/kueue/metrics.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
'kueue_local_queue_admitted_workloads': 'local_queue.admitted.workloads',
2727
'kueue_local_queue_evicted_workloads': 'local_queue.evicted_workloads',
2828
'kueue_local_queue_finished_workloads': 'local_queue.finished_workloads',
29-
'kueue_local_queue_finished_workloads_total': 'local_queue.finished_workloads',
3029
'kueue_local_queue_pending_workloads': 'local_queue.pending_workloads',
3130
'kueue_local_queue_quota_reserved_wait_time_seconds': 'local_queue.quota_reserved.wait_time.seconds',
3231
'kueue_local_queue_quota_reserved_workloads': 'local_queue.quota_reserved.workloads',
@@ -99,16 +98,14 @@
9998
'kueue_cohort_weighted_share': 'cohort.weighted_share',
10099
'kueue_evicted_workloads': 'evicted_workloads',
101100
'kueue_evicted_workloads_once': 'evicted_workloads_once',
102-
'kueue_finished_workloads': 'finished_workloads',
103-
'kueue_finished_workloads_total': 'finished_workloads',
101+
'kueue_finished_workloads': {'name': 'finished_workloads', 'type': 'native_dynamic'},
104102
'kueue_pending_workloads': 'pending_workloads',
105103
'kueue_pods_ready_to_evicted_time_seconds': 'pods_ready_to_evicted_time.seconds',
106104
'kueue_preempted_workloads': 'preempted_workloads',
107105
'kueue_quota_reserved_wait_time_seconds': 'quota_reserved.wait_time.seconds',
108106
'kueue_quota_reserved_workloads': 'quota_reserved.workloads',
109107
'kueue_ready_wait_time_seconds': 'ready_wait_time.seconds',
110108
'kueue_replaced_workload_slices': 'replaced_workload_slices',
111-
'kueue_resource_flavor_quota_reserved_workloads': 'resource_flavor.quota_reserved_workloads',
112109
'kueue_reserving_active_workloads': 'reserving.active_workloads',
113110
'kueue_workload_creation_latency_seconds': 'workload.creation_latency.seconds',
114111
'kueue_workload_eviction_latency_seconds': 'workload.eviction_latency.seconds',

0 commit comments

Comments
 (0)