diff --git a/.chloggen/bmchelix-enrich-metric-names.yaml b/.chloggen/bmchelix-enrich-metric-names.yaml new file mode 100644 index 0000000000000..a4283d9247693 --- /dev/null +++ b/.chloggen/bmchelix-enrich-metric-names.yaml @@ -0,0 +1,33 @@ +# 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. receiver/filelog) +component: exporter/bmchelix + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Enrich metric names with datapoint attributes for unique identification in BMC Helix Operations Management + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [46558] + +# (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 feature is controlled by the `enrich_metric_with_attributes` configuration option (default: `true`). + Set to `false` to disable enrichment and reduce metric cardinality. + Normalization is applied to ensure BHOM compatibility: + - `entityTypeId` and `entityName`: Invalid characters replaced with underscores (colons not allowed as they are used as separators in entityId) + - `metricName`: Normalized to match pattern `[a-zA-Z_:.][a-zA-Z0-9_:.]*` + - Label values: Commas replaced with whitespaces + +# 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: [] diff --git a/exporter/bmchelixexporter/README.md b/exporter/bmchelixexporter/README.md index c3bad4044ac75..798ac7f6e32f7 100644 --- a/exporter/bmchelixexporter/README.md +++ b/exporter/bmchelixexporter/README.md @@ -12,14 +12,14 @@ [contrib]: https://github.com/open-telemetry/opentelemetry-collector-releases/tree/main/distributions/otelcol-contrib -This exporter supports sending metrics to [BMC Helix Operations Management](https://www.bmc.com/it-solutions/bmc-helix-operations-management.html) through its [metric ingestion REST API](https://docs.bmc.com/docs/helixoperationsmanagement/244/en/metric-operation-management-endpoints-in-the-rest-api-1392780044.html). +This exporter supports sending metrics to [BMC Helix Operations Management](https://docs.bmc.com/xwiki/bin/view/IT-Operations-Management/Operations-Management/BMC-Helix-Operations-Management/bhom261/Getting-started/Product-overview/) through its [metric ingestion REST API](https://docs.bmc.com/docs/helixoperationsmanagement/244/en/metric-operation-management-endpoints-in-the-rest-api-1392780044.html). ## Getting Started The following settings are **required**: - `endpoint`: is the *BMC Helix Portal URL* of your environment, at **onbmc.com** for a BMC Helix SaaS tenant (e.g., `https://company.onbmc.com`), or your own Helix Portal URL for an on-prem instance. -- `api_key`: API key to authenticate the exporter. Connect to BMC Helix Operations Management, go to the Administration > Repository page, and click on the Copy API Key button to get your API Key. Alternatively, it is recommended to create and use a dedicated [authentication key for external integration](https://docs.bmc.com/docs/helixportal244/using-api-keys-for-external-integrations-1391501992.html). +- `api_key`: API key to authenticate the exporter. Connect to BMC Helix Operations Management, go to the Administration > Repository page, and click on the Copy API Key button to get your API Key. Alternatively, it is recommended to create and use a dedicated [authentication key for external integration](https://docs.bmc.com/xwiki/bin/view/Helix-Common-Services/BMC-Helix-Portal/BMC-Helix-Portal/helixportal261/Administering/Using-API-keys-for-external-integrations/). Example: @@ -37,6 +37,7 @@ exporters: The following settings can be **optionally configured**: - `timeout`: (default = `10s`) Timeout for requests made to the BMC Helix. +- `enrich_metric_with_attributes`: (default = `true`) When enabled, creates enriched metrics by appending datapoint attribute values to the metric name. This provides more detailed identification in BMC Helix Operations Management but increases metric cardinality. Set to `false` to reduce the number of unique metric series. - `retry_on_failure` [details here](https://github.com/open-telemetry/opentelemetry-collector/tree/main/exporter/exporterhelper#configuration) - `enabled` (default = true) - `initial_interval` (default = 5s) Time to wait after the first failure before retrying; ignored if `enabled` is false. @@ -51,6 +52,7 @@ exporters: endpoint: https://company.onbmc.com api_key: timeout: 20s + enrich_metric_with_attributes: false sending_queue: batch: retry_on_failure: diff --git a/exporter/bmchelixexporter/config.go b/exporter/bmchelixexporter/config.go index c0c4eb7ebf347..f122fa4552c36 100644 --- a/exporter/bmchelixexporter/config.go +++ b/exporter/bmchelixexporter/config.go @@ -14,8 +14,17 @@ import ( // Config struct is used to store the configuration of the exporter type Config struct { confighttp.ClientConfig `mapstructure:",squash"` - APIKey configopaque.String `mapstructure:"api_key"` - RetryConfig configretry.BackOffConfig `mapstructure:"retry_on_failure"` + // APIKey authenticates requests to BMC Helix Operations Management. + // Connect to BMC Helix Operations Management, go to the Administration > Repository page, and + // click on the Copy API Key button to get your API Key. + // Alternatively, it is recommended to create and use a dedicated authentication key for external integration: + // https://docs.bmc.com/xwiki/bin/view/Helix-Common-Services/BMC-Helix-Portal/BMC-Helix-Portal/helixportal261/Administering/Using-API-keys-for-external-integrations/ + APIKey configopaque.String `mapstructure:"api_key"` + RetryConfig configretry.BackOffConfig `mapstructure:"retry_on_failure"` + // EnrichMetricWithAttributes enables enriched metric creation by appending datapoint + // attribute values to the metric name. This increases metric cardinality but provides + // more detailed identification in BMC Helix Operations Management. Default is true. + EnrichMetricWithAttributes bool `mapstructure:"enrich_metric_with_attributes"` } // validate the configuration diff --git a/exporter/bmchelixexporter/config.schema.yaml b/exporter/bmchelixexporter/config.schema.yaml index cf73c3c2fd7e7..7f2acbdac5091 100644 --- a/exporter/bmchelixexporter/config.schema.yaml +++ b/exporter/bmchelixexporter/config.schema.yaml @@ -2,7 +2,11 @@ description: Config struct is used to store the configuration of the exporter type: object properties: api_key: + description: 'APIKey authenticates requests to BMC Helix Operations Management. Connect to BMC Helix Operations Management, go to the Administration > Repository page, and click on the Copy API Key button to get your API Key. Alternatively, it is recommended to create and use a dedicated authentication key for external integration: https://docs.bmc.com/xwiki/bin/view/Helix-Common-Services/BMC-Helix-Portal/BMC-Helix-Portal/helixportal261/Administering/Using-API-keys-for-external-integrations/' $ref: go.opentelemetry.io/collector/config/configopaque.string + enrich_metric_with_attributes: + description: EnrichMetricWithAttributes enables enriched metric creation by appending datapoint attribute values to the metric name. This increases metric cardinality but provides more detailed identification in BMC Helix Operations Management. Default is true. + type: boolean retry_on_failure: $ref: go.opentelemetry.io/collector/config/configretry.back_off_config allOf: diff --git a/exporter/bmchelixexporter/config_test.go b/exporter/bmchelixexporter/config_test.go index b08f6c95b3756..0a582afd4a452 100644 --- a/exporter/bmchelixexporter/config_test.go +++ b/exporter/bmchelixexporter/config_test.go @@ -33,9 +33,10 @@ func TestLoadConfig(t *testing.T) { { id: component.NewIDWithName(metadata.Type, "helix1"), expected: &Config{ - ClientConfig: createDefaultClientConfig("https://helix1:8080", 10*time.Second), - APIKey: "api_key", - RetryConfig: configretry.NewDefaultBackOffConfig(), + ClientConfig: createDefaultClientConfig("https://helix1:8080", 10*time.Second), + APIKey: "api_key", + RetryConfig: configretry.NewDefaultBackOffConfig(), + EnrichMetricWithAttributes: true, }, }, { @@ -51,6 +52,7 @@ func TestLoadConfig(t *testing.T) { MaxInterval: 1 * time.Minute, MaxElapsedTime: 8 * time.Minute, }, + EnrichMetricWithAttributes: true, }, }, } diff --git a/exporter/bmchelixexporter/exporter_metrics.go b/exporter/bmchelixexporter/exporter_metrics.go index 9751c293a112f..884a929251464 100644 --- a/exporter/bmchelixexporter/exporter_metrics.go +++ b/exporter/bmchelixexporter/exporter_metrics.go @@ -61,7 +61,7 @@ func (me *metricsExporter) start(ctx context.Context, host component.Host) error me.logger.Info("Starting BMC Helix Metrics Exporter") // Initialize and store the MetricsProducer - me.producer = om.NewMetricsProducer(me.logger) + me.producer = om.NewMetricsProducer(me.logger, me.config.EnrichMetricWithAttributes) // Initialize and store the MetricsClient client, err := om.NewMetricsClient(ctx, me.config.ClientConfig, me.config.APIKey, host, me.telemetrySettings, me.logger) diff --git a/exporter/bmchelixexporter/factory.go b/exporter/bmchelixexporter/factory.go index afd77e7c6f02d..f9e67f1abbd4e 100644 --- a/exporter/bmchelixexporter/factory.go +++ b/exporter/bmchelixexporter/factory.go @@ -31,8 +31,9 @@ func createDefaultConfig() component.Config { httpClientConfig.Timeout = 10 * time.Second return &Config{ - ClientConfig: httpClientConfig, - RetryConfig: configretry.NewDefaultBackOffConfig(), + ClientConfig: httpClientConfig, + RetryConfig: configretry.NewDefaultBackOffConfig(), + EnrichMetricWithAttributes: true, } } diff --git a/exporter/bmchelixexporter/internal/operationsmanagement/metrics_producer.go b/exporter/bmchelixexporter/internal/operationsmanagement/metrics_producer.go index 9a01d79641d47..d1e2f88465d18 100644 --- a/exporter/bmchelixexporter/internal/operationsmanagement/metrics_producer.go +++ b/exporter/bmchelixexporter/internal/operationsmanagement/metrics_producer.go @@ -30,15 +30,17 @@ type BMCHelixOMSample struct { // MetricsProducer is responsible for converting OpenTelemetry metrics into BMC Helix Operations Management metrics type MetricsProducer struct { - logger *zap.Logger - previousCounters map[string]BMCHelixOMSample + logger *zap.Logger + previousCounters map[string]BMCHelixOMSample + enrichMetricWithAttributes bool } // NewMetricsProducer creates a new MetricsProducer -func NewMetricsProducer(logger *zap.Logger) *MetricsProducer { +func NewMetricsProducer(logger *zap.Logger, enrichMetricWithAttributes bool) *MetricsProducer { return &MetricsProducer{ - logger: logger, - previousCounters: make(map[string]BMCHelixOMSample), + logger: logger, + previousCounters: make(map[string]BMCHelixOMSample), + enrichMetricWithAttributes: enrichMetricWithAttributes, } } @@ -159,6 +161,27 @@ func (mp *MetricsProducer) createHelixMetrics(metric pmetric.Metric, resourceAtt metricPayload.Labels[rateMetricFlag] = "true" } + // This will create a new enriched metric with distinguishing attributes appended to the metric name + // for consistent identification in BMC Helix Operations Management + if mp.enrichMetricWithAttributes { + if enriched := createEnrichedMetricWithDpAttributes(metricPayload, dp.Attributes().AsRaw()); enriched != nil { + // Propagate rate flag if needed (Sum monotonic) + if metricPayload.Labels[rateMetricFlag] == "true" { + enriched.Labels[rateMetricFlag] = "true" + } + helixMetrics = append(helixMetrics, *enriched) + + // Remove entityId from the original metric since a derived metric is being created. + // Without entityId, the original metric is forwarded to victoriametrics only (not BHOM), + // while the enriched metric with its unique name is reported in BHOM. + delete(metricPayload.Labels, "entityId") + + // Remove rate flag from the original metric to prevent rate computation with an + // empty entityId (":"+metricName key collision) + delete(metricPayload.Labels, rateMetricFlag) + } + } + helixMetrics = append(helixMetrics, *metricPayload) } case pmetric.MetricTypeGauge: @@ -171,18 +194,25 @@ func (mp *MetricsProducer) createHelixMetrics(metric pmetric.Metric, resourceAtt mp.logger.Warn("Failed to create Helix metric from datapoint", zap.Error(err)) continue } + // This will create a new enriched metric with distinguishing attributes appended to the metric name + // for consistent identification in BMC Helix Operations Management + if mp.enrichMetricWithAttributes { + if enriched := createEnrichedMetricWithDpAttributes(metricPayload, dp.Attributes().AsRaw()); enriched != nil { + helixMetrics = append(helixMetrics, *enriched) + + // Remove entityId from the original metric since a derived metric is being created. + // Without entityId, the original metric is forwarded to victoriametrics only (not BHOM), + // while the enriched metric with its unique name is reported in BHOM. + delete(metricPayload.Labels, "entityId") + } + } + helixMetrics = append(helixMetrics, *metricPayload) } default: return nil, fmt.Errorf("unsupported metric type %s", metric.Type()) } - // Enrich metric names with attributes - // This will modify the metric names based on the attributes that have more than one distinct value - // across the metrics with the same entityId and metricName - // This is done to ensure that the metric names are unique and meaningful in the BMC Helix Operations Management payload - helixMetrics = enrichMetricNamesWithAttributes(helixMetrics) - // Add percentage variants for ratio metrics (unit "1") helixMetrics = addPercentageVariants(helixMetrics) @@ -220,8 +250,10 @@ func (mp *MetricsProducer) createSingleDatapointMetric(dp pmetric.NumberDataPoin labels := make(map[string]string) labels["source"] = "OTEL" - // Add resource attributes - maps.Copy(labels, resourceAttrs) + // Add resource attributes with normalization + for k, v := range resourceAttrs { + labels[k] = NormalizeLabelValue(v) + } // Set the metric unit labels["unit"] = metric.Unit() @@ -233,7 +265,11 @@ func (mp *MetricsProducer) createSingleDatapointMetric(dp pmetric.NumberDataPoin labels["isDeviceMappingEnabled"] = "true" // Update the metric name for the BMC Helix Operations Management payload - labels["metricName"] = metric.Name() + normalizedMetricName := NormalizeMetricName(metric.Name()) + if normalizedMetricName == "" { + return nil, fmt.Errorf("the normalized metric name is empty for metric %s. Metric datapoint will be skipped", metric.Name()) + } + labels["metricName"] = normalizedMetricName // Update the entity information err := mp.updateEntityInformation(labels, metric.Name(), resourceAttrs, dp.Attributes().AsRaw()) @@ -268,8 +304,9 @@ func (*MetricsProducer) updateEntityInformation(labels map[string]string, metric // Convert metricAttrs from map[string]any to map[string]string for compatibility stringMetricAttrs := make(map[string]string) for k, v := range dpAttributes { - stringMetricAttrs[k] = fmt.Sprintf("%v", v) - labels[k] = fmt.Sprintf("%v", v) + normalizedValue := NormalizeLabelValue(fmt.Sprint(v)) + stringMetricAttrs[k] = normalizedValue + labels[k] = normalizedValue } // Add the resource attributes to the metric attributes @@ -292,17 +329,12 @@ func (*MetricsProducer) updateEntityInformation(labels map[string]string, metric instanceName = entityName } - // Trim trailing and leading colons from entityName - entityName = strings.Trim(entityName, ":") - - // Remove any colons from the entityName to ensure compatibility with BMC Helix Operations Management - entityName = strings.ReplaceAll(entityName, ":", "") - // Set the entityTypeId, entityId, instanceName and entityName in labels - labels["entityTypeId"] = entityTypeID - labels["entityName"] = entityName + // Use NormalizeEntityValue to ensure ":" is not present as it is the separator in entityId + labels["entityTypeId"] = NormalizeEntityValue(entityTypeID) + labels["entityName"] = NormalizeEntityValue(entityName) labels["instanceName"] = instanceName - labels["entityId"] = fmt.Sprintf("%s:%s:%s:%s", labels["source"], labels["hostname"], entityTypeID, entityName) + labels["entityId"] = fmt.Sprintf("%s:%s:%s:%s", labels["source"], labels["hostname"], labels["entityTypeId"], labels["entityName"]) return nil } @@ -333,87 +365,54 @@ func extractResourceAttributes(resource pcommon.Resource) map[string]string { return attributes } -// enrichMetricNamesWithAttributes modifies the metric names by appending distinguishing attributes -// that have more than one distinct value across the metrics with the same entityId and metricName -// A copy of the metric is created without entityId, entityTypeId, and entityName attributes -// to ensure that the original metric is preserved for the BMC Helix VictoriaMetrics. -// This is done to ensure that the metric names are unique in the BMC Helix Operations Management payload -func enrichMetricNamesWithAttributes(metrics []BMCHelixOMMetric) []BMCHelixOMMetric { - // Step 1: Group metrics by (entityId + metricName) - groups := make(map[string][]*BMCHelixOMMetric) - for i := range metrics { - m := &metrics[i] - key := m.Labels["entityId"] + ":" + m.Labels["metricName"] - groups[key] = append(groups[key], m) - } - - finalMetrics := make([]BMCHelixOMMetric, 0, len(metrics)*2) - - // Step 2: Process each group - for _, group := range groups { - attrValues := make(map[string]map[string]struct{}) - - // Collect all attribute values for each key - for _, m := range group { - for k, v := range m.Labels { - if _, shouldSkip := coreAttributes[k]; shouldSkip { - continue - } - if _, exists := attrValues[k]; !exists { - attrValues[k] = make(map[string]struct{}) - } - attrValues[k][v] = struct{}{} - } +// createEnrichedMetricWithDpAttributes creates a copy of the metric with non-core datapoint attribute +// values appended to the metric name as a dot-separated suffix. Attribute keys are sorted +// alphabetically and the resulting metric name is normalized (e.g., special chars replaced with "_"). +// Returns nil if no non-core attributes exist. The original metric is not modified. +func createEnrichedMetricWithDpAttributes(metric *BMCHelixOMMetric, dpAttrs map[string]any) *BMCHelixOMMetric { + // Collect dp attribute keys excluding core ones (sorted using insertSorted) + var keys []string + for k, v := range dpAttrs { + if _, shouldSkip := coreAttributes[k]; shouldSkip { + continue } - - // Step 3: Identifying attributes (those with >1 distinct value) - var identifyingKeys []string - for k, vals := range attrValues { - if len(vals) > 1 { - identifyingKeys = insertSorted(identifyingKeys, k) - } + if v == nil { + continue + } + s := fmt.Sprint(v) + if s == "" { + continue } + keys = insertSorted(keys, k) + } - // Step 4: Modify metric names by appending attribute values - for _, m := range group { - originalMetricName := m.Labels["metricName"] + if len(keys) == 0 { + return nil + } - // Build suffix - var suffixParts []string - for _, attrKey := range identifyingKeys { - if val, ok := m.Labels[attrKey]; ok { - suffixParts = append(suffixParts, val) - } - } + // Values-only suffix in sorted key order + suffixParts := make([]string, 0, len(keys)) + for _, k := range keys { + suffixParts = append(suffixParts, fmt.Sprint(dpAttrs[k])) + } - // Only create copy + modify metric if there's a suffix to apply - if len(suffixParts) > 0 { - // Step 1: Raw copy without entityId/entityTypeId/entityName - rawCopy := BMCHelixOMMetric{ - Labels: make(map[string]string), - Samples: m.Samples, - } - for k, v := range m.Labels { - if k != "entityId" && k != "entityTypeId" && k != "entityName" { - rawCopy.Labels[k] = v - } - } - rawCopy.Labels["metricName"] = originalMetricName - finalMetrics = append(finalMetrics, rawCopy) + dup := BMCHelixOMMetric{ + Labels: make(map[string]string, len(metric.Labels)), + Samples: metric.Samples, + } + maps.Copy(dup.Labels, metric.Labels) - // Step 2: Modify the original metric - m.Labels["metricName"] = originalMetricName + "." + strings.Join(suffixParts, ".") - for _, attrKey := range identifyingKeys { - delete(m.Labels, attrKey) // Remove identifying attributes from the metric labels - } - } + original := dup.Labels["metricName"] + enrichedMetricName := NormalizeMetricName(original + "." + strings.Join(suffixParts, ".")) - // Always keep the (possibly modified) original metric - finalMetrics = append(finalMetrics, *m) - } + // If the enriched metric name is empty, skip this enriched metric + if enrichedMetricName == "" { + return nil } - return finalMetrics + dup.Labels["metricName"] = enrichedMetricName + + return &dup } // Binary-inserted sorted slice diff --git a/exporter/bmchelixexporter/internal/operationsmanagement/metrics_producer_test.go b/exporter/bmchelixexporter/internal/operationsmanagement/metrics_producer_test.go index ee5bd141e971e..7b57d2dc7d234 100644 --- a/exporter/bmchelixexporter/internal/operationsmanagement/metrics_producer_test.go +++ b/exporter/bmchelixexporter/internal/operationsmanagement/metrics_producer_test.go @@ -74,7 +74,7 @@ func TestProduceHelixPayload(t *testing.T) { expectedPayload := []BMCHelixOMMetric{parent, metric1, metric2} - producer := NewMetricsProducer(zap.NewExample()) + producer := NewMetricsProducer(zap.NewExample(), true) tests := []struct { name string @@ -118,10 +118,57 @@ func TestProduceHelixPayload(t *testing.T) { } } +// TestProduceHelixPayloadWithEnrichmentDisabled verifies that when enrichMetricWithAttributes is false, +// no enriched metrics are created and the original metrics retain their entityId. +func TestProduceHelixPayloadWithEnrichmentDisabled(t *testing.T) { + t.Parallel() + + producer := NewMetricsProducer(zap.NewExample(), false) + + // Generate metrics with non-core attributes that would normally trigger enrichment + metrics := pmetric.NewMetrics() + rm := metrics.ResourceMetrics().AppendEmpty() + rm.Resource().Attributes().PutStr("host.name", "test-hostname") + sm := rm.ScopeMetrics().AppendEmpty() + metric := sm.Metrics().AppendEmpty() + metric.SetName("system.cpu.time") + metric.SetUnit("s") + + dp := metric.SetEmptyGauge().DataPoints().AppendEmpty() + dp.SetTimestamp(1750926531000000000) + dp.SetDoubleValue(42.0) + dp.Attributes().PutStr("entityTypeId", "cpu") + dp.Attributes().PutStr("entityName", "cpu0") + dp.Attributes().PutStr("cpu.mode", "idle") // Non-core attribute that would trigger enrichment + + payload, err := producer.ProduceHelixPayload(metrics) + assert.NoError(t, err) + + // With enrichment disabled, we should get: + // 1. Parent entity (identity metric) + // 2. Original metric with entityId preserved (no enriched variant) + // Total: 2 metrics (not 3 as would be with enrichment enabled) + assert.Len(t, payload, 2, "Should have parent + original metric only, no enriched variant") + + // Find the actual metric (not the parent identity) + var actualMetric *BMCHelixOMMetric + for i := range payload { + if payload[i].Labels["metricName"] != "identity" { + actualMetric = &payload[i] + break + } + } + + assert.NotNil(t, actualMetric, "Should have found the actual metric") + assert.Equal(t, "system.cpu.time", actualMetric.Labels["metricName"], "Metric name should not be enriched") + assert.NotEmpty(t, actualMetric.Labels["entityId"], "entityId should be preserved when enrichment is disabled") +} + // Mock data generation for testing func generateMockMetrics(setMetricType func(metric pmetric.Metric) pmetric.NumberDataPointSlice) pmetric.Metrics { metrics := pmetric.NewMetrics() rm := metrics.ResourceMetrics().AppendEmpty() + rm.Resource().Attributes().PutStr("host.name", "test-hostname") il := rm.ScopeMetrics().AppendEmpty().Metrics() metric := il.AppendEmpty() metric.SetName("test_metric") @@ -132,7 +179,6 @@ func generateMockMetrics(setMetricType func(metric pmetric.Metric) pmetric.Numbe // First datapoint dp1 := dps.AppendEmpty() - dp1.Attributes().PutStr("host.name", "test-hostname") dp1.Attributes().PutStr("entityName", "test-entity-1") dp1.Attributes().PutStr("entityTypeId", "test-entity-type-id") dp1.Attributes().PutStr("instanceName", "test-entity-Name-1") @@ -141,7 +187,6 @@ func generateMockMetrics(setMetricType func(metric pmetric.Metric) pmetric.Numbe // Second datapoint dp2 := dps.AppendEmpty() - dp2.Attributes().PutStr("host.name", "test-hostname") dp2.Attributes().PutStr("entityName", "test-entity-2") dp2.Attributes().PutStr("entityTypeId", "test-entity-type-id") dp2.Attributes().PutStr("instanceName", "test-entity-Name-2") @@ -151,133 +196,117 @@ func generateMockMetrics(setMetricType func(metric pmetric.Metric) pmetric.Numbe return metrics } -func TestEnrichMetricNamesWithAttributes(t *testing.T) { +func TestCreateEnrichedMetricWithDpAttributes(t *testing.T) { t.Parallel() tests := []struct { - name string - inputMetrics []BMCHelixOMMetric - expectedLabels []map[string]string + name string + inputMetric *BMCHelixOMMetric + dpAttrs map[string]any + expectedMetricName string + expectNil bool }{ { - name: "Single metric without varying attributes", - inputMetrics: []BMCHelixOMMetric{ - { - Labels: map[string]string{ - "entityId": "host:cpu:core0", - "metricName": "system.cpu.time", - "cpu.mode": "idle", - "cpu.logical_number": "0", - }, + name: "No non-core attributes returns nil", + inputMetric: &BMCHelixOMMetric{ + Labels: map[string]string{ + "entityId": "host:cpu:core0", + "metricName": "system.cpu.time", + "hostname": "myhost", }, }, - expectedLabels: []map[string]string{ - { - "entityId": "host:cpu:core0", - "metricName": "system.cpu.time", - "cpu.mode": "idle", - "cpu.logical_number": "0", - }, + dpAttrs: map[string]any{ + "hostname": "myhost", // core attribute only }, + expectNil: true, }, { - name: "Metrics with different state values", - inputMetrics: []BMCHelixOMMetric{ - { - Labels: map[string]string{ - "entityId": "host:cpu:core0", - "metricName": "system.cpu.time", - "cpu.mode": "idle", - }, - }, - { - Labels: map[string]string{ - "entityId": "host:cpu:core0", - "metricName": "system.cpu.time", - "cpu.mode": "user", - }, - }, - }, - expectedLabels: []map[string]string{ - { - "metricName": "system.cpu.time", - "cpu.mode": "idle", - }, - { + name: "Single non-core attribute appends normalized value", + inputMetric: &BMCHelixOMMetric{ + Labels: map[string]string{ "entityId": "host:cpu:core0", - "metricName": "system.cpu.time.idle", - }, - { "metricName": "system.cpu.time", - "cpu.mode": "user", }, - { + }, + dpAttrs: map[string]any{ + "cpu.mode": "idle", + }, + expectedMetricName: "system.cpu.time.idle", + }, + { + name: "Multiple non-core attributes sorted by key", + inputMetric: &BMCHelixOMMetric{ + Labels: map[string]string{ "entityId": "host:cpu:core0", - "metricName": "system.cpu.time.user", + "metricName": "system.cpu.time", }, }, + dpAttrs: map[string]any{ + "cpu.mode": "user", + "cpu.core": "0", + }, + expectedMetricName: "system.cpu.time.0.user", // cpu.core < cpu.mode }, { - name: "Metrics with multiple varying attributes", - inputMetrics: []BMCHelixOMMetric{ - { - Labels: map[string]string{ - "entityId": "host:cpu:core0", - "metricName": "system.cpu.time", - "cpu.mode": "system", - "cpu.mode.code": "0", - "cpu.logical_number": "0", - }, - }, - { - Labels: map[string]string{ - "entityId": "host:cpu:core0", - "metricName": "system.cpu.time", - "cpu.mode": "user", - "cpu.mode.code": "1", - "cpu.logical_number": "0", - }, + name: "Attribute values are normalized", + inputMetric: &BMCHelixOMMetric{ + Labels: map[string]string{ + "metricName": "my.metric", }, }, - expectedLabels: []map[string]string{ - { - "metricName": "system.cpu.time", - "cpu.mode": "system", - "cpu.mode.code": "0", - "cpu.logical_number": "0", - }, - { - "entityId": "host:cpu:core0", - "metricName": "system.cpu.time.system.0", - "cpu.logical_number": "0", - }, - { - "metricName": "system.cpu.time", - "cpu.mode": "user", - "cpu.mode.code": "1", - "cpu.logical_number": "0", + dpAttrs: map[string]any{ + "state": "Read/Write (Active)", + }, + expectedMetricName: "my.metric.Read_Write_Active_", + }, + { + name: "Empty and nil attribute values are skipped", + inputMetric: &BMCHelixOMMetric{ + Labels: map[string]string{ + "metricName": "my.metric", }, - { - "entityId": "host:cpu:core0", - "metricName": "system.cpu.time.user.1", - "cpu.logical_number": "0", + }, + dpAttrs: map[string]any{ + "valid": "ok", + "empty": "", + "nil": nil, + }, + expectedMetricName: "my.metric.ok", + }, + { + name: "All attributes empty or nil returns nil", + inputMetric: &BMCHelixOMMetric{ + Labels: map[string]string{ + "metricName": "my.metric", }, }, + dpAttrs: map[string]any{ + "empty": "", + "nil": nil, + }, + expectNil: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := enrichMetricNamesWithAttributes(tt.inputMetrics) - - var actualLabels []map[string]string - for _, m := range result { - // Copy to a new map to avoid mutation side-effects - labelsCopy := make(map[string]string) - maps.Copy(labelsCopy, m.Labels) - actualLabels = append(actualLabels, labelsCopy) + // Snapshot input labels before the call to verify immutability + originalLabels := make(map[string]string, len(tt.inputMetric.Labels)) + maps.Copy(originalLabels, tt.inputMetric.Labels) + + result := createEnrichedMetricWithDpAttributes(tt.inputMetric, tt.dpAttrs) + + // Ensure the input metric is not mutated (including entityId and all other labels) + assert.Equal(t, originalLabels, tt.inputMetric.Labels, "input metric labels must not be mutated") + + if tt.expectNil { + assert.Nil(t, result) + return } - assert.ElementsMatch(t, tt.expectedLabels, actualLabels) + assert.NotNil(t, result) + assert.Equal(t, tt.expectedMetricName, result.Labels["metricName"]) + // Ensure the enriched metric name differs from the original + assert.NotEqual(t, originalLabels["metricName"], result.Labels["metricName"]) }) } } @@ -409,7 +438,7 @@ func TestAddPercentageVariants(t *testing.T) { func TestAddRateVariants(t *testing.T) { t.Parallel() - producer := NewMetricsProducer(zap.NewExample()) + producer := NewMetricsProducer(zap.NewExample(), true) // Create a base counter metric originalLabels := map[string]string{ @@ -456,3 +485,87 @@ func TestAddRateVariants(t *testing.T) { assert.InDelta(t, expectedRate, rate.Samples[0].Value, 0.001) assert.Equal(t, t2, rate.Samples[0].Timestamp) } + +func TestEmptyMetricNameSkipsPayload(t *testing.T) { + t.Parallel() + + producer := NewMetricsProducer(zap.NewExample(), true) + + // Test 1: Metric with empty name (normalizes to empty string and should be skipped) + metrics := pmetric.NewMetrics() + rm := metrics.ResourceMetrics().AppendEmpty() + resource := rm.Resource() + resource.Attributes().PutStr("host.name", "test-host") + + sm := rm.ScopeMetrics().AppendEmpty() + metric := sm.Metrics().AppendEmpty() + metric.SetName("") // Empty name normalizes to empty and should be skipped + + gauge := metric.SetEmptyGauge() + dp := gauge.DataPoints().AppendEmpty() + dp.SetTimestamp(1750926531000000000) + dp.SetDoubleValue(42.0) + dp.Attributes().PutStr("entityTypeId", "test-entity") + dp.Attributes().PutStr("entityName", "test-name") + + payload, err := producer.ProduceHelixPayload(metrics) + + // Should not return an error, but the payload should be empty since the metric was skipped + assert.NoError(t, err) + assert.Empty(t, payload, "Metrics with empty normalized names should be skipped") +} + +func TestCreateEnrichedMetricWithEmptyName(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + metricName string + dpAttrs map[string]any + expectNil bool + description string + }{ + { + name: "Enriched metric name becomes empty", + metricName: "valid.metric", + dpAttrs: map[string]any{ + "attribute": "!@#$%", // This will normalize to empty, making the whole enriched name potentially empty + }, + expectNil: false, // The enriched name would be "valid.metric." which is not empty + description: "Even if attribute value normalizes poorly, base metric name keeps it non-empty", + }, + { + name: "Base metric name empty, enrichment attempted", + metricName: "", // Empty base metric name + dpAttrs: map[string]any{ + "attribute": "value", + }, + expectNil: false, // Will create ".value" which normalizes to "value" + description: "Empty base with valid attribute creates enriched metric", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + inputMetric := &BMCHelixOMMetric{ + Labels: map[string]string{ + "metricName": tt.metricName, + "entityId": "test:entity:id", + }, + Samples: []BMCHelixOMSample{{Value: 1.0, Timestamp: 1000}}, + } + + result := createEnrichedMetricWithDpAttributes(inputMetric, tt.dpAttrs) + + if tt.expectNil { + assert.Nil(t, result, tt.description) + } else { + assert.NotNil(t, result, tt.description) + if result != nil { + // Verify the metric name is not empty + assert.NotEmpty(t, result.Labels["metricName"], "Enriched metric name should not be empty") + } + } + }) + } +} diff --git a/exporter/bmchelixexporter/internal/operationsmanagement/normalize_entity_value.go b/exporter/bmchelixexporter/internal/operationsmanagement/normalize_entity_value.go new file mode 100644 index 0000000000000..ddbca65612fde --- /dev/null +++ b/exporter/bmchelixexporter/internal/operationsmanagement/normalize_entity_value.go @@ -0,0 +1,39 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package operationsmanagement // import "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/bmchelixexporter/internal/operationsmanagement" + +import ( + "strings" + "unicode" +) + +// allowedEntityValueChars contains all special characters allowed in entity values +// (entityTypeId, entityName) besides alphanumerics and whitespace. +// https://docs.bmc.com/xwiki/bin/view/IT-Operations-Management/Operations-Management/BMC-Helix-Operations-Management/bhom244/Policy-event-data-and-metric-data-management-endpoints-in-the-REST-API/Metric-operation-management-endpoints-in-the-REST-API/ +const allowedEntityValueChars = "~!@#$^&*()-_=[];'?./\\" + +// NormalizeEntityValue normalizes entity values (entityTypeId, entityName) to contain only valid characters. +// Allowed characters: a-zA-Z0-9~!@#$^&*()-_=[];'?./\ and whitespace +// Invalid characters are replaced with underscores. +func NormalizeEntityValue(value string) string { + if value == "" { + return value + } + + // Replace all invalid characters with underscores + return strings.Map(sanitizeEntityValueRune, value) +} + +// sanitizeEntityValueRune returns the rune if it's valid for an entity value, +// otherwise returns '_'. +// Valid characters: letters, digits, whitespace, and special chars in allowedEntityValueChars +func sanitizeEntityValueRune(r rune) rune { + if unicode.IsLetter(r) || unicode.IsDigit(r) || unicode.IsSpace(r) { + return r + } + if strings.ContainsRune(allowedEntityValueChars, r) { + return r + } + return '_' +} diff --git a/exporter/bmchelixexporter/internal/operationsmanagement/normalize_entity_value_test.go b/exporter/bmchelixexporter/internal/operationsmanagement/normalize_entity_value_test.go new file mode 100644 index 0000000000000..51f121983a834 --- /dev/null +++ b/exporter/bmchelixexporter/internal/operationsmanagement/normalize_entity_value_test.go @@ -0,0 +1,107 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package operationsmanagement + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNormalizeEntityValue(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "empty string", + input: "", + expected: "", + }, + { + name: "simple alphanumeric", + input: "value123", + expected: "value123", + }, + { + name: "value with spaces", + input: "hello world", + expected: "hello world", + }, + { + name: "value with allowed special chars", + input: "test~!@#$^&*()-_=[];'?./\\value", + expected: "test~!@#$^&*()-_=[];'?./\\value", + }, + { + name: "value with colons replaced", + input: "host:port:path", + expected: "host_port_path", + }, + { + name: "value with disallowed chars", + input: "test<>|value", + expected: "test___value", + }, + { + name: "value with percent", + input: "50%", + expected: "50_", + }, + { + name: "value with curly braces", + input: "{test}", + expected: "_test_", + }, + { + name: "value with mixed chars", + input: "device=eth0", + expected: "device=eth0", + }, + { + name: "path-like value", + input: "/var/log/test.log", + expected: "/var/log/test.log", + }, + { + name: "entityTypeId with colons", + input: "container:docker:nginx", + expected: "container_docker_nginx", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := NormalizeEntityValue(tt.input) + require.Equal(t, tt.expected, result) + }) + } +} + +func TestSanitizeEntityValueRune(t *testing.T) { + // Valid runes should be returned as-is + require.Equal(t, 'a', sanitizeEntityValueRune('a')) + require.Equal(t, 'Z', sanitizeEntityValueRune('Z')) + require.Equal(t, '5', sanitizeEntityValueRune('5')) + require.Equal(t, ' ', sanitizeEntityValueRune(' ')) + require.Equal(t, '\t', sanitizeEntityValueRune('\t')) + + // Allowed special chars should be returned as-is (excluding ":") + allowedSpecials := []rune{'~', '!', '@', '#', '$', '^', '&', '*', '(', ')', '-', '_', '=', '[', ']', ';', '\'', '?', '.', '/', '\\'} + for _, r := range allowedSpecials { + require.Equal(t, r, sanitizeEntityValueRune(r), "Expected %c to be allowed", r) + } + + // Colon should be replaced with underscore + require.Equal(t, '_', sanitizeEntityValueRune(':')) + + // Invalid runes should be replaced with '_' + require.Equal(t, '_', sanitizeEntityValueRune('<')) + require.Equal(t, '_', sanitizeEntityValueRune('>')) + require.Equal(t, '_', sanitizeEntityValueRune('|')) + require.Equal(t, '_', sanitizeEntityValueRune('%')) + require.Equal(t, '_', sanitizeEntityValueRune('{')) + require.Equal(t, '_', sanitizeEntityValueRune('}')) +} diff --git a/exporter/bmchelixexporter/internal/operationsmanagement/normalize_label_value.go b/exporter/bmchelixexporter/internal/operationsmanagement/normalize_label_value.go new file mode 100644 index 0000000000000..7f9cf40342d9d --- /dev/null +++ b/exporter/bmchelixexporter/internal/operationsmanagement/normalize_label_value.go @@ -0,0 +1,14 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package operationsmanagement // import "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/bmchelixexporter/internal/operationsmanagement" + +import ( + "strings" +) + +// NormalizeLabelValue normalizes the label value by replacing commas with whitespace. +// Commas are not allowed in label values as they may interfere with parsing. +func NormalizeLabelValue(value string) string { + return strings.ReplaceAll(value, ",", " ") +} diff --git a/exporter/bmchelixexporter/internal/operationsmanagement/normalize_label_value_test.go b/exporter/bmchelixexporter/internal/operationsmanagement/normalize_label_value_test.go new file mode 100644 index 0000000000000..a0c113e96c418 --- /dev/null +++ b/exporter/bmchelixexporter/internal/operationsmanagement/normalize_label_value_test.go @@ -0,0 +1,66 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package operationsmanagement + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNormalizeLabelValue(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "empty string", + input: "", + expected: "", + }, + { + name: "simple alphanumeric", + input: "value123", + expected: "value123", + }, + { + name: "value with spaces", + input: "hello world", + expected: "hello world", + }, + { + name: "value with special chars preserved", + input: "test~!@#$^&*()-_=[];'?./\\:value", + expected: "test~!@#$^&*()-_=[];'?./\\:value", + }, + { + name: "value with comma replaced", + input: "value1,value2,value3", + expected: "value1 value2 value3", + }, + { + name: "value with colons preserved", + input: "host:port:path", + expected: "host:port:path", + }, + { + name: "path-like value", + input: "/var/log/test.log", + expected: "/var/log/test.log", + }, + { + name: "complex value with comma", + input: "cpu=0,mode=idle", + expected: "cpu=0 mode=idle", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := NormalizeLabelValue(tt.input) + require.Equal(t, tt.expected, result) + }) + } +} diff --git a/exporter/bmchelixexporter/internal/operationsmanagement/normalize_metric_name.go b/exporter/bmchelixexporter/internal/operationsmanagement/normalize_metric_name.go new file mode 100644 index 0000000000000..698c05b06c62b --- /dev/null +++ b/exporter/bmchelixexporter/internal/operationsmanagement/normalize_metric_name.go @@ -0,0 +1,44 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package operationsmanagement // import "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/bmchelixexporter/internal/operationsmanagement" + +import ( + "strings" +) + +// NormalizeMetricName normalizes the metric name so that it matches [a-zA-Z_:.][a-zA-Z0-9_:.]* +// Only ASCII letters, digits, underscores, colons, and dots are accepted. +// Unsupported characters are replaced with underscores. +// If the metric name starts with a digit, it is prefixed with an underscore. +// Consecutive underscores are collapsed into a single underscore. +func NormalizeMetricName(name string) string { + if name == "" { + return name + } + + // Replace all unsupported characters with underscores + name = strings.Map(sanitizeMetricNameRune, name) + + // Metric name cannot start with a digit, so prefix it with "_" in this case + if name != "" && name[0] >= '0' && name[0] <= '9' { + name = "_" + name + } + + // Collapse consecutive underscores + for strings.Contains(name, "__") { + name = strings.ReplaceAll(name, "__", "_") + } + + return name +} + +// sanitizeMetricNameRune returns the rune if it's valid for a metric name, +// otherwise returns '_'. +// Valid characters: ASCII letters (a-z, A-Z), ASCII digits (0-9), underscore, colon, dot. +func sanitizeMetricNameRune(r rune) rune { + if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '_' || r == ':' || r == '.' { + return r + } + return '_' +} diff --git a/exporter/bmchelixexporter/internal/operationsmanagement/normalize_metric_name_test.go b/exporter/bmchelixexporter/internal/operationsmanagement/normalize_metric_name_test.go new file mode 100644 index 0000000000000..98ab74ae00c37 --- /dev/null +++ b/exporter/bmchelixexporter/internal/operationsmanagement/normalize_metric_name_test.go @@ -0,0 +1,145 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package operationsmanagement + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNormalizeMetricName(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "empty string", + input: "", + expected: "", + }, + { + name: "valid metric name", + input: "system.cpu.usage", + expected: "system.cpu.usage", + }, + { + name: "metric name with colons", + input: "system:cpu:usage", + expected: "system:cpu:usage", + }, + { + name: "metric name with underscores", + input: "system_cpu_usage", + expected: "system_cpu_usage", + }, + { + name: "metric name starting with digit", + input: "123metric", + expected: "_123metric", + }, + { + name: "metric name with spaces", + input: "system cpu usage", + expected: "system_cpu_usage", + }, + { + name: "metric name with special chars", + input: "system/cpu\\usage", + expected: "system_cpu_usage", + }, + { + name: "metric name with brackets", + input: "system[cpu](usage)", + expected: "system_cpu_usage_", + }, + { + name: "metric name with consecutive special chars", + input: "system..cpu//usage", + expected: "system..cpu_usage", + }, + { + name: "metric name with leading/trailing underscores", + input: "_system_cpu_usage_", + expected: "_system_cpu_usage_", + }, + { + name: "metric name with equals and semicolons", + input: "metric=value;test", + expected: "metric_value_test", + }, + { + name: "metric name from enriched attribute - eth0 receive", + input: "system.network.io.eth0.receive", + expected: "system.network.io.eth0.receive", + }, + { + name: "metric name with curly braces", + input: "metric{label}", + expected: "metric_label_", + }, + { + name: "metric name with only special characters results in underscore", + input: "!@#$%^&*()", + expected: "_", + }, + { + name: "metric name with only underscores collapses to single underscore", + input: "____", + expected: "_", + }, + { + name: "metric name that becomes all underscores after normalization", + input: "___!!!___", + expected: "_", + }, + { + name: "metric name with unicode letters is sanitized to ASCII only", + input: "café_metric", + expected: "caf_metric", + }, + { + name: "metric name with unicode digits is sanitized to ASCII only", + input: "metric_\u0660\u0661\u0662", + expected: "metric_", + }, + { + name: "metric name with unicode in the middle is sanitized", + input: "metric_\u0660_test", + expected: "metric_test", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := NormalizeMetricName(tt.input) + require.Equal(t, tt.expected, result) + }) + } +} + +func TestSanitizeMetricNameRune(t *testing.T) { + // Valid runes should be returned as-is + require.Equal(t, 'a', sanitizeMetricNameRune('a')) + require.Equal(t, 'Z', sanitizeMetricNameRune('Z')) + require.Equal(t, '5', sanitizeMetricNameRune('5')) + require.Equal(t, '_', sanitizeMetricNameRune('_')) + require.Equal(t, ':', sanitizeMetricNameRune(':')) + require.Equal(t, '.', sanitizeMetricNameRune('.')) + + // Invalid runes should be replaced with '_' + require.Equal(t, '_', sanitizeMetricNameRune(' ')) + require.Equal(t, '_', sanitizeMetricNameRune('/')) + require.Equal(t, '_', sanitizeMetricNameRune('\\')) + require.Equal(t, '_', sanitizeMetricNameRune('[')) + require.Equal(t, '_', sanitizeMetricNameRune(']')) + require.Equal(t, '_', sanitizeMetricNameRune('{')) + require.Equal(t, '_', sanitizeMetricNameRune('}')) + + // Unicode letters and digits should be replaced with '_' + require.Equal(t, '_', sanitizeMetricNameRune('é')) + require.Equal(t, '_', sanitizeMetricNameRune('ñ')) + require.Equal(t, '_', sanitizeMetricNameRune('\u0660')) // Arabic-Indic digit zero +} diff --git a/exporter/bmchelixexporter/testdata/config.yaml b/exporter/bmchelixexporter/testdata/config.yaml index 51156b1deb055..f665653b10a6e 100644 --- a/exporter/bmchelixexporter/testdata/config.yaml +++ b/exporter/bmchelixexporter/testdata/config.yaml @@ -1,10 +1,12 @@ bmchelix/helix1: endpoint: https://helix1:8080 api_key: api_key + enrich_metric_with_attributes: true bmchelix/helix2: endpoint: https://helix2:8080 api_key: api_key timeout: 20s + enrich_metric_with_attributes: true retry_on_failure: enabled: true initial_interval: 5s