add ecs task payload #281

kangyili · 2024-01-05T14:17:11Z

What does this PR do?

Agent PR DataDog/datadog-agent#21836

Motivation

Additional Notes

Possible Drawbacks / Trade-offs

Describe how to test/QA your changes

Reviewer's Checklist

Reviewers: please see the review guidelines.

fisherevans · 2024-01-23T18:21:20Z

nit: your code uses tabs, but the rest of the file uses spaces for indentation.

xlucas · 2024-02-08T15:41:06Z

proto/process/agent.proto

@@ -669,6 +755,7 @@ enum K8sResource {
    CR = 21;
    VERTICALPODAUTOSCALER = 22;
    HORIZONTALPODAUTOSCALER = 23;
+    ECSTASK = 24;


That looks odd to add an ECS resource to a K8sResource enum, we could rename it to OrchestratorResource and start at a different offset for ECS tasks so we have room for new Kubernetes resources.

agent-payload/proto/process/agent.proto

Lines 742 to 769 in 7f5124f

enum OrchestratorResource {

OrchestratorResource_UNSPECIFIED = 0;

POD = 1;

REPLICASET = 2;

SERVICE = 3;

NODE = 4;

CLUSTER = 5;

JOB = 6;

CRONJOB = 7;

DAEMONSET = 8;

STATEFULSET = 9;

PERSISTENTVOLUME = 10;

PERSISTENTVOLUMECLAIM = 11;

ROLE = 12;

ROLEBINDING = 13;

CLUSTERROLE = 14;

CLUSTERROLEBINDING = 15;

SERVICEACCOUNT = 16;

INGRESS = 17;

DEPLOYMENT = 18;

NAMESPACE = 19;

CRD = 20;

CR = 21;

VERTICALPODAUTOSCALER = 22;

HORIZONTALPODAUTOSCALER = 23;

NETWORKPOLICY = 24;

ECSTASK = 150;

}

go.mod

aureleoules

Looks good to me

aureleoules · 2024-03-18T12:55:05Z

proto/process/agent.proto

+	string family = 6;
+	string version = 7;


nit: Maybe specify platformVersion instead of version as it may be confusing since taskVersion exists. Same for family -> platformFamily ?

I'll delete taskVersion. family and version here stand for task definition family and version

My bad , actually taskVersion is a custom field that is used to track changes. I'll rename it to resource version

aureleoules · 2024-03-18T12:59:47Z

proto/process/agent.proto

+	string family = 6;
+	string version = 7;
+	string availabilityZone = 8;
+	map<string, double> limits = 9;


Is this field supposed to contain both memory and cpu limits like this?
{"cpu": 8, "memory": 4}

Yes. I feel I still prefer to use a map instead of using two dedicated fields as we can check if the value is set or not

if val, ok := limits["CPU"]; ok { //do somthing }

maybe we should define some constants for the keys?

we can't use enum type as key for a map in proto 😞
It's a usage we are already using https://github.com/DataDog/agent-payload/blob/master/proto/process/agent.proto#L996

It seems it's impossible to define constant strings in protobuf3. And if we were to split the limits field into 2 (memLimits, cpuLimits) it's not possible to check if the field has a value or not since numeric types are initialized with 0 in golang.
So leaving this as is is the best option I think

kangyili · 2024-03-18T13:51:28Z

proto/process/agent.proto

@@ -681,6 +767,7 @@ enum K8sResource {
    VERTICALPODAUTOSCALER = 22;
    HORIZONTALPODAUTOSCALER = 23;
    NETWORKPOLICY = 24;
+    ECSTASK = 150;


This change matches https://github.com/DataDog/datadog-agent/blob/076db95494f6cd8ee8d53fb469f44fb40b923c46/pkg/orchestrator/model/types.go#L83

kangyili requested review from a team as code owners January 5, 2024 14:17

kangyili mentioned this pull request Jan 5, 2024

Update Workloadmeta ECS collector to use metadata v4 endpoint DataDog/datadog-agent#21836

Merged

10 tasks

kangyili mentioned this pull request Feb 1, 2024

[Orchestrator] add new check to collect ecs tasks DataDog/datadog-agent#22060

Merged

10 tasks

xlucas reviewed Feb 8, 2024

View reviewed changes

jeremy-hanna requested changes Feb 8, 2024

View reviewed changes

go.mod Outdated Show resolved Hide resolved

jeremy-hanna approved these changes Feb 9, 2024

View reviewed changes

kangyili force-pushed the kangyi/ecs-task branch 5 times, most recently from 88d488e to 7f5124f Compare February 28, 2024 14:07

add ecs task payload

7d4963c

kangyili force-pushed the kangyi/ecs-task branch from 7f5124f to 7d4963c Compare March 18, 2024 10:26

kangyili removed request for a team March 18, 2024 10:37

aureleoules reviewed Mar 18, 2024

View reviewed changes

kangyili commented Mar 18, 2024

View reviewed changes

add resourceVersion

eed77af

kangyili force-pushed the kangyi/ecs-task branch from 7f472b8 to eed77af Compare March 18, 2024 16:08

kangyili merged commit 107f466 into master Mar 19, 2024
7 checks passed

kangyili mentioned this pull request Mar 20, 2024

add TypeCollectorECSTask #296

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add ecs task payload #281

add ecs task payload #281

kangyili commented Jan 5, 2024 •

edited

Loading

fisherevans commented Jan 23, 2024

xlucas Feb 8, 2024 •

edited

Loading

kangyili Feb 28, 2024 •

edited

Loading

aureleoules left a comment

aureleoules Mar 18, 2024

kangyili Mar 18, 2024

kangyili Mar 18, 2024 •

edited

Loading

aureleoules Mar 18, 2024

kangyili Mar 18, 2024

fisherevans Mar 18, 2024

kangyili Mar 18, 2024

aureleoules Mar 18, 2024 •

edited

Loading

kangyili Mar 18, 2024 •

edited

Loading

	enum OrchestratorResource {
	OrchestratorResource_UNSPECIFIED = 0;
	POD = 1;
	REPLICASET = 2;
	SERVICE = 3;
	NODE = 4;
	CLUSTER = 5;
	JOB = 6;
	CRONJOB = 7;
	DAEMONSET = 8;
	STATEFULSET = 9;
	PERSISTENTVOLUME = 10;
	PERSISTENTVOLUMECLAIM = 11;
	ROLE = 12;
	ROLEBINDING = 13;
	CLUSTERROLE = 14;
	CLUSTERROLEBINDING = 15;
	SERVICEACCOUNT = 16;
	INGRESS = 17;
	DEPLOYMENT = 18;
	NAMESPACE = 19;
	CRD = 20;
	CR = 21;
	VERTICALPODAUTOSCALER = 22;
	HORIZONTALPODAUTOSCALER = 23;
	NETWORKPOLICY = 24;
	ECSTASK = 150;
	}

add ecs task payload #281

add ecs task payload #281

Conversation

kangyili commented Jan 5, 2024 • edited Loading

What does this PR do?

Motivation

Additional Notes

Possible Drawbacks / Trade-offs

Describe how to test/QA your changes

Reviewer's Checklist

fisherevans commented Jan 23, 2024

xlucas Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

kangyili Feb 28, 2024 • edited Loading

Choose a reason for hiding this comment

aureleoules left a comment

Choose a reason for hiding this comment

aureleoules Mar 18, 2024

Choose a reason for hiding this comment

kangyili Mar 18, 2024

Choose a reason for hiding this comment

kangyili Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

aureleoules Mar 18, 2024

Choose a reason for hiding this comment

kangyili Mar 18, 2024

Choose a reason for hiding this comment

fisherevans Mar 18, 2024

Choose a reason for hiding this comment

kangyili Mar 18, 2024

Choose a reason for hiding this comment

aureleoules Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

kangyili Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

kangyili commented Jan 5, 2024 •

edited

Loading

xlucas Feb 8, 2024 •

edited

Loading

kangyili Feb 28, 2024 •

edited

Loading

kangyili Mar 18, 2024 •

edited

Loading

aureleoules Mar 18, 2024 •

edited

Loading

kangyili Mar 18, 2024 •

edited

Loading