Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
mabdinur committed Aug 5, 2024
1 parent f973d9b commit cb7a318
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 25 deletions.
21 changes: 11 additions & 10 deletions pkg/otlp/metrics/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,13 @@ type translatorConfig struct {

// withRemapping reports whether certain metrics that are only available when using
// the Datadog Agent should be obtained by remapping from OTEL counterparts (e.g.
// container.* and system.* metrics).
// container.* and system.* metrics). This configuration also enables withOTelPrefix.
withRemapping bool

// withRenaming reports whether OpenTelemetry system and process metrics should be
// renamed with the `otel.` prefix. Enabling this configuration disables computing
// Datadog metrics from OpenTelemetry metrics. This is useful when the translator is
// running as part of a Collector with the Datadog Agent.
withRenaming bool
// withOTelPrefix reports whether some OpenTelemetry metrics (ex: host metrics) should be
// renamed with the `otel.` prefix. This prevents the Collector and Datadog
// Agent from computing metrics with the same names.
withOTelPrefix bool

// cache configuration
sweepInterval int64
Expand All @@ -64,16 +63,18 @@ type TranslatorOption func(*translatorConfig) error
func WithRemapping() TranslatorOption {
return func(t *translatorConfig) error {
t.withRemapping = true
// to maintain backward compatibility with the old remapping logic
// withRemapping must rename some otel metrics
t.withOTelPrefix = true
return nil
}
}

// withRenaming appends the `otel.` prefix to otel system and process metrics.
// Also disables the remapping of OpenTelemetry metrics to Datadog metrics. T
// withOTelPrefix appends the `otel.` prefix to OpenTelemetry metrics.
// This is useful when the translator is running as part of a Collector with the Datadog Agent.
func WithRenaming() TranslatorOption {
func WithOTelPrefix() TranslatorOption {
return func(t *translatorConfig) error {
t.withRenaming = true
t.withOTelPrefix = true
return nil
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/otlp/metrics/kafka_mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ func remapJvmMetrics(all pmetric.MetricSlice, m pmetric.Metric) {
}
}

// renameKafkaMetrics renames otel kafka metrics to avoid conflicts with DD agent metrics
// renameKafkaMetrics renames otel kafka metrics to avoid conflicts with DD agent metrics.
func renameKafkaMetrics(m pmetric.Metric) {
switch m.Name() {
case "kafka.producer.request-rate", "kafka.producer.response-rate", "kafka.producer.request-latency-avg", "kafka.consumer.fetch-size-avg", "kafka.producer.compression-rate", "kafka.producer.record-retry-rate", "kafka.producer.record-send-rate", "kafka.producer.record-error-rate":
Expand Down
15 changes: 8 additions & 7 deletions pkg/otlp/metrics/metrics_remapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,16 @@ func remapMetrics(all pmetric.MetricSlice, m pmetric.Metric) {
remapJvmMetrics(all, m)
}

// renameMetrics adds the `otel.` prefix to metrics.
func renameMetrics(m pmetric.Metric) {
renameSystemMetrics(m)
renameHostMetrics(m)
renameKafkaMetrics(m)
}

// remapSystemMetrics extracts system metrics from m and appends them to all.
func remapSystemMetrics(all pmetric.MetricSlice, m pmetric.Metric) {
name := m.Name()
if !isSystemMetric(name) {
if !isHostMetric(name) {
return
}
switch name {
Expand Down Expand Up @@ -132,8 +133,8 @@ func remapContainerMetrics(all pmetric.MetricSlice, m pmetric.Metric) {
}
}

// isSystemMetric determines whether a metric is a system metric.
func isSystemMetric(name string) bool {
// isHostMetric determines whether a metric is a system metric.
func isHostMetric(name string) bool {
return strings.HasPrefix(name, "process.") || strings.HasPrefix(name, "system.")
}

Expand Down Expand Up @@ -230,9 +231,9 @@ func hasAny(point pmetric.NumberDataPoint, tags ...kv) bool {
return false
}

// renameSystemMetrics renames otel system metrics to avoid conflicts with DD agent metrics
func renameSystemMetrics(m pmetric.Metric) {
if isSystemMetric(m.Name()) {
// renameHostMetrics renames otel host metrics to avoid conflicts with DD agent metrics.
func renameHostMetrics(m pmetric.Metric) {
if isHostMetric(m.Name()) {
m.SetName("otel." + m.Name())
}
}
6 changes: 2 additions & 4 deletions pkg/otlp/metrics/metrics_translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -805,10 +805,8 @@ func (t *Translator) MapMetrics(ctx context.Context, md pmetric.Metrics, consume

if t.cfg.withRemapping {
remapMetrics(newMetrics, md)
// to maintain backward compatibility with the old remapping logic
// we also need to rename some otel metrics
renameMetrics(md)
} else if t.cfg.withRenaming {
}
if t.cfg.withOTelPrefix {
renameMetrics(md)
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/otlp/metrics/metrics_translator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1018,10 +1018,10 @@ func TestMapRuntimeMetricsHasMappingCollector(t *testing.T) {
assert.Equal(t, []string{"go"}, rmt.Languages)
}

func TestMapSystemMetricsRenamedWithRenaming(t *testing.T) {
func TestMapSystemMetricsRenamedWithOTelPrefix(t *testing.T) {
ctx := context.Background()
// WithRenaming() is used to rename the system metrics, this overrides WithRemapping.
tr := NewTestTranslator(t, WithRenaming())
// WithOTelPrefix() is used to rename the system metrics, this overrides WithRemapping.
tr := NewTestTranslator(t, WithOTelPrefix())
consumer := &mockFullConsumer{}
systemDims := newDims("system.cpu.utilization")
processDims := newDims("process.runtime.go.goroutines")
Expand Down

0 comments on commit cb7a318

Please sign in to comment.