[AGENTONB-2760] use UUID instead of HostName in metadata payload

[swang392]

What does this PR do?

• sobotka requires either UUID or HostName present to send metadata payloads through it to REDAPL.
• This PR switches from hostName to UUID. This makes more sense because metadata is not sent per host, rather it is sent once per cluster.
• UUID is set to clusterUID

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

Minimum Agent Versions

Are there minimum versions of the Datadog Agent and/or Cluster Agent required?

• Agent: vX.Y.Z
• Cluster Agent: vX.Y.Z

Describe your test plan

• Deploy the operator
• Verify that metadata forwarders are starting and sending metadata as expected

Tested on a staging cluster (gizmo), and verified that the payload is being successfully sent -- the modification_detected_at field in DDSQL has been updated after the test image was deployed.

Checklist

• PR has at least one valid label: bug, enhancement, refactoring, documentation, tooling, and/or dependencies
• PR has a milestone or the qa/skip-qa label
• All commits are signed (see: signing commits)

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 37.30%. Comparing base (c16f392) to head (3e4fb8c).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2468   +/-   ##
=======================================
  Coverage   37.30%   37.30%           
=======================================
  Files         290      290           
  Lines       24699    24686   -13     
=======================================
- Hits         9213     9209    -4     
+ Misses      14773    14764    -9     
  Partials      713      713           

Flag	Coverage Δ
unittests	37.30% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
pkg/controller/utils/metadata/crd_metadata.go	36.08% <ø> (+0.54%)	⬆️
pkg/controller/utils/metadata/helm_metadata.go	31.89% <100.00%> (+0.40%)	⬆️
pkg/controller/utils/metadata/operator_metadata.go	14.73% <100.00%> (+0.45%)	⬆️
pkg/controller/utils/metadata/shared_metadata.go	85.93% <ø> (-0.83%)	⬇️

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c16f392...3e4fb8c. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[levan-m]

Does this require schema or writer change?

[swang392]

No, the dd-go backend creates a schema resource based on the HelmMetadataPayload object HelmMetadata (datadog_operator_helm_metadata) and the schema matches that struct.

[tbavelier]

We have payload.clusterUID, payload.UUID and payload.metadata.clusterUID for the same value, can we possibly remove clusterID from OperatorMetadata, CRDMetadata and HelmMetadata structs ? Also, possibly remove it from payload if it's not necessary in the schema at intake to only keep UUID ? (since it can be derived from UUID ?)

More generally (and outside the PR scope), we repeat a lot of OperatorVersion and KubernetesVersion keys in the structs, would it be possible to reduce duplication by using something like:

// CommonMetadata contains metadata fields shared across all metadata payload types.
// It is embedded anonymously in payload-specific metadata structs so the fields are
// flattened in JSON (e.g., "operator_version", "kubernetes_version").
type CommonMetadata struct {
	OperatorVersion   string `json:"operator_version"`
	KubernetesVersion string `json:"kubernetes_version"`
}

type OperatorMetadata struct {
	// Shared
	CommonMetadata

	InstallMethodTool             string         `json:"install_method_tool"`
[...]

type HelmMetadata struct {
	// Shared
	CommonMetadata

	ChartName                 string `json:"chart_name"`

Or differently since they're also present in sharedmetadata

[tbavelier]

Not directly related to this PR, but we have a potential data race in func (omf *OperatorMetadataForwarder) GetPayload(clusterUID string).
In OperatorMetadataForwarder.GetPayload(), we use omf.mutex.RLock() (read lock) and then mutate shared state omf.OperatorMetadata.*.

• Multiple goroutines can hold an RLock() at the same time, so two callers could write to omf.OperatorMetadata concurrently.
• OperatorMetadata contains ResourceCounts map[string]int with maps not being safe for concurrent access if anyone might write.

A safe approach could be: copy under lock, unlock, then mutate the local copy and marshal

	omf.mutex.RLock()
	// Copy metadata while holding the lock, to avoid data races.
	operatorMetadata := omf.OperatorMetadata
	if omf.OperatorMetadata.ResourceCounts != nil {
		operatorMetadata.ResourceCounts = make(map[string]int, len(omf.OperatorMetadata.ResourceCounts))
		maps.Copy(operatorMetadata.ResourceCounts, omf.OperatorMetadata.ResourceCounts)
	}
	omf.mutex.RUnlock()

[swang392]

addressed in a separate PR: #2481

[swang392]

We have payload.clusterUID, payload.UUID and payload.metadata.clusterUID for the same value, can we possibly remove clusterID from OperatorMetadata, CRDMetadata and HelmMetadata structs ? Also, possibly remove it from payload if it's not necessary in the schema at intake to only keep UUID ? (since it can be derived from UUID ?)

I think these will need to be kept since each instance of clusterID is needed for something different. payload.UUID is needed by sobotka, payload.clusterUID is needed in dd-go to create the metadata resource from payload.metadata, and it can't be derived from payload.UUID. FA needs payload.metadata.clusterUID for the foreign key.

One possible option would be to rely on payload.metadata.clusterUID to create the resource instead of payload.clusterUID, but that would introduce inconsistency between how cluster-level metadata payloads are created between the operator and cluster agent.

[swang392]

More generally (and outside the PR scope), we repeat a lot of OperatorVersion and KubernetesVersion keys in the structs, would it be possible to reduce duplication by using something like:

yep, I have a card here to refactor sharedMetadata and reduce duplication.