Skip to content

Commit

Permalink
Remove high-cardinality attribute from AWS Container Insight metrics (#…
Browse files Browse the repository at this point in the history
…37697)

#### Description
Per code inspection it looks like `tags` was being used as a convenience
feature to pass the timestamp when converting the metrics to OTLP.
However, timestamp should not be a resource attribute due to causing
high-cardinality time series.

This change keeps the current usage of `tags`, but, ensures that
timestamp is not added as a resource attribute. Code owners should
consider if later the timestamp should be passed outside the `tags` map
- a change much larger than the current one.

#### Link to tracking issue
Fixes #35861

#### Testing
Updated respective tests.

#### Documentation
Changelog added.
  • Loading branch information
pjanotti authored Feb 24, 2025
1 parent 94e11cc commit fa6262e
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 22 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
change_type: breaking

component: awscontainerinsightreceiver

note: Remove high cardinality attribute `Timestamp` from metrics generated by `awscontainerinsightreceiver`

issues: [35861]

change_logs: [user, api]
10 changes: 6 additions & 4 deletions internal/aws/containerinsight/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"log"
"strconv"
"strings"
"time"

"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/pmetric"
Expand Down Expand Up @@ -154,7 +153,9 @@ func GetUnitForMetric(metric string) string {
return metricToUnitMap[metric]
}

// ConvertToOTLPMetrics converts a field containing metric values and a tag containing the relevant labels to OTLP metrics
// ConvertToOTLPMetrics converts a field containing metric values and tags containing the relevant labels to OTLP metrics.
// For legacy reasons, the timestamp is stored in the tags map with the key "Timestamp", but, unlike other tags,
// it is not added as a resource attribute to avoid high-cardinality metrics.
func ConvertToOTLPMetrics(fields map[string]any, tags map[string]string, logger *zap.Logger) pmetric.Metrics {
md := pmetric.NewMetrics()
rms := md.ResourceMetrics()
Expand All @@ -166,8 +167,9 @@ func ConvertToOTLPMetrics(fields map[string]any, tags map[string]string, logger
if tagKey == Timestamp {
timeNs, _ := strconv.ParseUint(tagValue, 10, 64)
timestamp = pcommon.Timestamp(timeNs)
// convert from nanosecond to millisecond (as emf log use millisecond timestamp)
tagValue = strconv.FormatUint(timeNs/uint64(time.Millisecond), 10)

// Do not add Timestamp as a resource attribute to avoid high-cardinality.
continue
}
resource.Attributes().PutStr(tagKey, tagValue)
}
Expand Down
14 changes: 10 additions & 4 deletions internal/aws/containerinsight/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,16 +140,22 @@ func checkMetricsAreExpected(t *testing.T, md pmetric.Metrics, fields map[string
// check the attributes are expected
rm := rms.At(0)
attributes := rm.Resource().Attributes()
assert.Equal(t, len(tags), attributes.Len())
assert.Equal(t, len(tags)-1, attributes.Len()) // tags has Timestamp but attributes should not have it
var timeUnixNano uint64
for key, val := range tags {
log.Printf("key=%v value=%v", key, val)
attr, ok := attributes.Get(key)
assert.True(t, ok)
if key == Timestamp {
timeUnixNano, _ = strconv.ParseUint(val, 10, 64)
val = strconv.FormatUint(timeUnixNano/uint64(time.Millisecond), 10)
// Capture the timestamp for later check.
var err error
timeUnixNano, err = strconv.ParseUint(val, 10, 64)
assert.NoError(t, err)

// Timestamp is passed on tags but should NOT be in attributes.
assert.False(t, ok)
continue
}
assert.True(t, ok)
assert.Equal(t, val, attr.Str())
}

Expand Down
14 changes: 0 additions & 14 deletions receiver/awscontainerinsightreceiver/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,6 @@ kubectl apply -f config.yaml
| ClusterName |
| NodeName |
| Type |
| Timestamp |
| Version |
| Sources |

Expand All @@ -362,7 +361,6 @@ kubectl apply -f config.yaml
| NodeName |
| Namespace |
| Type |
| Timestamp |
| Version |
| Sources |
| kubernete |
Expand All @@ -384,7 +382,6 @@ kubectl apply -f config.yaml
| Namespace |
| Service |
| Type |
| Timestamp |
| Version |
| Sources |
| kubernete |
Expand Down Expand Up @@ -437,7 +434,6 @@ kubectl apply -f config.yaml
| ClusterName |
| InstanceType |
| NodeName |
| Timestamp |
| Type |
| Version |
| Sources |
Expand Down Expand Up @@ -468,7 +464,6 @@ kubectl apply -f config.yaml
| InstanceId |
| InstanceType |
| NodeName |
| Timestamp |
| EBSVolumeId |
| device |
| Type |
Expand Down Expand Up @@ -496,7 +491,6 @@ kubectl apply -f config.yaml
| InstanceId |
| InstanceType |
| NodeName |
| Timestamp |
| EBSVolumeId |
| device |
| fstype |
Expand Down Expand Up @@ -528,7 +522,6 @@ kubectl apply -f config.yaml
| InstanceId |
| InstanceType |
| NodeName |
| Timestamp |
| Type |
| Version |
| interface |
Expand Down Expand Up @@ -588,7 +581,6 @@ kubectl apply -f config.yaml
| Namespace |
| NodeName |
| PodId |
| Timestamp |
| Type |
| Version |
| Sources |
Expand Down Expand Up @@ -621,7 +613,6 @@ kubectl apply -f config.yaml
| Namespace |
| NodeName |
| PodId |
| Timestamp |
| Type |
| Version |
| interface |
Expand Down Expand Up @@ -672,7 +663,6 @@ kubectl apply -f config.yaml
| Namespace |
| NodeName |
| PodId |
| Timestamp |
| Type |
| Version |
| Sources |
Expand Down Expand Up @@ -779,7 +769,6 @@ To deploy to an ECS cluster check this [doc](https://aws-otel.github.io/docs/set
| ClusterName |
| InstanceType |
| AutoScalingGroupName |
| Timestamp |
| Type |
| Version |
| Sources |
Expand Down Expand Up @@ -810,7 +799,6 @@ To deploy to an ECS cluster check this [doc](https://aws-otel.github.io/docs/set
| ClusterName |
| InstanceType |
| AutoScalingGroupName |
| Timestamp |
| Type |
| Version |
| Sources |
Expand All @@ -837,7 +825,6 @@ To deploy to an ECS cluster check this [doc](https://aws-otel.github.io/docs/set
| ClusterName |
| InstanceType |
| AutoScalingGroupName |
| Timestamp |
| Type |
| Version |
| Sources |
Expand Down Expand Up @@ -866,7 +853,6 @@ To deploy to an ECS cluster check this [doc](https://aws-otel.github.io/docs/set
| ClusterName |
| InstanceType |
| AutoScalingGroupName |
| Timestamp |
| Type |
| Version |
| Sources |
Expand Down

0 comments on commit fa6262e

Please sign in to comment.