Add root-level cluster_name and standardize collection_interval for OpenTelemetry features - #2189
Conversation
23cab82 to
59cea26
Compare
d7d65ea to
23ade6b
Compare
| "opentelemetry": map[string]interface{}{ | ||
| "collect": map[string]interface{}{ | ||
| "container_insights": map[string]interface{}{ | ||
| "cluster_name": "test-cluster", |
There was a problem hiding this comment.
seems like the cluster_name field is still showing up under container_insights section instead of showing up under opentelemetry/collect section, mind updating this please ?
5689959 to
65265df
Compare
Binary Size Reportlinux/amd64
Notable changes:
linux/arm64
windows/amd64
Investigating size changesUse go-size-analyzer to compare binaries: GOEXPERIMENT=jsonv2 go install github.com/Zxilly/go-size-analyzer/cmd/gsa@latest
gsa diff --old <baseline-binary> --new <new-binary> |
10d20a4 to
8ca11d7
Compare
ba39f5f to
ec09e03
Compare
384e0a9 to
7acb80a
Compare
…OpenTelemetry features
…OpenTelemetry features
115f621 to
d1a157e
Compare
d1a157e to
f42dcc3
Compare
| func GetClusterName(conf *confmap.Conf) string { | ||
| val, ok := GetString(conf, ConfigKey(LogsKey, MetricsCollectedKey, KubernetesKey, "cluster_name")) | ||
| if ok && val != "" { | ||
| func GetClusterName(conf *confmap.Conf, key string) string { |
There was a problem hiding this comment.
seems like this is missing cluster name validation ? https://github.com/aws/amazon-cloudwatch-agent/blob/main/translator/translate/otel/pipeline/opentelemetry/containerinsights/common.go#L81
| "cluster_name": { | ||
| "description": "The name of the K8s cluster. Applied to all telemetry collected under opentelemetry.", | ||
| "type": "string", | ||
| "pattern": "^[0-9A-Za-z][A-Za-z0-9\\-_]*$" |
There was a problem hiding this comment.
seems like this is a different regex than https://github.com/aws/amazon-cloudwatch-agent/blob/main/translator/translate/otel/pipeline/opentelemetry/containerinsights/common.go#L81
| processors.Set(k8sattributesprocessor.NewTranslator(common.OpenTelemetryKey)) | ||
| } | ||
| // Apply root-level cluster name if set | ||
| clusterName := common.GetClusterName(conf, common.OtelClusterNameKey) |
There was a problem hiding this comment.
nit - this block of code seems to be repeating across the metric, logs and traces pipeline, might be worth moving into a common helper function
Description of the issue
The OpenTelemetry config section was inconsistent: cluster_name had to be set separately on each feature, and the collection interval field was named differently across features (metrics_collection_interval vs collection_interval).
Description of changes
opentelemetry.cluster_nameso one value applies to all features (Container Insights, Prometheus, Host Metrics, OTLP) instead of being repeated per component. Container Insights requires it; the others apply it when present.Tests
License
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.