Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .chloggen/43394.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: connector/spanmetrics

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Add `add_resource_attributes` opt-in config option to keep resource attributes in generated metrics

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [43394]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
This configuration option allows users to override the `connector.spanmetrics.excludeResourceMetrics` feature gate
and restore the old behavior of including resource attributes in metrics. This is needed for customers whose
existing dashboards depend on resource attributes being present in the generated metrics.

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []
3 changes: 2 additions & 1 deletion connector/spanmetricsconnector/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,10 @@ The following settings can be optionally configured:
- `events`: Use to configure the events metric.
- `enabled`: (default: `false`): enabling will add the events metric.
- `dimensions`: (mandatory if `enabled`) the list of the span's event attributes to add as dimensions to the `traces.span.metrics.events` metric, which will be included _on top of_ the common and configured `dimensions` for span attributes and resource attributes.
- `resource_metrics_key_attributes`: Filter the resource attributes used to produce the resource metrics key map hash(It's only used to build the hash key, not copy the attributes to metrics resource attributes).
- `resource_metrics_key_attributes`: Filter the resource attributes used to produce the resource metrics key map hash(It's only used to build the hash key, not copy the attributes to metrics resource attributes).
Use this in case changing resource attributes (e.g. process id) are breaking counter metrics.
- `aggregation_cardinality_limit` (default: `0`): Defines the maximum number of unique combinations of dimensions that will be tracked for metrics aggregation. When the limit is reached, additional unique combinations will be dropped but registered under a new entry with `otel.metric.overflow="true"`. A value of `0` means no limit is applied.
- `add_resource_attributes` (default: `false`): Add the resource attributes to the resulting metrics. This option enables the old behavior before the `connector.spanmetrics.excludeResourceMetrics` feature gate was introduced. When set to `true`, resource attributes will be included in the metrics even if the feature gate is enabled. See [GitHub issue #42103](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/42103) for more context.

The feature gate `connector.spanmetrics.legacyMetricNames` (disabled by default) controls the connector to use legacy metric names.

Expand Down
5 changes: 5 additions & 0 deletions connector/spanmetricsconnector/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ type Config struct {
IncludeInstrumentationScope []string `mapstructure:"include_instrumentation_scope"`

AggregationCardinalityLimit int `mapstructure:"aggregation_cardinality_limit"`

// Add the resource attributes to the resulting metrics (disabled by default)
// This option enables the old behavior
// https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/42103
AddResourceAttributes bool `mapstructure:"add_resource_attributes"`
}

type HistogramConfig struct {
Expand Down
2 changes: 1 addition & 1 deletion connector/spanmetricsconnector/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ func (p *connectorImp) buildMetrics() pmetric.Metrics {

p.resourceMetrics.ForEach(func(_ resourceKey, rawMetrics *resourceMetrics) {
rm := m.ResourceMetrics().AppendEmpty()
if !excludeResourceMetrics.IsEnabled() {
if !excludeResourceMetrics.IsEnabled() || p.config.AddResourceAttributes {
rawMetrics.attributes.CopyTo(rm.Resource().Attributes())
}

Expand Down
104 changes: 104 additions & 0 deletions connector/spanmetricsconnector/connector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1061,6 +1061,110 @@ func TestResourceMetricsKeyAttributes(t *testing.T) {
assert.Equal(t, 2, p.resourceMetrics.Len())
}

func TestAddResourceAttributesConfig(t *testing.T) {
tests := []struct {
name string
addResourceAttributes bool
featureGateEnabled bool
expectResourceAttributes bool
}{
{
name: "feature gate disabled, config false - should include resource attributes",
addResourceAttributes: false,
featureGateEnabled: false,
expectResourceAttributes: true,
},
{
name: "feature gate enabled, config false - should exclude resource attributes",
addResourceAttributes: false,
featureGateEnabled: true,
expectResourceAttributes: false,
},
{
name: "feature gate enabled, config true - should include resource attributes (override)",
addResourceAttributes: true,
featureGateEnabled: true,
expectResourceAttributes: true,
},
{
name: "feature gate disabled, config true - should include resource attributes",
addResourceAttributes: true,
featureGateEnabled: false,
expectResourceAttributes: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Save and restore the feature gate state
previousValue := excludeResourceMetrics.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(excludeResourceMetrics.ID(), tt.featureGateEnabled))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(excludeResourceMetrics.ID(), previousValue))
}()

// Create a custom config with AddResourceAttributes
cfg := &Config{
AggregationTemporality: cumulative,
Histogram: explicitHistogramsConfig(),
Exemplars: disabledExemplarsConfig(),
Events: disabledEventsConfig(),
ResourceMetricsCacheSize: resourceMetricsCacheSize,
ResourceMetricsKeyAttributes: []string{},
Dimensions: []Dimension{},
MetricsFlushInterval: time.Nanosecond,
AddResourceAttributes: tt.addResourceAttributes,
}

p, err := newConnector(zaptest.NewLogger(t), cfg, clockwork.NewFakeClock(), instanceID)
require.NoError(t, err)
p.metricsConsumer = consumertest.NewNop()

// Create a trace with resource attributes
traces := ptrace.NewTraces()
rs := traces.ResourceSpans().AppendEmpty()
rs.Resource().Attributes().PutStr(string(conventions.ServiceNameKey), "test-service")
rs.Resource().Attributes().PutStr("test.resource.attr", "test-value")
rs.Resource().Attributes().PutStr(regionResourceAttrName, sampleRegion)

ils := rs.ScopeSpans().AppendEmpty()
span := ils.Spans().AppendEmpty()
span.SetName("/test")
span.SetKind(ptrace.SpanKindServer)
span.SetTraceID([16]byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, 0x10})
span.SetSpanID([8]byte{0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18})
span.SetStartTimestamp(pcommon.NewTimestampFromTime(time.Now()))
span.SetEndTimestamp(pcommon.NewTimestampFromTime(time.Now().Add(sampleDuration)))

// Consume the traces
ctx := metadata.NewIncomingContext(t.Context(), nil)
err = p.ConsumeTraces(ctx, traces)
require.NoError(t, err)

// Build metrics and verify
m := p.buildMetrics()
require.Equal(t, 1, m.ResourceMetrics().Len())

rm := m.ResourceMetrics().At(0)
resourceAttrs := rm.Resource().Attributes()

if tt.expectResourceAttributes {
// Should have resource attributes
assert.Positive(t, resourceAttrs.Len(), "Expected resource attributes to be present")
val, ok := resourceAttrs.Get(string(conventions.ServiceNameKey))
assert.True(t, ok, "Expected service.name attribute to be present")
assert.Equal(t, "test-service", val.Str())
val, ok = resourceAttrs.Get("test.resource.attr")
assert.True(t, ok, "Expected test.resource.attr attribute to be present")
assert.Equal(t, "test-value", val.Str())
} else {
// Should not have resource attributes
assert.Equal(t, 0, resourceAttrs.Len(), "Expected no resource attributes")
}
})
}
}

func BenchmarkConnectorConsumeTraces(b *testing.B) {
// Prepare
conn, err := newConnectorImp(stringp("defaultNullValue"), explicitHistogramsConfig, disabledExemplarsConfig, disabledEventsConfig, cumulative, 0, []string{}, 1000, clockwork.NewFakeClock())
Expand Down