Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ linters:
- linters:
- goconst
- gosec
- forbidigo
path: (.+)_test\.go
paths:
- third_party$
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* [CHANGE] **BREAKING CHANGE** Deprecating vParquet2 block format [#5688](https://github.com/grafana/tempo/pull/5688) (@ie-pham)
* [ENHANCEMENT] On startup, first record for live store to consume is not older than two complete block timeouts [#5693](https://github.com/grafana/tempo/pull/5693) (@ruslan-mikhailov)
* [ENHANCEMENT] Add secure connection support to tempo-cli [#5692](https://github.com/grafana/tempo/pull/5692) (@TheoBrigitte)
* [ENHANCEMENT] Add ability to drop instance label from span_metrics series [#5706](https://github.com/grafana/tempo/pull/5706) (@ie-pham)

# v2.9.0-rc.0

Expand Down
5 changes: 5 additions & 0 deletions docs/sources/tempo/configuration/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,9 @@ metrics_generator:
# Drop specific labels from `traces_target_info` metrics
[target_info_excluded_dimensions: <list of string>]

# Drop instance label from all span metrics series
[drop_instance_label: <bool>]

local_blocks:

# Block configuration
Expand Down Expand Up @@ -1813,6 +1816,8 @@ overrides:
[enable_target_info: <bool>]
# Drop specific resource labels from traces_target_info
[target_info_excluded_dimensions: <list of string>]
# drop instance label from all span metrics series
[drop_instance_label: <bool>]

# Configuration for the local-blocks processor
local-blocks:
Expand Down
1 change: 1 addition & 0 deletions docs/sources/tempo/configuration/manifest.md
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,7 @@ metrics_generator:
2: true
filter_policies: []
target_info_excluded_dimensions: []
drop_instance_label: false
local_blocks:
block:
bloom_filter_false_positive: 0.01
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ By default, the metrics processor adds the following labels to each metric: `ser
- `STATUS_CODE_ERROR` - The span operation completed with an error
- `status_message` (optionally enabled) - The message that details the reason for the `status_code` label
- `job` - The name of the job, a combination of namespace and service; only added if `metrics_generator.processor.span_metrics.enable_target_info: true`
- `instance` - The instance ID; only added if `metrics_generator.processor.span_metrics.enable_target_info: true`
- `instance` - The instance ID; only added if `metrics_generator.processor.span_metrics.enable_target_info: true` and can be dropped by setting `metrics_generator.processor.span_metrics.drop_instance_label: true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rlankfo isn't instance semi required for target_info to work correctly?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IE when we go to set the value on the Gauge your likely to override/not override appropriately in SetForTargetInfo

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain why it would not work appropriately?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the original requirement also states that if instance is empty to skip it entirely, which sounds to me like it's not a hard requirement.

Copy link
Copy Markdown
Contributor

@mattdurham mattdurham Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is not a job or instance then the metric will never get recorded, which feels like different behavior than reducing cardinality. The change in behavior would be if there is no job label and drop instance is true then the metric will not be recorded, whereas previously it would be. Though granted that should be pretty rare that a service.name (job) is not declared.


Additional user defined labels can be created using the [`dimensions` configuration option](https://grafana.com/docs/tempo/<TEMPO_VERSION>/configuration#metrics-generator).
When a configured dimension collides with one of the default labels (for example, `status_code`), the label for the respective dimension is prefixed with double underscore (for example, `__status_code`).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ metrics_generator:
]
[enable_target_info: <bool>]
[target_info_excluded_dimensions: <list of string>]
[drop_instance_label: <bool>]

host_info:
[metric_name: <string>]
Expand Down
8 changes: 8 additions & 0 deletions integration/e2e/overrides_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ func TestOverrides(t *testing.T) {
patch := &client.Limits{
MetricsGenerator: client.LimitsMetricsGenerator{
DisableCollection: boolPtr(true),
Processor: client.LimitsMetricsGeneratorProcessor{
SpanMetrics: client.LimitsMetricsGeneratorProcessorSpanMetrics{
DropInstanceLabel: boolPtr(true),
},
},
},
}

Expand All @@ -154,6 +159,9 @@ func TestOverrides(t *testing.T) {
processors, ok = limits.GetMetricsGenerator().GetProcessors()
assert.True(t, ok)
assert.ElementsMatch(t, keys(processors.GetMap()), []string{"span-metrics"})
dropInstanceLabel, dropInstanceLabelIsSet := limits.GetMetricsGenerator().GetProcessor().GetSpanMetrics().GetDropInstanceLabel()
assert.True(t, dropInstanceLabel)
assert.True(t, dropInstanceLabelIsSet)

// Delete overrides
if !tc.skipVersioning && tc.name != "gcs" {
Expand Down
4 changes: 4 additions & 0 deletions modules/generator/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,10 @@ func (cfg *ProcessorConfig) copyWithOverrides(o metricsGeneratorOverrides, userI

copyCfg.SpanMetrics.TargetInfoExcludedDimensions = o.MetricsGeneratorProcessorSpanMetricsTargetInfoExcludedDimensions(userID)

if dropInstanceLabel, ok := o.MetricsGeneratorProcessorSpanMetricsDropInstanceLabel(userID); ok {
copyCfg.SpanMetrics.DropInstanceLabel = dropInstanceLabel
}

if enableClientServerPrefix := o.MetricsGeneratorProcessorServiceGraphsEnableClientServerPrefix(userID); enableClientServerPrefix {
copyCfg.ServiceGraphs.EnableClientServerPrefix = enableClientServerPrefix
}
Expand Down
1 change: 1 addition & 0 deletions modules/generator/overrides.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type metricsGeneratorOverrides interface {
MetricsGeneratorProcessorServiceGraphsEnableMessagingSystemLatencyHistogram(userID string) (bool, bool)
MetricsGeneratorProcessorServiceGraphsEnableVirtualNodeLabel(userID string) (bool, bool)
MetricsGeneratorProcessorSpanMetricsTargetInfoExcludedDimensions(userID string) []string
MetricsGeneratorProcessorSpanMetricsDropInstanceLabel(userID string) (bool, bool)
MetricsGeneratorProcessorHostInfoHostIdentifiers(userID string) []string
MetricsGeneratorProcessorHostInfoMetricName(userID string) string
DedicatedColumns(userID string) backend.DedicatedColumns
Expand Down
9 changes: 9 additions & 0 deletions modules/generator/overrides_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type mockOverrides struct {
spanMetricsDimensionMappings []sharedconfig.DimensionMappings
spanMetricsEnableTargetInfo *bool
spanMetricsTargetInfoExcludedDimensions []string
spanMetricsDropInstanceLabel *bool
localBlocksMaxLiveTraces uint64
localBlocksMaxBlockDuration time.Duration
localBlocksMaxBlockBytes uint64
Expand Down Expand Up @@ -178,6 +179,14 @@ func (m *mockOverrides) MetricsGeneratorProcessorSpanMetricsTargetInfoExcludedDi
return m.spanMetricsTargetInfoExcludedDimensions
}

func (m *mockOverrides) MetricsGeneratorProcessorSpanMetricsDropInstanceLabel(string) (bool, bool) {
dropInstanceLabel := m.spanMetricsDropInstanceLabel
if dropInstanceLabel != nil {
return *dropInstanceLabel, true
}
return false, false
}

func (m *mockOverrides) DedicatedColumns(string) backend.DedicatedColumns {
return m.dedicatedColumns
}
Expand Down
3 changes: 3 additions & 0 deletions modules/generator/processor/spanmetrics/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ type Config struct {

// Allow user to specify labels they want to drop from target_info
TargetInfoExcludedDimensions []string `yaml:"target_info_excluded_dimensions"`

// Allow user to drop instance label
DropInstanceLabel bool `yaml:"drop_instance_label"`
}

func (cfg *Config) RegisterFlagsAndApplyDefaults(string, *flag.FlagSet) {
Expand Down
4 changes: 2 additions & 2 deletions modules/generator/processor/spanmetrics/spanmetrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func (p *Processor) aggregateMetricsForSpan(svcName string, jobName string, inst
labelValues = append(labelValues, jobName)
}
// add instance label only if job is not blank
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to your code but this comment looks like its wrong.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this is a chance to the original requirement. I will change the comments.

if instanceID != "" && p.Cfg.EnableTargetInfo {
if instanceID != "" && p.Cfg.EnableTargetInfo && !p.Cfg.DropInstanceLabel {
labels = append(labels, dimInstance)
labelValues = append(labelValues, instanceID)
}
Expand Down Expand Up @@ -246,7 +246,7 @@ func (p *Processor) aggregateMetricsForSpan(svcName string, jobName string, inst
targetInfoLabelValues = append(targetInfoLabelValues, jobName)
}
// add instance label to target info only if job is not blank
if instanceID != "" {
if instanceID != "" && !p.Cfg.DropInstanceLabel {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment issue here.

targetInfoLabels = append(targetInfoLabels, dimInstance)
targetInfoLabelValues = append(targetInfoLabelValues, instanceID)
}
Expand Down
Loading
Loading