diff --git a/enrichments/enricher.go b/enrichments/enricher.go index 6dc43b8..571124f 100644 --- a/enrichments/enricher.go +++ b/enrichments/enricher.go @@ -18,12 +18,13 @@ package enrichments import ( - "github.com/elastic/opentelemetry-lib/enrichments/config" - "github.com/elastic/opentelemetry-lib/enrichments/internal/elastic" "github.com/ua-parser/uap-go/uaparser" "go.opentelemetry.io/collector/pdata/plog" "go.opentelemetry.io/collector/pdata/pmetric" "go.opentelemetry.io/collector/pdata/ptrace" + + "github.com/elastic/opentelemetry-lib/enrichments/config" + "github.com/elastic/opentelemetry-lib/enrichments/internal/elastic" ) // Enricher enriches the OTel traces with attributes required to power @@ -40,6 +41,7 @@ type Enricher struct { // 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. +// Any existing attributes will not be enriched or modified. func (e *Enricher) EnrichTraces(pt ptrace.Traces) { resSpans := pt.ResourceSpans() for i := 0; i < resSpans.Len(); i++ { @@ -59,6 +61,7 @@ func (e *Enricher) EnrichTraces(pt ptrace.Traces) { // EnrichLogs enriches the OTel logs with attributes required to power // functionalities in the Elastic UI. The logs passed to this function are mutated. +// Any existing attributes will not be enriched or modified. func (e *Enricher) EnrichLogs(pl plog.Logs) { resLogs := pl.ResourceLogs() for i := 0; i < resLogs.Len(); i++ { @@ -80,6 +83,7 @@ func (e *Enricher) EnrichLogs(pl plog.Logs) { // EnrichMetrics enriches the OTel metrics with attributes required to power // functionalities in the Elastic UI. The metrics passed to this function are mutated. +// Any existing attributes will not be enriched or modified. func (e *Enricher) EnrichMetrics(pl pmetric.Metrics) { resMetrics := pl.ResourceMetrics() for i := 0; i < resMetrics.Len(); i++ { diff --git a/enrichments/internal/attribute/attribute.go b/enrichments/internal/attribute/attribute.go new file mode 100644 index 0000000..75c86fc --- /dev/null +++ b/enrichments/internal/attribute/attribute.go @@ -0,0 +1,54 @@ +// 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 attribute + +import ( + "go.opentelemetry.io/collector/pdata/pcommon" +) + +// PutStr wrapper around the attribute map `PutStr` method +// that only inserts the entry if no key-value exists. +func PutStr(attrs pcommon.Map, key string, value string) { + if _, ok := attrs.Get(key); !ok { + attrs.PutStr(key, value) + } +} + +// PutInt wrapper around the attribute map `PutInt` method +// that only inserts the entry if no key-value exists +func PutInt(attrs pcommon.Map, key string, value int64) { + if _, ok := attrs.Get(key); !ok { + attrs.PutInt(key, value) + } +} + +// PutDouble wrapper around the attribute map `PutDouble` method +// that only inserts the entry if no key-value exists +func PutDouble(attrs pcommon.Map, key string, value float64) { + if _, ok := attrs.Get(key); !ok { + attrs.PutDouble(key, value) + } +} + +// PutBool wrapper around the attribute map `PutBool` method +// that only inserts the entry if no key-value exists +func PutBool(attrs pcommon.Map, key string, value bool) { + if _, ok := attrs.Get(key); !ok { + attrs.PutBool(key, value) + } +} diff --git a/enrichments/internal/attribute/attribute_test.go b/enrichments/internal/attribute/attribute_test.go new file mode 100644 index 0000000..e6dc6f8 --- /dev/null +++ b/enrichments/internal/attribute/attribute_test.go @@ -0,0 +1,116 @@ +// 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 attribute + +import ( + "testing" + + "go.opentelemetry.io/collector/pdata/pcommon" +) + +func TestPut(t *testing.T) { + var ( + key = "test_key" + + oldStr = "old_str_value" + oldInt = int64(123) + oldDouble = 2.71 + oldBool = false + + newStr = "test_str_value" + newInt = int64(42) + newDouble = 3.14 + newBool = true + ) + + tests := []struct { + name string + value any + exists bool + expected any + }{ + {name: "PutStr attr does not exist", value: newStr, exists: false, expected: newStr}, + {name: "PutStr attr exists", value: newStr, exists: true, expected: oldStr}, + {name: "PutInt attr does not exist", value: newInt, exists: false, expected: newInt}, + {name: "PutInt attr exists", value: newInt, exists: true, expected: oldInt}, + {name: "PutDouble attr does not exist", value: newDouble, exists: false, expected: newDouble}, + {name: "PutDouble attr exists", value: newDouble, exists: true, expected: oldDouble}, + {name: "PutBool attr does not exist", value: newBool, exists: false, expected: newBool}, + {name: "PutBool attr exists", value: newBool, exists: true, expected: oldBool}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Setup map based on the value type and if a prior value should exists + attrs := pcommon.NewMap() + if tt.exists { + switch v := tt.value.(type) { + case string: + attrs.PutStr(key, oldStr) + case int64: + attrs.PutInt(key, oldInt) + case float64: + attrs.PutDouble(key, oldDouble) + case bool: + attrs.PutBool(key, oldBool) + default: + t.Fatalf("unexpected value type: %T", v) + } + } + + // Attempt to add attribute based on value type + switch v := tt.value.(type) { + case string: + PutStr(attrs, key, v) + case int64: + PutInt(attrs, key, v) + case float64: + PutDouble(attrs, key, v) + case bool: + PutBool(attrs, key, v) + default: + t.Fatalf("unexpected value type: %T", v) + } + + // Read value from map + val, exists := attrs.Get(key) + if !exists { + t.Error("expected attribute to exist") + } + + // Validate the read value + var actualValue any + switch val.Type() { + case pcommon.ValueTypeStr: + actualValue = val.Str() + case pcommon.ValueTypeInt: + actualValue = val.Int() + case pcommon.ValueTypeDouble: + actualValue = val.Double() + case pcommon.ValueTypeBool: + actualValue = val.Bool() + default: + t.Fatalf("unexpected value type: %v", val.Type()) + } + + if actualValue != tt.expected { + t.Errorf("value = %v, expected %v", actualValue, tt.expected) + } + }) + } +} diff --git a/enrichments/internal/elastic/log.go b/enrichments/internal/elastic/log.go index 5a22836..375d9fd 100644 --- a/enrichments/internal/elastic/log.go +++ b/enrichments/internal/elastic/log.go @@ -20,15 +20,14 @@ package elastic import ( "github.com/elastic/opentelemetry-lib/elasticattr" "github.com/elastic/opentelemetry-lib/enrichments/config" + "github.com/elastic/opentelemetry-lib/enrichments/internal/attribute" "github.com/elastic/opentelemetry-lib/enrichments/internal/elastic/mobile" "go.opentelemetry.io/collector/pdata/plog" ) func EnrichLog(resourceAttrs map[string]any, log plog.LogRecord, cfg config.Config) { if cfg.Log.ProcessorEvent.Enabled { - if _, exists := log.Attributes().Get(elasticattr.ProcessorEvent); !exists { - log.Attributes().PutStr(elasticattr.ProcessorEvent, "log") - } + attribute.PutStr(log.Attributes(), elasticattr.ProcessorEvent, "log") } eventName, ok := getEventName(log) if ok { diff --git a/enrichments/internal/elastic/metric.go b/enrichments/internal/elastic/metric.go index 6e7a38b..a09af5e 100644 --- a/enrichments/internal/elastic/metric.go +++ b/enrichments/internal/elastic/metric.go @@ -20,13 +20,12 @@ package elastic import ( "github.com/elastic/opentelemetry-lib/elasticattr" "github.com/elastic/opentelemetry-lib/enrichments/config" + "github.com/elastic/opentelemetry-lib/enrichments/internal/attribute" "go.opentelemetry.io/collector/pdata/pmetric" ) func EnrichMetric(metric pmetric.ResourceMetrics, cfg config.Config) { if cfg.Metric.ProcessorEvent.Enabled { - if _, exists := metric.Resource().Attributes().Get(elasticattr.ProcessorEvent); !exists { - metric.Resource().Attributes().PutStr(elasticattr.ProcessorEvent, "metric") - } + attribute.PutStr(metric.Resource().Attributes(), elasticattr.ProcessorEvent, "metric") } } diff --git a/enrichments/internal/elastic/metric_test.go b/enrichments/internal/elastic/metric_test.go new file mode 100644 index 0000000..3bf8e80 --- /dev/null +++ b/enrichments/internal/elastic/metric_test.go @@ -0,0 +1,93 @@ +// 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 elastic + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "go.opentelemetry.io/collector/pdata/pmetric" + + "github.com/elastic/opentelemetry-lib/elasticattr" + "github.com/elastic/opentelemetry-lib/enrichments/config" +) + +func TestEnrichMetric(t *testing.T) { + getMetric := func() pmetric.ResourceMetrics { + metrics := pmetric.NewMetrics() + resourceMetrics := metrics.ResourceMetrics().AppendEmpty() + + // Add a metric data point to make it a valid metric + scopeMetrics := resourceMetrics.ScopeMetrics().AppendEmpty() + metric := scopeMetrics.Metrics().AppendEmpty() + metric.SetName("test.metric") + metric.SetUnit("1") + metric.SetEmptyGauge() + metric.Gauge().DataPoints().AppendEmpty().SetDoubleValue(1.0) + + return resourceMetrics + } + + for _, tc := range []struct { + name string + input pmetric.ResourceMetrics + config config.Config + expectedAttrs map[string]any + }{ + { + name: "existing_attributes_not_overridden", + input: func() pmetric.ResourceMetrics { + resourceMetrics := getMetric() + resource := resourceMetrics.Resource() + + // Set existing attributes that should not be overridden + resource.Attributes().PutStr(elasticattr.ProcessorEvent, "existing-processor-event") + resource.Attributes().PutStr(elasticattr.AgentName, "existing-agent-name") + resource.Attributes().PutStr(elasticattr.AgentVersion, "existing-agent-version") + + return resourceMetrics + }(), + config: config.Enabled(), + expectedAttrs: map[string]any{ + elasticattr.ProcessorEvent: "existing-processor-event", + elasticattr.AgentName: "existing-agent-name", + elasticattr.AgentVersion: "existing-agent-version", + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + expectedResourceMetrics := pmetric.NewResourceMetrics() + tc.input.Resource().CopyTo(expectedResourceMetrics.Resource()) + + // Merge with the expected attributes + for k, v := range tc.expectedAttrs { + expectedResourceMetrics.Resource().Attributes().PutEmpty(k).FromRaw(v) + } + + // Enrich the metric + EnrichMetric(tc.input, tc.config) + EnrichResource(tc.input.Resource(), tc.config.Resource) + + // Verify attributes match expected + actualAttrs := tc.input.Resource().Attributes().AsRaw() + expectedAttrs := expectedResourceMetrics.Resource().Attributes().AsRaw() + + assert.Equal(t, expectedAttrs, actualAttrs, "resource attributes should match expected") + }) + } +} diff --git a/enrichments/internal/elastic/mobile/event.go b/enrichments/internal/elastic/mobile/event.go index 023445c..bb62ea1 100644 --- a/enrichments/internal/elastic/mobile/event.go +++ b/enrichments/internal/elastic/mobile/event.go @@ -22,9 +22,11 @@ import ( "encoding/hex" "io" - "github.com/elastic/opentelemetry-lib/elasticattr" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/plog" + + "github.com/elastic/opentelemetry-lib/elasticattr" + "github.com/elastic/opentelemetry-lib/enrichments/internal/attribute" ) // EventContext contains contextual information for log event enrichment @@ -34,7 +36,7 @@ type EventContext struct { } func EnrichLogEvent(ctx EventContext, logRecord plog.LogRecord) { - logRecord.Attributes().PutStr(elasticattr.EventKind, "event") + attribute.PutStr(logRecord.Attributes(), elasticattr.EventKind, "event") if ctx.EventName == "device.crash" { enrichCrashEvent(logRecord, ctx.ResourceAttributes) @@ -46,10 +48,10 @@ func enrichCrashEvent(logRecord plog.LogRecord, resourceAttrs map[string]any) { if timestamp == 0 { timestamp = logRecord.ObservedTimestamp() } - logRecord.Attributes().PutStr(elasticattr.ProcessorEvent, "error") - logRecord.Attributes().PutInt(elasticattr.TimestampUs, getTimestampUs(timestamp)) + attribute.PutStr(logRecord.Attributes(), elasticattr.ProcessorEvent, "error") + attribute.PutInt(logRecord.Attributes(), elasticattr.TimestampUs, getTimestampUs(timestamp)) if id, err := newUniqueID(); err == nil { - logRecord.Attributes().PutStr(elasticattr.ErrorID, id) + attribute.PutStr(logRecord.Attributes(), elasticattr.ErrorID, id) } stacktrace, ok := logRecord.Attributes().Get("exception.stacktrace") if ok { @@ -57,15 +59,15 @@ func enrichCrashEvent(logRecord plog.LogRecord, resourceAttrs map[string]any) { if hasLanguage { switch language { case "java": - logRecord.Attributes().PutStr(elasticattr.ErrorGroupingKey, CreateJavaStacktraceGroupingKey(stacktrace.AsString())) + attribute.PutStr(logRecord.Attributes(), elasticattr.ErrorGroupingKey, CreateJavaStacktraceGroupingKey(stacktrace.AsString())) case "swift": if key, err := CreateSwiftStacktraceGroupingKey(stacktrace.AsString()); err == nil { - logRecord.Attributes().PutStr(elasticattr.ErrorGroupingKey, key) + attribute.PutStr(logRecord.Attributes(), elasticattr.ErrorGroupingKey, key) } } } } - logRecord.Attributes().PutStr(elasticattr.ErrorType, "crash") + attribute.PutStr(logRecord.Attributes(), elasticattr.ErrorType, "crash") } func newUniqueID() (string, error) { diff --git a/enrichments/internal/elastic/mobile/event_test.go b/enrichments/internal/elastic/mobile/event_test.go index 90da99e..a4b5ed2 100644 --- a/enrichments/internal/elastic/mobile/event_test.go +++ b/enrichments/internal/elastic/mobile/event_test.go @@ -148,6 +148,67 @@ func TestEnrichEvents(t *testing.T) { "event.kind": "event", }, }, + { + name: "crash_event_all_attributes_present", + eventName: "device.crash", + resourceAttrs: map[string]any{ + "telemetry.sdk.language": "java", + }, + input: func() plog.LogRecord { + logRecord := plog.NewLogRecord() + logRecord.SetTimestamp(timestamp) + logRecord.Attributes().PutStr("event.name", "device.crash") + logRecord.Attributes().PutStr("exception.stacktrace", javaStacktrace) + // Set all attributes that enrichment would normally set + logRecord.Attributes().PutStr("event.kind", "existing-event-kind") + logRecord.Attributes().PutStr("processor.event", "existing-processor-event") + logRecord.Attributes().PutInt("timestamp.us", int64(99999)) + logRecord.Attributes().PutStr("error.id", "0123456789abcdef0123456789abcdef") + logRecord.Attributes().PutStr("error.type", "existing-error-type") + logRecord.Attributes().PutStr("error.grouping_key", "existing-grouping-key") + return logRecord + }, + expectedAttributes: map[string]any{ + "event.name": "device.crash", + "exception.stacktrace": javaStacktrace, + // existing attributes that are not overridden + "event.kind": "existing-event-kind", + "processor.event": "existing-processor-event", + "timestamp.us": int64(99999), + "error.id": "0123456789abcdef0123456789abcdef", + "error.type": "existing-error-type", + "error.grouping_key": "existing-grouping-key", + }, + }, + { + name: "crash_event_some_attributes_missing", + eventName: "device.crash", + resourceAttrs: map[string]any{ + "telemetry.sdk.language": "java", + }, + input: func() plog.LogRecord { + logRecord := plog.NewLogRecord() + logRecord.SetTimestamp(timestamp) + logRecord.Attributes().PutStr("event.name", "device.crash") + logRecord.Attributes().PutStr("exception.stacktrace", javaStacktrace) + logRecord.Attributes().PutStr("event.kind", "existing-event-kind") + logRecord.Attributes().PutStr("processor.event", "existing-processor-event") + // timestamp.us, error.id, error.type, and error.grouping_key are missing + return logRecord + }, + expectedAttributes: map[string]any{ + // Input attributes + "event.name": "device.crash", + "exception.stacktrace": javaStacktrace, + // existing attributes that are not overridden + "event.kind": "existing-event-kind", + "processor.event": "existing-processor-event", + // attributes that are added by enrichment + "timestamp.us": timestamp.AsTime().UnixMicro(), + "error.grouping_key": javaStacktraceHash, + "error.type": "crash", + }, + }, } { t.Run(tc.name, func(t *testing.T) { inputLogRecord := tc.input() diff --git a/enrichments/internal/elastic/resource.go b/enrichments/internal/elastic/resource.go index 9a560ac..ad87ffa 100644 --- a/enrichments/internal/elastic/resource.go +++ b/enrichments/internal/elastic/resource.go @@ -26,6 +26,7 @@ import ( "github.com/elastic/opentelemetry-lib/elasticattr" "github.com/elastic/opentelemetry-lib/enrichments/config" + "github.com/elastic/opentelemetry-lib/enrichments/internal/attribute" ) // EnrichResource derives and adds Elastic specific resource attributes. @@ -83,14 +84,10 @@ func (s *resourceEnrichmentContext) Enrich(resource pcommon.Resource, cfg config // agent.name and version are set by classic Elastic APM Agents - if the value is present, we take it // otherwise the setAgent[Name|Version] functions are called to derive the values if cfg.AgentName.Enabled { - if _, exists := resource.Attributes().Get(elasticattr.AgentName); !exists { - s.setAgentName(resource) - } + s.setAgentName(resource) } if cfg.AgentVersion.Enabled { - if _, exists := resource.Attributes().Get(elasticattr.AgentVersion); !exists { - s.setAgentVersion(resource) - } + s.setAgentVersion(resource) } if cfg.OverrideHostName.Enabled { @@ -111,7 +108,8 @@ func (s *resourceEnrichmentContext) Enrich(resource pcommon.Resource, cfg config // we duplicate the value and also send it with the old field name to make the alias work. func (s *resourceEnrichmentContext) setDeploymentEnvironment(resource pcommon.Resource) { if s.deploymentEnvironmentName != "" && s.deploymentEnvironment == "" { - resource.Attributes().PutStr( + attribute.PutStr( + resource.Attributes(), string(semconv25.DeploymentEnvironmentKey), s.deploymentEnvironmentName, ) @@ -142,7 +140,7 @@ func (s *resourceEnrichmentContext) setAgentName(resource pcommon.Resource) { s.telemetrySDKLanguage, ) } - resource.Attributes().PutStr(elasticattr.AgentName, agentName) + attribute.PutStr(resource.Attributes(), elasticattr.AgentName, agentName) } func (s *resourceEnrichmentContext) setAgentVersion(resource pcommon.Resource) { @@ -157,7 +155,7 @@ func (s *resourceEnrichmentContext) setAgentVersion(resource pcommon.Resource) { case s.telemetrySDKVersion != "": agentVersion = s.telemetrySDKVersion } - resource.Attributes().PutStr(elasticattr.AgentVersion, agentVersion) + attribute.PutStr(resource.Attributes(), elasticattr.AgentVersion, agentVersion) } func (s *resourceEnrichmentContext) overrideHostNameWithK8sNodeName(resource pcommon.Resource) { @@ -174,14 +172,9 @@ func (s *resourceEnrichmentContext) overrideHostNameWithK8sNodeName(resource pco ) } -// setServiceInstanceID sets service.instance.id from container.id or host.name -// if service.instance.id is not already set. This follows the existing APM logic for -// `service.node.name`. +// setServiceInstanceID sets service.instance.id from container.id or host.name. +// This follows the existing APM logic for `service.node.name`. func (s *resourceEnrichmentContext) setServiceInstanceID(resource pcommon.Resource) { - if s.serviceInstanceID != "" { - return - } - switch { case s.containerID != "": s.serviceInstanceID = s.containerID @@ -191,5 +184,5 @@ func (s *resourceEnrichmentContext) setServiceInstanceID(resource pcommon.Resour // no instance id could be derived return } - resource.Attributes().PutStr(string(semconv25.ServiceInstanceIDKey), s.serviceInstanceID) + attribute.PutStr(resource.Attributes(), string(semconv25.ServiceInstanceIDKey), s.serviceInstanceID) } diff --git a/enrichments/internal/elastic/resource_test.go b/enrichments/internal/elastic/resource_test.go index c23bc7f..41ff0e2 100644 --- a/enrichments/internal/elastic/resource_test.go +++ b/enrichments/internal/elastic/resource_test.go @@ -251,7 +251,6 @@ func TestResourceEnrich(t *testing.T) { name: "service_instance_id_derived_from_container_id", input: func() pcommon.Resource { res := pcommon.NewResource() - res.Attributes().PutStr(string(semconv.ServiceInstanceIDKey), "") res.Attributes().PutStr(string(semconv25.ContainerIDKey), "container-id") res.Attributes().PutStr(string(semconv25.HostNameKey), "k8s-node") return res @@ -269,7 +268,6 @@ func TestResourceEnrich(t *testing.T) { name: "service_instance_id_derived_from_host_name", input: func() pcommon.Resource { res := pcommon.NewResource() - res.Attributes().PutStr(string(semconv.ServiceInstanceIDKey), "") res.Attributes().PutStr(string(semconv25.HostNameKey), "k8s-node") return res }(), @@ -299,6 +297,32 @@ func TestResourceEnrich(t *testing.T) { elasticattr.AgentVersion: "unknown", }, }, + { + name: "all_existing_attributes_preserved", + input: func() pcommon.Resource { + res := pcommon.NewResource() + res.Attributes().PutStr(elasticattr.AgentName, "existing-agent-name") + res.Attributes().PutStr(elasticattr.AgentVersion, "existing-agent-version") + res.Attributes().PutStr(string(semconv25.ServiceInstanceIDKey), "existing-service-instance-id") + res.Attributes().PutStr(string(semconv.TelemetrySDKNameKey), "customflavor") + res.Attributes().PutStr(string(semconv.TelemetrySDKVersionKey), "9.999.9") + res.Attributes().PutStr(string(semconv25.ContainerIDKey), "container-id") + res.Attributes().PutStr(string(semconv.HostNameKey), "host-name") + return res + }(), + config: config.Enabled().Resource, + enrichedAttrs: map[string]any{ + // existing attributes are preserved (not overwritten) + elasticattr.AgentName: "existing-agent-name", + elasticattr.AgentVersion: "existing-agent-version", + string(semconv25.ServiceInstanceIDKey): "existing-service-instance-id", + // source attributes remain unchanged + string(semconv.TelemetrySDKNameKey): "customflavor", + string(semconv.TelemetrySDKVersionKey): "9.999.9", + string(semconv25.ContainerIDKey): "container-id", + string(semconv.HostNameKey): "host-name", + }, + }, } { t.Run(tc.name, func(t *testing.T) { // Merge existing resource attrs with the attrs added diff --git a/enrichments/internal/elastic/scope.go b/enrichments/internal/elastic/scope.go index b9358b9..d649e81 100644 --- a/enrichments/internal/elastic/scope.go +++ b/enrichments/internal/elastic/scope.go @@ -20,6 +20,7 @@ package elastic import ( "github.com/elastic/opentelemetry-lib/elasticattr" "github.com/elastic/opentelemetry-lib/enrichments/config" + "github.com/elastic/opentelemetry-lib/enrichments/internal/attribute" "go.opentelemetry.io/collector/pdata/pcommon" ) @@ -28,8 +29,8 @@ func EnrichScope(scope pcommon.InstrumentationScope, cfg config.Config) { attrs := scope.Attributes() if cfg.Scope.ServiceFrameworkName.Enabled { if name := scope.Name(); name != "" { - attrs.PutStr(elasticattr.ServiceFrameworkName, name) - attrs.PutStr(elasticattr.ServiceFrameworkVersion, scope.Version()) + attribute.PutStr(attrs, elasticattr.ServiceFrameworkName, name) + attribute.PutStr(attrs, elasticattr.ServiceFrameworkVersion, scope.Version()) } } } diff --git a/enrichments/internal/elastic/scope_test.go b/enrichments/internal/elastic/scope_test.go index c25f5ba..f8435c7 100644 --- a/enrichments/internal/elastic/scope_test.go +++ b/enrichments/internal/elastic/scope_test.go @@ -66,6 +66,22 @@ func TestScopeEnrich(t *testing.T) { elasticattr.ServiceFrameworkVersion: "v1.0.0", }, }, + { + name: "existing_attributes_not_overridden", + input: func() pcommon.InstrumentationScope { + scope := pcommon.NewInstrumentationScope() + scope.SetName("test") + scope.SetVersion("v1.0.0") + scope.Attributes().PutStr(elasticattr.ServiceFrameworkName, "existing-framework-name") + scope.Attributes().PutStr(elasticattr.ServiceFrameworkVersion, "existing-framework-version") + return scope + }(), + config: config.Enabled().Scope, + enrichedAttrs: map[string]any{ + elasticattr.ServiceFrameworkName: "existing-framework-name", + elasticattr.ServiceFrameworkVersion: "existing-framework-version", + }, + }, } { t.Run(tc.name, func(t *testing.T) { // Merge existing resource attrs with the attrs added diff --git a/enrichments/internal/elastic/span.go b/enrichments/internal/elastic/span.go index 94e7772..79f20b3 100644 --- a/enrichments/internal/elastic/span.go +++ b/enrichments/internal/elastic/span.go @@ -41,6 +41,7 @@ import ( "github.com/elastic/opentelemetry-lib/elasticattr" "github.com/elastic/opentelemetry-lib/enrichments/config" + "github.com/elastic/opentelemetry-lib/enrichments/internal/attribute" ) // defaultRepresentativeCount is the representative count to use for adjusting @@ -84,6 +85,7 @@ type spanEnrichmentContext struct { messagingDestinationName string genAiSystem string typeValue string + transactionType string // The inferred* attributes are derived from a base attribute userAgentOriginal string @@ -210,6 +212,8 @@ func (s *spanEnrichmentContext) Enrich( s.userAgentVersion = v.Str() case "type": s.typeValue = v.Str() + case elasticattr.TransactionType: + s.transactionType = v.Str() } return true }) @@ -245,32 +249,32 @@ func (s *spanEnrichmentContext) enrichTransaction( cfg config.ElasticTransactionConfig, ) { if cfg.TimestampUs.Enabled { - span.Attributes().PutInt(elasticattr.TimestampUs, getTimestampUs(span.StartTimestamp())) + attribute.PutInt(span.Attributes(), elasticattr.TimestampUs, getTimestampUs(span.StartTimestamp())) } if cfg.Sampled.Enabled { - span.Attributes().PutBool(elasticattr.TransactionSampled, s.getSampled()) + attribute.PutBool(span.Attributes(), elasticattr.TransactionSampled, s.getSampled()) } if cfg.ID.Enabled { - span.Attributes().PutStr(elasticattr.TransactionID, span.SpanID().String()) + attribute.PutStr(span.Attributes(), elasticattr.TransactionID, span.SpanID().String()) } if cfg.Root.Enabled { - span.Attributes().PutBool(elasticattr.TransactionRoot, isTraceRoot(span)) + attribute.PutBool(span.Attributes(), elasticattr.TransactionRoot, isTraceRoot(span)) } if cfg.Name.Enabled { - span.Attributes().PutStr(elasticattr.TransactionName, span.Name()) + attribute.PutStr(span.Attributes(), elasticattr.TransactionName, span.Name()) } if cfg.ProcessorEvent.Enabled { - span.Attributes().PutStr(elasticattr.ProcessorEvent, "transaction") + attribute.PutStr(span.Attributes(), elasticattr.ProcessorEvent, "transaction") } if cfg.RepresentativeCount.Enabled { repCount := getRepresentativeCount(span.TraceState().AsRaw()) - span.Attributes().PutDouble(elasticattr.TransactionRepresentativeCount, repCount) + attribute.PutDouble(span.Attributes(), elasticattr.TransactionRepresentativeCount, repCount) } if cfg.DurationUs.Enabled { - span.Attributes().PutInt(elasticattr.TransactionDurationUs, getDurationUs(span)) + attribute.PutInt(span.Attributes(), elasticattr.TransactionDurationUs, getDurationUs(span)) } if cfg.Type.Enabled { - span.Attributes().PutStr(elasticattr.TransactionType, s.getTxnType()) + attribute.PutStr(span.Attributes(), elasticattr.TransactionType, s.getTxnType()) } if cfg.Result.Enabled { s.setTxnResult(span) @@ -295,14 +299,14 @@ func (s *spanEnrichmentContext) enrichSpan( var spanType, spanSubtype string if cfg.TimestampUs.Enabled { - span.Attributes().PutInt(elasticattr.TimestampUs, getTimestampUs(span.StartTimestamp())) + attribute.PutInt(span.Attributes(), elasticattr.TimestampUs, getTimestampUs(span.StartTimestamp())) } if cfg.Name.Enabled { - span.Attributes().PutStr(elasticattr.SpanName, span.Name()) + attribute.PutStr(span.Attributes(), elasticattr.SpanName, span.Name()) } if cfg.RepresentativeCount.Enabled { repCount := getRepresentativeCount(span.TraceState().AsRaw()) - span.Attributes().PutDouble(elasticattr.SpanRepresentativeCount, repCount) + attribute.PutDouble(span.Attributes(), elasticattr.SpanRepresentativeCount, repCount) } if cfg.TypeSubtype.Enabled { spanType, spanSubtype = s.setSpanTypeSubtype(span) @@ -311,7 +315,7 @@ func (s *spanEnrichmentContext) enrichSpan( s.setEventOutcome(span) } if cfg.DurationUs.Enabled { - span.Attributes().PutInt(elasticattr.SpanDurationUs, getDurationUs(span)) + attribute.PutInt(span.Attributes(), elasticattr.SpanDurationUs, getDurationUs(span)) } if cfg.ServiceTarget.Enabled { s.setServiceTarget(span) @@ -323,13 +327,16 @@ func (s *spanEnrichmentContext) enrichSpan( s.setInferredSpans(span) } if cfg.ProcessorEvent.Enabled && !isExitRootSpan { - span.Attributes().PutStr(elasticattr.ProcessorEvent, "span") + attribute.PutStr(span.Attributes(), elasticattr.ProcessorEvent, "span") } if cfg.UserAgent.Enabled { s.setUserAgentIfRequired(span) } - if isExitRootSpan && transactionTypeEnabled && s.typeValue == "" { + // The transaction type should not be updated if it was originally provided (s.transactionType is not empty) + // Prior enrichment logic may have set this value by using `s.getTxnType()`, in this + // case it is okay to update the transaction type with a more specific value. + if isExitRootSpan && transactionTypeEnabled && s.typeValue == "" && s.transactionType == "" { if spanType != "" { transactionType := spanType if spanSubtype != "" { @@ -395,7 +402,7 @@ func (s *spanEnrichmentContext) setTxnResult(span ptrace.Span) { } } - span.Attributes().PutStr(elasticattr.TransactionResult, result) + attribute.PutStr(span.Attributes(), elasticattr.TransactionResult, result) } func (s *spanEnrichmentContext) setEventOutcome(span ptrace.Span) { @@ -414,8 +421,9 @@ func (s *spanEnrichmentContext) setEventOutcome(span ptrace.Span) { outcome = "failure" successCount = 0 } - span.Attributes().PutStr(elasticattr.EventOutcome, outcome) - span.Attributes().PutInt(elasticattr.SuccessCount, int64(successCount)) + + attribute.PutStr(span.Attributes(), elasticattr.EventOutcome, outcome) + attribute.PutInt(span.Attributes(), elasticattr.SuccessCount, int64(successCount)) } func (s *spanEnrichmentContext) setSpanTypeSubtype(span ptrace.Span) (spanType string, spanSubtype string) { @@ -445,14 +453,9 @@ func (s *spanEnrichmentContext) setSpanTypeSubtype(span ptrace.Span) (spanType s } } - // do not overwrite existing span.type and span.subtype attributes - if existingSpanType, _ := span.Attributes().Get(elasticattr.SpanType); existingSpanType.Str() == "" { - span.Attributes().PutStr(elasticattr.SpanType, spanType) - } + attribute.PutStr(span.Attributes(), elasticattr.SpanType, spanType) if spanSubtype != "" { - if existingSpanSubtype, _ := span.Attributes().Get(elasticattr.SpanSubtype); existingSpanSubtype.Str() == "" { - span.Attributes().PutStr(elasticattr.SpanSubtype, spanSubtype) - } + attribute.PutStr(span.Attributes(), elasticattr.SpanSubtype, spanSubtype) } return spanType, spanSubtype @@ -502,13 +505,8 @@ func (s *spanEnrichmentContext) setServiceTarget(span ptrace.Span) { // set either target.type or target.name if at least one is available if targetType != "" || targetName != "" { - // do not overwrite existing target.type and target.name attributes - if existingTargetType, _ := span.Attributes().Get(elasticattr.ServiceTargetType); existingTargetType.Str() == "" { - span.Attributes().PutStr(elasticattr.ServiceTargetType, targetType) - } - if existingTargetName, _ := span.Attributes().Get(elasticattr.ServiceTargetName); existingTargetName.Str() == "" { - span.Attributes().PutStr(elasticattr.ServiceTargetName, targetName) - } + attribute.PutStr(span.Attributes(), elasticattr.ServiceTargetType, targetType) + attribute.PutStr(span.Attributes(), elasticattr.ServiceTargetName, targetName) } } @@ -548,14 +546,15 @@ func (s *spanEnrichmentContext) setDestinationService(span ptrace.Span) { } if destnResource != "" { - // do not overwrite existing span.destination.service.resource attribute - if existingDestnResource, _ := span.Attributes().Get(elasticattr.SpanDestinationServiceResource); existingDestnResource.Str() == "" { - span.Attributes().PutStr(elasticattr.SpanDestinationServiceResource, destnResource) - } + attribute.PutStr(span.Attributes(), elasticattr.SpanDestinationServiceResource, destnResource) } } func (s *spanEnrichmentContext) setInferredSpans(span ptrace.Span) { + if _, exists := span.Attributes().Get(elasticattr.ChildIDs); exists { + return + } + spanLinks := span.Links() childIDs := pcommon.NewSlice() spanLinks.RemoveIf(func(spanLink ptrace.SpanLink) (remove bool) { @@ -581,13 +580,15 @@ func (s *spanEnrichmentContext) setInferredSpans(span ptrace.Span) { func (s *spanEnrichmentContext) setUserAgentIfRequired(span ptrace.Span) { if s.userAgentName == "" && s.inferredUserAgentName != "" { - span.Attributes().PutStr( + attribute.PutStr( + span.Attributes(), string(semconv27.UserAgentNameKey), s.inferredUserAgentName, ) } if s.userAgentVersion == "" && s.inferredUserAgentVersion != "" { - span.Attributes().PutStr( + attribute.PutStr( + span.Attributes(), string(semconv27.UserAgentVersionKey), s.inferredUserAgentVersion, ) @@ -625,10 +626,10 @@ func (s *spanEventEnrichmentContext) enrich( // Enrich span event attributes. if cfg.TimestampUs.Enabled { - se.Attributes().PutInt(elasticattr.TimestampUs, getTimestampUs(se.Timestamp())) + attribute.PutInt(se.Attributes(), elasticattr.TimestampUs, getTimestampUs(se.Timestamp())) } if cfg.ProcessorEvent.Enabled && s.exception { - se.Attributes().PutStr(elasticattr.ProcessorEvent, "error") + attribute.PutStr(se.Attributes(), elasticattr.ProcessorEvent, "error") } if s.exceptionType == "" && s.exceptionMessage == "" { // Span event does not represent an exception @@ -638,11 +639,11 @@ func (s *spanEventEnrichmentContext) enrich( // Span event represents exception if cfg.ErrorID.Enabled { if id, err := newUniqueID(); err == nil { - se.Attributes().PutStr(elasticattr.ErrorID, id) + attribute.PutStr(se.Attributes(), elasticattr.ErrorID, id) } } if cfg.ErrorExceptionHandled.Enabled { - se.Attributes().PutBool(elasticattr.ErrorExceptionHandled, !s.exceptionEscaped) + attribute.PutBool(se.Attributes(), elasticattr.ErrorExceptionHandled, !s.exceptionEscaped) } if cfg.ErrorGroupingKey.Enabled { // See https://github.com/elastic/apm-data/issues/299 @@ -653,21 +654,19 @@ func (s *spanEventEnrichmentContext) enrich( } else if s.exceptionMessage != "" { io.WriteString(hash, s.exceptionMessage) } - se.Attributes().PutStr(elasticattr.ErrorGroupingKey, hex.EncodeToString(hash.Sum(nil))) + attribute.PutStr(se.Attributes(), elasticattr.ErrorGroupingKey, hex.EncodeToString(hash.Sum(nil))) } - if cfg.ErrorGroupingName.Enabled { - if s.exceptionMessage != "" { - se.Attributes().PutStr(elasticattr.ErrorGroupingName, s.exceptionMessage) - } + if cfg.ErrorGroupingName.Enabled && s.exceptionMessage != "" { + attribute.PutStr(se.Attributes(), elasticattr.ErrorGroupingName, s.exceptionMessage) } // Transaction type and sampled are added as span event enrichment only for errors if parentCtx.isTransaction && s.exception { if cfg.TransactionSampled.Enabled { - se.Attributes().PutBool(elasticattr.TransactionSampled, parentCtx.getSampled()) + attribute.PutBool(se.Attributes(), elasticattr.TransactionSampled, parentCtx.getSampled()) } if cfg.TransactionType.Enabled { - se.Attributes().PutStr(elasticattr.TransactionType, parentCtx.getTxnType()) + attribute.PutStr(se.Attributes(), elasticattr.TransactionType, parentCtx.getTxnType()) } } } @@ -732,7 +731,15 @@ func isTraceRoot(span ptrace.Span) bool { func isElasticTransaction(span ptrace.Span) bool { flags := tracepb.SpanFlags(span.Flags()) + + // Events may have already been defined as an elastic transaction. + // check the processor.event value to avoid incorrectly classifying + // a span. + processorEvent, _ := span.Attributes().Get(elasticattr.ProcessorEvent) + switch { + case processorEvent.Str() == "transaction": + return true case isTraceRoot(span): return true case (flags & tracepb.SpanFlags_SPAN_FLAGS_CONTEXT_HAS_IS_REMOTE_MASK) == 0: diff --git a/enrichments/internal/elastic/span_test.go b/enrichments/internal/elastic/span_test.go index 8c574a8..9456dfb 100644 --- a/enrichments/internal/elastic/span_test.go +++ b/enrichments/internal/elastic/span_test.go @@ -229,6 +229,31 @@ func TestElasticTransactionEnrich(t *testing.T) { elasticattr.TransactionType: "request", }, }, + { + name: "http_status_ok_processor_event_set", + input: func() ptrace.Span { + span := getElasticTxn() + span.SetName("testtxn") + span.Attributes().PutInt(string(semconv25.HTTPStatusCodeKey), http.StatusOK) + span.Attributes().PutStr(elasticattr.ProcessorEvent, "transaction") + return span + }(), + config: config.Enabled().Transaction, + enrichedAttrs: map[string]any{ + elasticattr.TimestampUs: startTs.AsTime().UnixMicro(), + elasticattr.TransactionSampled: true, + elasticattr.TransactionRoot: true, + elasticattr.TransactionID: "0100000000000000", + elasticattr.TransactionName: "testtxn", + elasticattr.ProcessorEvent: "transaction", + elasticattr.TransactionRepresentativeCount: float64(1), + elasticattr.TransactionDurationUs: expectedDuration.Microseconds(), + elasticattr.EventOutcome: "success", + elasticattr.SuccessCount: int64(1), + elasticattr.TransactionResult: "HTTP 2xx", + elasticattr.TransactionType: "request", + }, + }, { name: "http_status_1xx", input: func() ptrace.Span { @@ -464,6 +489,59 @@ func TestElasticTransactionEnrich(t *testing.T) { return &spanLinks }(), }, + { + name: "inferred_spans_with_existing_child_ids", + input: func() ptrace.Span { + span := getElasticTxn() + span.SetName("testtxn") + span.SetSpanID([8]byte{1}) + // Set existing child.ids attribute + existingChildIDs := span.Attributes().PutEmptySlice(elasticattr.ChildIDs) + existingChildIDs.AppendEmpty().SetStr("existing-child-id-1") + existingChildIDs.AppendEmpty().SetStr("existing-child-id-2") + + normalLink := span.Links().AppendEmpty() + normalLink.SetSpanID([8]byte{2}) + + childLink := span.Links().AppendEmpty() + childLink.SetSpanID([8]byte{3}) + childLink.Attributes().PutBool("is_child", true) + + childLink2 := span.Links().AppendEmpty() + childLink2.SetSpanID([8]byte{4}) + childLink2.Attributes().PutBool("elastic.is_child", true) + return span + }(), + config: config.Enabled().Transaction, + enrichedAttrs: map[string]any{ + elasticattr.TimestampUs: startTs.AsTime().UnixMicro(), + elasticattr.TransactionSampled: true, + elasticattr.TransactionRoot: true, + elasticattr.TransactionID: "0100000000000000", + elasticattr.TransactionName: "testtxn", + elasticattr.ProcessorEvent: "transaction", + elasticattr.TransactionRepresentativeCount: float64(1), + elasticattr.TransactionDurationUs: expectedDuration.Microseconds(), + elasticattr.EventOutcome: "success", + elasticattr.SuccessCount: int64(1), + elasticattr.TransactionResult: "Success", + elasticattr.TransactionType: "unknown", + elasticattr.ChildIDs: []any{"existing-child-id-1", "existing-child-id-2"}, + }, + expectedSpanLinks: func() *ptrace.SpanLinkSlice { + spanLinks := ptrace.NewSpanLinkSlice() + // span links should remain since child.ids were not overwritten + normalLink := spanLinks.AppendEmpty() + normalLink.SetSpanID([8]byte{2}) + childLink := spanLinks.AppendEmpty() + childLink.SetSpanID([8]byte{3}) + childLink.Attributes().PutBool("is_child", true) + childLink2 := spanLinks.AppendEmpty() + childLink2.SetSpanID([8]byte{4}) + childLink2.Attributes().PutBool("elastic.is_child", true) + return &spanLinks + }(), + }, { name: "user_agent_parse_name_version", input: func() ptrace.Span { @@ -643,6 +721,85 @@ func TestRootSpanAsDependencyEnrich(t *testing.T) { elasticattr.SpanRepresentativeCount: float64(1), }, }, + { + name: "outgoing_http_root_span_with_existing_transaction_type", + input: func() ptrace.Span { + span := ptrace.NewSpan() + span.SetName("rootClientSpan") + span.SetSpanID([8]byte{1}) + span.SetKind(ptrace.SpanKindClient) + span.Attributes().PutStr(string(semconv27.HTTPRequestMethodKey), "GET") + span.Attributes().PutStr(string(semconv27.URLFullKey), "http://localhost:8080") + span.Attributes().PutInt(string(semconv27.HTTPResponseStatusCodeKey), 200) + span.Attributes().PutStr(string(semconv27.NetworkProtocolVersionKey), "1.1") + span.Attributes().PutStr(elasticattr.TransactionType, "existing-transaction-type") + return span + }(), + config: config.Enabled(), + enrichedAttrs: map[string]any{ + elasticattr.TimestampUs: int64(0), + elasticattr.TransactionName: "rootClientSpan", + elasticattr.ProcessorEvent: "transaction", + elasticattr.SpanType: "external", + elasticattr.SpanSubtype: "http", + elasticattr.SpanDestinationServiceResource: "localhost:8080", + elasticattr.SpanName: "rootClientSpan", + elasticattr.EventOutcome: "success", + elasticattr.SuccessCount: int64(1), + elasticattr.ServiceTargetName: "localhost:8080", + elasticattr.ServiceTargetType: "http", + elasticattr.TransactionID: "0100000000000000", + elasticattr.TransactionDurationUs: int64(0), + elasticattr.TransactionRepresentativeCount: float64(1), + elasticattr.TransactionResult: "HTTP 2xx", + // transaction.type should not be updated + elasticattr.TransactionType: "existing-transaction-type", + elasticattr.TransactionSampled: true, + elasticattr.TransactionRoot: true, + elasticattr.SpanDurationUs: int64(0), + elasticattr.SpanRepresentativeCount: float64(1), + }, + }, + { + name: "outgoing_http_root_span_with_user_provided_type_attribute", + input: func() ptrace.Span { + span := ptrace.NewSpan() + span.SetName("rootClientSpan") + span.SetSpanID([8]byte{1}) + span.SetKind(ptrace.SpanKindClient) + span.Attributes().PutStr(string(semconv27.HTTPRequestMethodKey), "GET") + span.Attributes().PutStr(string(semconv27.URLFullKey), "http://localhost:8080") + span.Attributes().PutInt(string(semconv27.HTTPResponseStatusCodeKey), 200) + span.Attributes().PutStr(string(semconv27.NetworkProtocolVersionKey), "1.1") + // User provided "type" attribute (not transaction.type directly) + span.Attributes().PutStr("type", "user-provided-type") + return span + }(), + config: config.Enabled(), + enrichedAttrs: map[string]any{ + elasticattr.TimestampUs: int64(0), + elasticattr.TransactionName: "rootClientSpan", + elasticattr.ProcessorEvent: "transaction", + elasticattr.SpanType: "external", + elasticattr.SpanSubtype: "http", + elasticattr.SpanDestinationServiceResource: "localhost:8080", + elasticattr.SpanName: "rootClientSpan", + elasticattr.EventOutcome: "success", + elasticattr.SuccessCount: int64(1), + elasticattr.ServiceTargetName: "localhost:8080", + elasticattr.ServiceTargetType: "http", + elasticattr.TransactionID: "0100000000000000", + elasticattr.TransactionDurationUs: int64(0), + elasticattr.TransactionRepresentativeCount: float64(1), + elasticattr.TransactionResult: "HTTP 2xx", + // transaction.type should be set from "type" attribute, not overridden by exit root span logic + elasticattr.TransactionType: "user-provided-type", + elasticattr.TransactionSampled: true, + elasticattr.TransactionRoot: true, + elasticattr.SpanDurationUs: int64(0), + elasticattr.SpanRepresentativeCount: float64(1), + }, + }, { name: "db_root_span", input: func() ptrace.Span { @@ -680,6 +837,42 @@ func TestRootSpanAsDependencyEnrich(t *testing.T) { elasticattr.SpanRepresentativeCount: float64(1), }, }, + { + name: "db_root_span_transaction_type_enrichment", + input: func() ptrace.Span { + span := ptrace.NewSpan() + span.SetName("rootClientSpan") + span.SetSpanID([8]byte{1}) + span.SetKind(ptrace.SpanKindClient) + span.Attributes().PutStr(string(semconv25.DBSystemKey), "postgresql") + span.Attributes().PutStr(string(semconv25.DBNameKey), "myDb") + return span + }(), + config: config.Enabled(), + enrichedAttrs: map[string]any{ + elasticattr.TimestampUs: int64(0), + elasticattr.TransactionName: "rootClientSpan", + elasticattr.ProcessorEvent: "transaction", + elasticattr.SpanType: "db", + elasticattr.SpanSubtype: "postgresql", + elasticattr.SpanDestinationServiceResource: "postgresql", + elasticattr.SpanName: "rootClientSpan", + elasticattr.EventOutcome: "success", + elasticattr.SuccessCount: int64(1), + elasticattr.ServiceTargetName: "myDb", + elasticattr.ServiceTargetType: "postgresql", + elasticattr.TransactionID: "0100000000000000", + elasticattr.TransactionDurationUs: int64(0), + elasticattr.TransactionRepresentativeCount: float64(1), + elasticattr.TransactionResult: "Success", + // enrichment sets transaction.type value + elasticattr.TransactionType: "db.postgresql", + elasticattr.TransactionSampled: true, + elasticattr.TransactionRoot: true, + elasticattr.SpanDurationUs: int64(0), + elasticattr.SpanRepresentativeCount: float64(1), + }, + }, { name: "producer_messaging_span", input: func() ptrace.Span { @@ -1461,6 +1654,55 @@ func TestElasticSpanEnrich(t *testing.T) { return &spanLinks }(), }, + { + name: "inferred_spans_with_existing_child_ids", + input: func() ptrace.Span { + span := getElasticSpan() + span.SetName("testspan") + span.SetSpanID([8]byte{1}) + // set existing child.ids attribute + existingChildIDs := span.Attributes().PutEmptySlice(elasticattr.ChildIDs) + existingChildIDs.AppendEmpty().SetStr("existing-child-id-1") + existingChildIDs.AppendEmpty().SetStr("existing-child-id-2") + + normalLink := span.Links().AppendEmpty() + normalLink.SetSpanID([8]byte{2}) + + childLink := span.Links().AppendEmpty() + childLink.SetSpanID([8]byte{3}) + childLink.Attributes().PutBool("is_child", true) + + childLink2 := span.Links().AppendEmpty() + childLink2.SetSpanID([8]byte{4}) + childLink2.Attributes().PutBool("elastic.is_child", true) + return span + }(), + config: config.Enabled().Span, + enrichedAttrs: map[string]any{ + elasticattr.TimestampUs: startTs.AsTime().UnixMicro(), + elasticattr.SpanName: "testspan", + elasticattr.ProcessorEvent: "span", + elasticattr.SpanRepresentativeCount: float64(1), + elasticattr.SpanType: "unknown", + elasticattr.SpanDurationUs: expectedDuration.Microseconds(), + elasticattr.EventOutcome: "success", + elasticattr.SuccessCount: int64(1), + elasticattr.ChildIDs: []any{"existing-child-id-1", "existing-child-id-2"}, + }, + expectedSpanLinks: func() *ptrace.SpanLinkSlice { + spanLinks := ptrace.NewSpanLinkSlice() + // links should remain since child.ids was not overwritten + normalLink := spanLinks.AppendEmpty() + normalLink.SetSpanID([8]byte{2}) + childLink := spanLinks.AppendEmpty() + childLink.SetSpanID([8]byte{3}) + childLink.Attributes().PutBool("is_child", true) + childLink2 := spanLinks.AppendEmpty() + childLink2.SetSpanID([8]byte{4}) + childLink2.Attributes().PutBool("elastic.is_child", true) + return &spanLinks + }(), + }, { name: "genai_with_system", input: func() ptrace.Span { @@ -1613,6 +1855,84 @@ func TestElasticSpanEnrich(t *testing.T) { string(semconv27.UserAgentVersionKey): "51.0.2704", }, }, + { + name: "span_existing_attributes_not_overridden", + input: func() ptrace.Span { + span := getElasticSpan() + span.SetName("testspan") + span.Attributes().PutInt(elasticattr.TimestampUs, 1234567890) + span.Attributes().PutStr(elasticattr.SpanName, "existing-span-name") + span.Attributes().PutStr(elasticattr.ProcessorEvent, "existing-event") + span.Attributes().PutDouble(elasticattr.SpanRepresentativeCount, 99.5) + span.Attributes().PutInt(elasticattr.SpanDurationUs, 999999) + span.Attributes().PutStr(elasticattr.SpanType, "existing-type") + span.Attributes().PutStr(elasticattr.SpanSubtype, "existing-subtype") + span.Attributes().PutStr(elasticattr.ServiceTargetType, "existing-target-type") + span.Attributes().PutStr(elasticattr.ServiceTargetName, "existing-target-name") + span.Attributes().PutStr(elasticattr.SpanDestinationServiceResource, "existing-destination") + span.Attributes().PutStr(elasticattr.EventOutcome, "existing-outcome") + span.Attributes().PutInt(elasticattr.SuccessCount, 99) + // attributes needed for enrichment logic to run + span.Attributes().PutStr(string(semconv25.PeerServiceKey), "testsvc") + span.Attributes().PutInt(string(semconv25.HTTPResponseStatusCodeKey), http.StatusOK) + return span + }(), + config: config.Enabled().Span, + enrichedAttrs: map[string]any{ + elasticattr.TimestampUs: int64(1234567890), + elasticattr.SpanName: "existing-span-name", + elasticattr.ProcessorEvent: "existing-event", + elasticattr.SpanRepresentativeCount: float64(99.5), + elasticattr.SpanDurationUs: int64(999999), + elasticattr.SpanType: "existing-type", + elasticattr.SpanSubtype: "existing-subtype", + elasticattr.ServiceTargetType: "existing-target-type", + elasticattr.ServiceTargetName: "existing-target-name", + elasticattr.SpanDestinationServiceResource: "existing-destination", + elasticattr.EventOutcome: "existing-outcome", + elasticattr.SuccessCount: int64(99), + }, + }, + { + name: "transaction_existing_attributes_not_overridden", + input: func() ptrace.Span { + span := ptrace.NewSpan() + span.SetSpanID([8]byte{1}) + span.SetStartTimestamp(startTs) + span.SetEndTimestamp(endTs) + span.SetName("testtxn") + span.Attributes().PutInt(elasticattr.TimestampUs, 1234567890) + span.Attributes().PutBool(elasticattr.TransactionSampled, false) + span.Attributes().PutStr(elasticattr.TransactionID, "existing-txn-id") + span.Attributes().PutBool(elasticattr.TransactionRoot, false) + span.Attributes().PutStr(elasticattr.TransactionName, "existing-txn-name") + span.Attributes().PutStr(elasticattr.ProcessorEvent, "existing-event") + span.Attributes().PutDouble(elasticattr.TransactionRepresentativeCount, 99.5) + span.Attributes().PutInt(elasticattr.TransactionDurationUs, 999999) + span.Attributes().PutStr(elasticattr.TransactionType, "existing-type") + span.Attributes().PutStr(elasticattr.TransactionResult, "existing-result") + span.Attributes().PutStr(elasticattr.EventOutcome, "existing-outcome") + span.Attributes().PutInt(elasticattr.SuccessCount, 99) + // Add attributes needed for enrichment logic to run + span.Attributes().PutInt(string(semconv25.HTTPStatusCodeKey), http.StatusOK) + return span + }(), + config: config.ElasticSpanConfig{}, + enrichedAttrs: map[string]any{ + elasticattr.TimestampUs: int64(1234567890), + elasticattr.TransactionSampled: false, + elasticattr.TransactionID: "existing-txn-id", + elasticattr.TransactionRoot: false, + elasticattr.TransactionName: "existing-txn-name", + elasticattr.ProcessorEvent: "existing-event", + elasticattr.TransactionRepresentativeCount: float64(99.5), + elasticattr.TransactionDurationUs: int64(999999), + elasticattr.TransactionType: "existing-type", + elasticattr.TransactionResult: "existing-result", + elasticattr.EventOutcome: "existing-outcome", + elasticattr.SuccessCount: int64(99), + }, + }, } { t.Run(tc.name, func(t *testing.T) { expectedSpan := ptrace.NewSpan() @@ -1629,9 +1949,16 @@ func TestElasticSpanEnrich(t *testing.T) { expectedSpan.Links().RemoveIf(func(_ ptrace.SpanLink) bool { return true }) } - EnrichSpan(tc.input, config.Config{ + enrichConfig := config.Config{ Span: tc.config, - }, uaparser.NewFromSaved()) + } + // For transaction test case, use Transaction config instead + if tc.name == "transaction_existing_attributes_not_overridden" { + enrichConfig = config.Config{ + Transaction: config.Enabled().Transaction, + } + } + EnrichSpan(tc.input, enrichConfig, uaparser.NewFromSaved()) assert.NoError(t, ptracetest.CompareSpan(expectedSpan, tc.input)) }) } @@ -1844,7 +2171,18 @@ func TestIsElasticTransaction(t *testing.T) { }(), isTxn: false, }, + { + name: "elastic transaction type is already defined", + input: func() ptrace.Span { + span := ptrace.NewSpan() + span.Attributes().PutStr(elasticattr.ProcessorEvent, "transaction") + return span + }(), + isTxn: true, + }, } { - assert.Equal(t, tc.isTxn, isElasticTransaction(tc.input)) + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.isTxn, isElasticTransaction(tc.input)) + }) } } diff --git a/enrichments/internal/logs_test.go b/enrichments/internal/logs_test.go index d55e6f0..3ba5402 100644 --- a/enrichments/internal/logs_test.go +++ b/enrichments/internal/logs_test.go @@ -20,13 +20,17 @@ package elastic import ( "path/filepath" "testing" + "time" - "github.com/elastic/opentelemetry-lib/enrichments" - "github.com/elastic/opentelemetry-lib/enrichments/config" "github.com/google/go-cmp/cmp" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/pdata/pcommon" + + "github.com/elastic/opentelemetry-lib/elasticattr" + "github.com/elastic/opentelemetry-lib/enrichments" + "github.com/elastic/opentelemetry-lib/enrichments/config" ) func TestEnrichResourceLog(t *testing.T) { @@ -81,4 +85,43 @@ func TestEnrichResourceLog(t *testing.T) { } }) } + + t.Run("existing_attributes_not_overridden", func(t *testing.T) { + // Create a new log record with existing attributes + logRecord := logRecords.AppendEmpty() + logRecord.SetEventName("device.crash") + logRecord.SetTimestamp(pcommon.NewTimestampFromTime(time.Unix(12345, 0))) + logRecord.Attributes().PutStr("exception.stacktrace", "test stacktrace") + + // Set existing attributes that enrichment would normally set + existingAttrs := map[string]any{ + elasticattr.EventKind: "existing-event-kind", + elasticattr.ProcessorEvent: "existing-processor-event", + elasticattr.TimestampUs: int64(12345), + elasticattr.ErrorID: "existing-error-id", + elasticattr.ErrorType: "existing-error-type", + elasticattr.ErrorGroupingKey: "existing-grouping-key", + } + + for k, v := range existingAttrs { + logRecord.Attributes().PutEmpty(k).FromRaw(v) + } + + // Store original attributes + originalAttrs := logRecord.Attributes().AsRaw() + + // Enrich the log + enricher := enrichments.NewEnricher(config.Enabled()) + enricher.EnrichLogs(logs) + + // Verify existing attributes are preserved + for k, expectedValue := range existingAttrs { + actualValue, ok := logRecord.Attributes().Get(k) + assert.True(t, ok, "attribute %s should exist", k) + assert.Equal(t, expectedValue, actualValue.AsRaw(), "attribute %s should not be overridden", k) + } + + // Verify the original attributes map is unchanged + assert.Empty(t, cmp.Diff(originalAttrs, logRecord.Attributes().AsRaw())) + }) }