From 368aa6640cde29b5bc0fe36f5122db9c71d75971 Mon Sep 17 00:00:00 2001 From: Vishal Raj Date: Mon, 22 Jul 2024 12:13:02 +0100 Subject: [PATCH 1/4] Add configuration to disable attribute enrichment --- enrichments/trace/config/config.go | 44 +++++++++++++++++++ enrichments/trace/internal/elastic/span.go | 34 +++++++++++--- .../trace/internal/elastic/span_test.go | 18 +++++++- enrichments/trace/trace.go | 18 +++++++- enrichments/trace/trace_test.go | 4 +- 5 files changed, 108 insertions(+), 10 deletions(-) create mode 100644 enrichments/trace/config/config.go diff --git a/enrichments/trace/config/config.go b/enrichments/trace/config/config.go new file mode 100644 index 0000000..cb2b9af --- /dev/null +++ b/enrichments/trace/config/config.go @@ -0,0 +1,44 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package config + +// Config configures the enrichment attributes produced. +type Config struct { + Transaction ElasticTransactionConfig `mapstructure:"elastic_transaction"` + Span ElasticSpanConfig `mapstructure:"elastic_span"` +} + +// ElasticTransactionConfig configures the enrichment attributes for the +// spans which are identified as elastic transaction. +type ElasticTransactionConfig struct { + Type AttributeConfig `mapstructure:"type"` + Result AttributeConfig `mapstructure:"result"` + EventOutcome AttributeConfig `mapstructure:"event_outcome"` +} + +// ElasticSpanConfig configures the enrichment attributes for the spans +// which are NOT identified as elastic transaction. +type ElasticSpanConfig struct { + EventOutcome AttributeConfig `mapstructure:"event_outcome"` + ServiceTarget AttributeConfig `mapstructure:"service_target"` +} + +// AttributeConfig is the configuration options for each attribute. +type AttributeConfig struct { + Disabled bool `mapstructure:"disabled"` +} diff --git a/enrichments/trace/internal/elastic/span.go b/enrichments/trace/internal/elastic/span.go index 9d81333..87463b0 100644 --- a/enrichments/trace/internal/elastic/span.go +++ b/enrichments/trace/internal/elastic/span.go @@ -24,6 +24,7 @@ import ( "net/url" "strconv" + "github.com/elastic/opentelemetry-lib/enrichments/trace/config" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/ptrace" semconv "go.opentelemetry.io/collector/semconv/v1.25.0" @@ -40,9 +41,9 @@ import ( // - Elastic spans, defined as all spans (including transactions). // However, for the enrichment logic spans are treated as a separate // entity i.e. all transactions are not enriched as spans and vice versa. -func EnrichSpan(span ptrace.Span) { +func EnrichSpan(span ptrace.Span, cfg config.Config) { var c spanEnrichmentContext - c.Enrich(span) + c.Enrich(span, cfg) } type spanEnrichmentContext struct { @@ -73,7 +74,7 @@ type spanEnrichmentContext struct { messagingDestinationTemp bool } -func (s *spanEnrichmentContext) Enrich(span ptrace.Span) { +func (s *spanEnrichmentContext) Enrich(span ptrace.Span, cfg config.Config) { // Extract top level span information. s.spanStatusCode = span.Status().Code() @@ -149,13 +150,36 @@ func (s *spanEnrichmentContext) Enrich(span ptrace.Span) { // Ensure all dependent attributes are handled. s.normalizeAttributes() - // Enrich the span depending on the nature of the span. if isElasticTransaction(span) { + s.enrichTransaction(span, cfg.Transaction) + } else { + s.enrichSpan(span, cfg.Span) + } +} + +func (s *spanEnrichmentContext) enrichTransaction( + span ptrace.Span, + cfg config.ElasticTransactionConfig, +) { + if !cfg.Type.Disabled { s.setTxnType(span) + } + if !cfg.Result.Disabled { s.setTxnResult(span) + } + if !cfg.EventOutcome.Disabled { s.setEventOutcome(span) - } else { + } +} + +func (s *spanEnrichmentContext) enrichSpan( + span ptrace.Span, + cfg config.ElasticSpanConfig, +) { + if !cfg.EventOutcome.Disabled { s.setEventOutcome(span) + } + if !cfg.ServiceTarget.Disabled { s.setServiceTarget(span) } } diff --git a/enrichments/trace/internal/elastic/span_test.go b/enrichments/trace/internal/elastic/span_test.go index 9ad1caf..357f0db 100644 --- a/enrichments/trace/internal/elastic/span_test.go +++ b/enrichments/trace/internal/elastic/span_test.go @@ -21,6 +21,7 @@ import ( "net/http" "testing" + "github.com/elastic/opentelemetry-lib/enrichments/trace/config" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/pdata/ptrace" @@ -34,6 +35,7 @@ func TestElasticTransactionEnrich(t *testing.T) { for _, tc := range []struct { name string input ptrace.Span + config config.ElasticTransactionConfig enrichedAttrs map[string]any }{ { @@ -45,6 +47,16 @@ func TestElasticTransactionEnrich(t *testing.T) { AttributeTransactionType: "unknown", }, }, + { + name: "all_disabled", + input: ptrace.NewSpan(), + config: config.ElasticTransactionConfig{ + Type: config.AttributeConfig{Disabled: true}, + Result: config.AttributeConfig{Disabled: true}, + EventOutcome: config.AttributeConfig{Disabled: true}, + }, + enrichedAttrs: map[string]any{}, + }, { name: "http_status_ok", input: func() ptrace.Span { @@ -177,7 +189,9 @@ func TestElasticTransactionEnrich(t *testing.T) { expectedAttrs[k] = v } - EnrichSpan(tc.input) + EnrichSpan(tc.input, config.Config{ + Transaction: tc.config, + }) assert.Empty(t, cmp.Diff(expectedAttrs, tc.input.Attributes().AsRaw())) }) @@ -422,7 +436,7 @@ func TestElasticSpanEnrich(t *testing.T) { expectedAttrs[k] = v } - EnrichSpan(tc.input) + EnrichSpan(tc.input, config.Config{}) assert.Empty(t, cmp.Diff(expectedAttrs, tc.input.Attributes().AsRaw())) }) diff --git a/enrichments/trace/trace.go b/enrichments/trace/trace.go index 8915cde..ef08213 100644 --- a/enrichments/trace/trace.go +++ b/enrichments/trace/trace.go @@ -18,15 +18,29 @@ package trace import ( + "github.com/elastic/opentelemetry-lib/enrichments/trace/config" "github.com/elastic/opentelemetry-lib/enrichments/trace/internal/elastic" "go.opentelemetry.io/collector/pdata/ptrace" ) +// Enricher enriches the OTel traces with attributes required to power +// functionalities in the Elastic UI. +type Enricher struct { + Config config.Config +} + +// NewEnricher creates a new instance of Enricher. +func NewEnricher(cfg config.Config) Enricher { + return Enricher{ + Config: cfg, + } +} + // Enrich enriches the OTel traces with attributes required to power // functionalities in the Elastic UI. The traces are processed as per the // Elastic's definition of transactions and spans. The traces passed to // this function are mutated. -func Enrich(pt ptrace.Traces) { +func (e *Enricher) Enrich(pt ptrace.Traces) { resSpans := pt.ResourceSpans() for i := 0; i < resSpans.Len(); i++ { resSpan := resSpans.At(i) @@ -35,7 +49,7 @@ func Enrich(pt ptrace.Traces) { scopeSpan := scopeSpans.At(j) spans := scopeSpan.Spans() for k := 0; k < spans.Len(); k++ { - elastic.EnrichSpan(spans.At(k)) + elastic.EnrichSpan(spans.At(k), e.Config) } } } diff --git a/enrichments/trace/trace_test.go b/enrichments/trace/trace_test.go index b91a435..1bb07a3 100644 --- a/enrichments/trace/trace_test.go +++ b/enrichments/trace/trace_test.go @@ -22,6 +22,7 @@ import ( "path/filepath" "testing" + "github.com/elastic/opentelemetry-lib/enrichments/trace/config" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/pdata/pcommon" @@ -31,11 +32,12 @@ func BenchmarkEnrich(b *testing.B) { traceFile := filepath.Join("testdata", "trace.yaml") traces, err := golden.ReadTraces(traceFile) require.NoError(b, err) + enricher := NewEnricher(config.Config{}) b.ReportAllocs() b.ResetTimer() for i := 0; i < b.N; i++ { - Enrich(traces) + enricher.Enrich(traces) } } From 4eb0699b117071e0fc9d1a6033c3ff9ef03a5628 Mon Sep 17 00:00:00 2001 From: Vishal Raj Date: Mon, 22 Jul 2024 12:23:03 +0100 Subject: [PATCH 2/4] Add disabled test for elastic span --- enrichments/trace/internal/elastic/span_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/enrichments/trace/internal/elastic/span_test.go b/enrichments/trace/internal/elastic/span_test.go index 357f0db..4731af5 100644 --- a/enrichments/trace/internal/elastic/span_test.go +++ b/enrichments/trace/internal/elastic/span_test.go @@ -208,6 +208,7 @@ func TestElasticSpanEnrich(t *testing.T) { for _, tc := range []struct { name string input ptrace.Span + config config.ElasticSpanConfig enrichedAttrs map[string]any }{ { @@ -217,6 +218,15 @@ func TestElasticSpanEnrich(t *testing.T) { AttributeEventOutcome: "success", }, }, + { + name: "all_disabled", + input: getElasticSpan(), + config: config.ElasticSpanConfig{ + EventOutcome: config.AttributeConfig{Disabled: true}, + ServiceTarget: config.AttributeConfig{Disabled: true}, + }, + enrichedAttrs: map[string]any{}, + }, { name: "peer_service", input: func() ptrace.Span { @@ -436,7 +446,9 @@ func TestElasticSpanEnrich(t *testing.T) { expectedAttrs[k] = v } - EnrichSpan(tc.input, config.Config{}) + EnrichSpan(tc.input, config.Config{ + Span: tc.config, + }) assert.Empty(t, cmp.Diff(expectedAttrs, tc.input.Attributes().AsRaw())) }) From 02a2d57a2d54de7d35650516e756c8d64f39d0e7 Mon Sep 17 00:00:00 2001 From: Vishal Raj Date: Thu, 25 Jul 2024 13:06:44 +0100 Subject: [PATCH 3/4] Use enabled to control attribute selection --- enrichments/trace/config/config.go | 17 ++++++- enrichments/trace/config/config_test.go | 42 +++++++++++++++++ enrichments/trace/internal/elastic/span.go | 10 ++-- .../trace/internal/elastic/span_test.go | 47 ++++++++++++------- 4 files changed, 93 insertions(+), 23 deletions(-) create mode 100644 enrichments/trace/config/config_test.go diff --git a/enrichments/trace/config/config.go b/enrichments/trace/config/config.go index cb2b9af..99a9de7 100644 --- a/enrichments/trace/config/config.go +++ b/enrichments/trace/config/config.go @@ -40,5 +40,20 @@ type ElasticSpanConfig struct { // AttributeConfig is the configuration options for each attribute. type AttributeConfig struct { - Disabled bool `mapstructure:"disabled"` + Enabled bool `mapstructure:"enabled"` +} + +// Enabled returns a config with all default enrichments enabled. +func Enabled() Config { + return Config{ + Transaction: ElasticTransactionConfig{ + Type: AttributeConfig{Enabled: true}, + Result: AttributeConfig{Enabled: true}, + EventOutcome: AttributeConfig{Enabled: true}, + }, + Span: ElasticSpanConfig{ + EventOutcome: AttributeConfig{Enabled: true}, + ServiceTarget: AttributeConfig{Enabled: true}, + }, + } } diff --git a/enrichments/trace/config/config_test.go b/enrichments/trace/config/config_test.go new file mode 100644 index 0000000..00af213 --- /dev/null +++ b/enrichments/trace/config/config_test.go @@ -0,0 +1,42 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package config + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestEnabled(t *testing.T) { + config := Enabled() + assertAllEnabled(t, reflect.ValueOf(config.Transaction)) + assertAllEnabled(t, reflect.ValueOf(config.Span)) +} + +func assertAllEnabled(t *testing.T, cfg reflect.Value) { + t.Helper() + + for i := 0; i < cfg.NumField(); i++ { + rAttrCfg := cfg.Field(i).Interface() + attrCfg, ok := rAttrCfg.(AttributeConfig) + require.True(t, ok, "must be a type of AttributeConfig") + require.True(t, attrCfg.Enabled, "must be enabled") + } +} diff --git a/enrichments/trace/internal/elastic/span.go b/enrichments/trace/internal/elastic/span.go index 87463b0..5f374e4 100644 --- a/enrichments/trace/internal/elastic/span.go +++ b/enrichments/trace/internal/elastic/span.go @@ -161,13 +161,13 @@ func (s *spanEnrichmentContext) enrichTransaction( span ptrace.Span, cfg config.ElasticTransactionConfig, ) { - if !cfg.Type.Disabled { + if cfg.Type.Enabled { s.setTxnType(span) } - if !cfg.Result.Disabled { + if cfg.Result.Enabled { s.setTxnResult(span) } - if !cfg.EventOutcome.Disabled { + if cfg.EventOutcome.Enabled { s.setEventOutcome(span) } } @@ -176,10 +176,10 @@ func (s *spanEnrichmentContext) enrichSpan( span ptrace.Span, cfg config.ElasticSpanConfig, ) { - if !cfg.EventOutcome.Disabled { + if cfg.EventOutcome.Enabled { s.setEventOutcome(span) } - if !cfg.ServiceTarget.Disabled { + if cfg.ServiceTarget.Enabled { s.setServiceTarget(span) } } diff --git a/enrichments/trace/internal/elastic/span_test.go b/enrichments/trace/internal/elastic/span_test.go index 4731af5..48b753c 100644 --- a/enrichments/trace/internal/elastic/span_test.go +++ b/enrichments/trace/internal/elastic/span_test.go @@ -39,8 +39,9 @@ func TestElasticTransactionEnrich(t *testing.T) { enrichedAttrs map[string]any }{ { - name: "empty", - input: ptrace.NewSpan(), + name: "empty", + input: ptrace.NewSpan(), + config: config.Enabled().Transaction, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeTransactionResult: "Success", @@ -48,13 +49,8 @@ func TestElasticTransactionEnrich(t *testing.T) { }, }, { - name: "all_disabled", - input: ptrace.NewSpan(), - config: config.ElasticTransactionConfig{ - Type: config.AttributeConfig{Disabled: true}, - Result: config.AttributeConfig{Disabled: true}, - EventOutcome: config.AttributeConfig{Disabled: true}, - }, + name: "all_disabled", + input: ptrace.NewSpan(), enrichedAttrs: map[string]any{}, }, { @@ -64,6 +60,7 @@ func TestElasticTransactionEnrich(t *testing.T) { span.Attributes().PutInt(semconv.AttributeHTTPStatusCode, http.StatusOK) return span }(), + config: config.Enabled().Transaction, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeTransactionResult: "HTTP 2xx", @@ -82,6 +79,7 @@ func TestElasticTransactionEnrich(t *testing.T) { ) return span }(), + config: config.Enabled().Transaction, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeTransactionResult: "HTTP 1xx", @@ -99,6 +97,7 @@ func TestElasticTransactionEnrich(t *testing.T) { semconv.AttributeHTTPStatusCode, http.StatusInternalServerError) return span }(), + config: config.Enabled().Transaction, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeTransactionResult: "HTTP 5xx", @@ -117,6 +116,7 @@ func TestElasticTransactionEnrich(t *testing.T) { ) return span }(), + config: config.Enabled().Transaction, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeTransactionResult: "OK", @@ -135,6 +135,7 @@ func TestElasticTransactionEnrich(t *testing.T) { ) return span }(), + config: config.Enabled().Transaction, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeTransactionResult: "Internal", @@ -148,6 +149,7 @@ func TestElasticTransactionEnrich(t *testing.T) { span.Status().SetCode(ptrace.StatusCodeOk) return span }(), + config: config.Enabled().Transaction, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeTransactionResult: "Success", @@ -161,6 +163,7 @@ func TestElasticTransactionEnrich(t *testing.T) { span.Status().SetCode(ptrace.StatusCodeError) return span }(), + config: config.Enabled().Transaction, enrichedAttrs: map[string]any{ AttributeEventOutcome: "failure", AttributeTransactionResult: "Error", @@ -174,6 +177,7 @@ func TestElasticTransactionEnrich(t *testing.T) { span.Attributes().PutStr(semconv.AttributeMessagingSystem, "kafka") return span }(), + config: config.Enabled().Transaction, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeTransactionResult: "Success", @@ -212,19 +216,16 @@ func TestElasticSpanEnrich(t *testing.T) { enrichedAttrs map[string]any }{ { - name: "empty", - input: getElasticSpan(), + name: "empty", + input: getElasticSpan(), + config: config.Enabled().Span, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", }, }, { - name: "all_disabled", - input: getElasticSpan(), - config: config.ElasticSpanConfig{ - EventOutcome: config.AttributeConfig{Disabled: true}, - ServiceTarget: config.AttributeConfig{Disabled: true}, - }, + name: "all_disabled", + input: getElasticSpan(), enrichedAttrs: map[string]any{}, }, { @@ -234,6 +235,7 @@ func TestElasticSpanEnrich(t *testing.T) { span.Attributes().PutStr(semconv.AttributePeerService, "testsvc") return span }(), + config: config.Enabled().Span, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeServiceTargetName: "testsvc", @@ -250,6 +252,7 @@ func TestElasticSpanEnrich(t *testing.T) { ) return span }(), + config: config.Enabled().Span, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeServiceTargetType: "http", @@ -273,6 +276,7 @@ func TestElasticSpanEnrich(t *testing.T) { ) return span }(), + config: config.Enabled().Span, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeServiceTargetType: "http", @@ -294,6 +298,7 @@ func TestElasticSpanEnrich(t *testing.T) { span.Attributes().PutInt(semconv.AttributeURLPort, 443) return span }(), + config: config.Enabled().Span, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeServiceTargetType: "http", @@ -311,6 +316,7 @@ func TestElasticSpanEnrich(t *testing.T) { ) return span }(), + config: config.Enabled().Span, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeServiceTargetType: "grpc", @@ -325,6 +331,7 @@ func TestElasticSpanEnrich(t *testing.T) { span.Attributes().PutStr(semconv.AttributeRPCSystem, "xmlrpc") return span }(), + config: config.Enabled().Span, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeServiceTargetType: "xmlrpc", @@ -341,6 +348,7 @@ func TestElasticSpanEnrich(t *testing.T) { span.Attributes().PutStr(semconv.AttributeRPCService, "service.Test") return span }(), + config: config.Enabled().Span, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeServiceTargetType: "external", @@ -355,6 +363,7 @@ func TestElasticSpanEnrich(t *testing.T) { span.Attributes().PutStr(semconv.AttributeMessagingSystem, "kafka") return span }(), + config: config.Enabled().Span, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeServiceTargetType: "kafka", @@ -369,6 +378,7 @@ func TestElasticSpanEnrich(t *testing.T) { span.Attributes().PutStr(semconv.AttributeMessagingDestinationName, "t1") return span }(), + config: config.Enabled().Span, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeServiceTargetType: "messaging", @@ -384,6 +394,7 @@ func TestElasticSpanEnrich(t *testing.T) { span.Attributes().PutStr(semconv.AttributeMessagingDestinationName, "t1") return span }(), + config: config.Enabled().Span, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeServiceTargetType: "messaging", @@ -405,6 +416,7 @@ func TestElasticSpanEnrich(t *testing.T) { ) return span }(), + config: config.Enabled().Span, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeServiceTargetType: "elasticsearch", @@ -431,6 +443,7 @@ func TestElasticSpanEnrich(t *testing.T) { ) return span }(), + config: config.Enabled().Span, enrichedAttrs: map[string]any{ AttributeEventOutcome: "success", AttributeServiceTargetType: "cassandra", From b67dffca086be4d59c194d419b8262be6722a958 Mon Sep 17 00:00:00 2001 From: Vishal Raj Date: Thu, 25 Jul 2024 17:05:13 +0100 Subject: [PATCH 4/4] Use ptr return type --- enrichments/trace/trace.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/enrichments/trace/trace.go b/enrichments/trace/trace.go index ef08213..0ec1d97 100644 --- a/enrichments/trace/trace.go +++ b/enrichments/trace/trace.go @@ -30,8 +30,8 @@ type Enricher struct { } // NewEnricher creates a new instance of Enricher. -func NewEnricher(cfg config.Config) Enricher { - return Enricher{ +func NewEnricher(cfg config.Config) *Enricher { + return &Enricher{ Config: cfg, } }