From dbdb683404d22c8472c0b43fcc442651acd85b8d Mon Sep 17 00:00:00 2001 From: Christian Date: Tue, 14 Nov 2023 16:30:54 +0100 Subject: [PATCH] [receiver/redis] include server.address and server.port resource attributes (#26707) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Description:** Two new resource attributes have been added: `server.address` and `server.port`. This is change aligned with [semantic conventions](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.25.0/specification/semantic-conventions.md?plain=1#L30) and allows users identifying a particular redis server. **Link to tracking Issue:** #22044 **Testing:** ``` ❯ go test -race -timeout 300s -parallel 4 --tags="",integration --count 1 ./... ok github.com/open-telemetry/opentelemetry-collector-contrib/receiver/redisreceiver 6.179s ok github.com/open-telemetry/opentelemetry-collector-contrib/receiver/redisreceiver/internal/metadata 1.464s ``` * Added: new test to check error when the configuration endpoint is missing or invalid * Updated: integration test to check new resource attributes **Documentation:** New attribute documentation was generated using `mdatagen` --------- Co-authored-by: Antoine Toulme --- ...edis-receiver-server-address-and-port.yaml | 27 +++++++++++++++++++ cmd/otelcontribcol/receivers_test.go | 6 +++++ receiver/redisreceiver/config.go | 17 ++++++++++++ receiver/redisreceiver/documentation.md | 2 ++ receiver/redisreceiver/integration_test.go | 6 +++++ .../internal/metadata/generated_config.go | 10 ++++++- .../metadata/generated_config_test.go | 16 ++++++++--- .../metadata/generated_metrics_test.go | 2 ++ .../internal/metadata/generated_resource.go | 14 ++++++++++ .../metadata/generated_resource_test.go | 14 +++++++++- .../internal/metadata/testdata/config.yaml | 8 ++++++ receiver/redisreceiver/metadata.yaml | 8 ++++++ receiver/redisreceiver/redis_scraper.go | 26 +++++++++++------- receiver/redisreceiver/redis_scraper_test.go | 8 ++++++ 14 files changed, 149 insertions(+), 15 deletions(-) create mode 100755 .chloggen/redis-receiver-server-address-and-port.yaml diff --git a/.chloggen/redis-receiver-server-address-and-port.yaml b/.chloggen/redis-receiver-server-address-and-port.yaml new file mode 100755 index 000000000000..c04e81bb18fc --- /dev/null +++ b/.chloggen/redis-receiver-server-address-and-port.yaml @@ -0,0 +1,27 @@ +# 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: redisreceiver + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: include server.address and server.port resource attributes + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [22044] + +# (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: + +# 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: [user] diff --git a/cmd/otelcontribcol/receivers_test.go b/cmd/otelcontribcol/receivers_test.go index c6a993aad492..a33ecc5fbc7c 100644 --- a/cmd/otelcontribcol/receivers_test.go +++ b/cmd/otelcontribcol/receivers_test.go @@ -35,6 +35,7 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/mongodbatlasreceiver" "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/otlpjsonfilereceiver" "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/prometheusreceiver" + "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/redisreceiver" "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/snmpreceiver" "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/syslogreceiver" "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/tcplogreceiver" @@ -340,6 +341,11 @@ func TestDefaultReceivers(t *testing.T) { }, { receiver: "redis", + getConfigFn: func() component.Config { + cfg := rcvrFactories["redis"].CreateDefaultConfig().(*redisreceiver.Config) + cfg.Endpoint = "localhost:6379" + return cfg + }, }, { receiver: "riak", diff --git a/receiver/redisreceiver/config.go b/receiver/redisreceiver/config.go index d8580c690fd2..70870380dbe1 100644 --- a/receiver/redisreceiver/config.go +++ b/receiver/redisreceiver/config.go @@ -4,6 +4,9 @@ package redisreceiver // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/redisreceiver" import ( + "fmt" + "net" + "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/config/configopaque" "go.opentelemetry.io/collector/config/configtls" @@ -34,3 +37,17 @@ type Config struct { MetricsBuilderConfig metadata.MetricsBuilderConfig `mapstructure:",squash"` } + +// configInfo holds configuration information to be used as resource/metrics attributes. +type configInfo struct { + Address string + Port string +} + +func newConfigInfo(cfg *Config) (configInfo, error) { + address, port, err := net.SplitHostPort(cfg.Endpoint) + if err != nil { + return configInfo{}, fmt.Errorf("invalid endpoint %q: %w", cfg.Endpoint, err) + } + return configInfo{Address: address, Port: port}, nil +} diff --git a/receiver/redisreceiver/documentation.md b/receiver/redisreceiver/documentation.md index 550f36253e66..ff610dd2d5db 100644 --- a/receiver/redisreceiver/documentation.md +++ b/receiver/redisreceiver/documentation.md @@ -348,3 +348,5 @@ Redis node's role | Name | Description | Values | Enabled | | ---- | ----------- | ------ | ------- | | redis.version | Redis server's version. | Any Str | true | +| server.address | Redis server's address | Any Str | false | +| server.port | Redis server's port | Any Str | false | diff --git a/receiver/redisreceiver/integration_test.go b/receiver/redisreceiver/integration_test.go index 1b18cf694c1d..4d64d6e9eddf 100644 --- a/receiver/redisreceiver/integration_test.go +++ b/receiver/redisreceiver/integration_test.go @@ -39,6 +39,12 @@ func TestIntegration(t *testing.T) { pmetrictest.IgnoreMetricDataPointsOrder(), pmetrictest.IgnoreStartTimestamp(), pmetrictest.IgnoreTimestamp(), + pmetrictest.ChangeResourceAttributeValue("server.address", func(_ string) string { + return "localhost" + }), + pmetrictest.ChangeResourceAttributeValue("server.port", func(_ string) string { + return redisPort + }), ), ).Run(t) } diff --git a/receiver/redisreceiver/internal/metadata/generated_config.go b/receiver/redisreceiver/internal/metadata/generated_config.go index 3f7be28c1b6f..023ef4e5826d 100644 --- a/receiver/redisreceiver/internal/metadata/generated_config.go +++ b/receiver/redisreceiver/internal/metadata/generated_config.go @@ -189,7 +189,9 @@ func (rac *ResourceAttributeConfig) Unmarshal(parser *confmap.Conf) error { // ResourceAttributesConfig provides config for redis resource attributes. type ResourceAttributesConfig struct { - RedisVersion ResourceAttributeConfig `mapstructure:"redis.version"` + RedisVersion ResourceAttributeConfig `mapstructure:"redis.version"` + ServerAddress ResourceAttributeConfig `mapstructure:"server.address"` + ServerPort ResourceAttributeConfig `mapstructure:"server.port"` } func DefaultResourceAttributesConfig() ResourceAttributesConfig { @@ -197,6 +199,12 @@ func DefaultResourceAttributesConfig() ResourceAttributesConfig { RedisVersion: ResourceAttributeConfig{ Enabled: true, }, + ServerAddress: ResourceAttributeConfig{ + Enabled: false, + }, + ServerPort: ResourceAttributeConfig{ + Enabled: false, + }, } } diff --git a/receiver/redisreceiver/internal/metadata/generated_config_test.go b/receiver/redisreceiver/internal/metadata/generated_config_test.go index f0043cd73831..1c37d02d5b19 100644 --- a/receiver/redisreceiver/internal/metadata/generated_config_test.go +++ b/receiver/redisreceiver/internal/metadata/generated_config_test.go @@ -62,7 +62,9 @@ func TestMetricsBuilderConfig(t *testing.T) { RedisUptime: MetricConfig{Enabled: true}, }, ResourceAttributes: ResourceAttributesConfig{ - RedisVersion: ResourceAttributeConfig{Enabled: true}, + RedisVersion: ResourceAttributeConfig{Enabled: true}, + ServerAddress: ResourceAttributeConfig{Enabled: true}, + ServerPort: ResourceAttributeConfig{Enabled: true}, }, }, }, @@ -106,7 +108,9 @@ func TestMetricsBuilderConfig(t *testing.T) { RedisUptime: MetricConfig{Enabled: false}, }, ResourceAttributes: ResourceAttributesConfig{ - RedisVersion: ResourceAttributeConfig{Enabled: false}, + RedisVersion: ResourceAttributeConfig{Enabled: false}, + ServerAddress: ResourceAttributeConfig{Enabled: false}, + ServerPort: ResourceAttributeConfig{Enabled: false}, }, }, }, @@ -143,13 +147,17 @@ func TestResourceAttributesConfig(t *testing.T) { { name: "all_set", want: ResourceAttributesConfig{ - RedisVersion: ResourceAttributeConfig{Enabled: true}, + RedisVersion: ResourceAttributeConfig{Enabled: true}, + ServerAddress: ResourceAttributeConfig{Enabled: true}, + ServerPort: ResourceAttributeConfig{Enabled: true}, }, }, { name: "none_set", want: ResourceAttributesConfig{ - RedisVersion: ResourceAttributeConfig{Enabled: false}, + RedisVersion: ResourceAttributeConfig{Enabled: false}, + ServerAddress: ResourceAttributeConfig{Enabled: false}, + ServerPort: ResourceAttributeConfig{Enabled: false}, }, }, } diff --git a/receiver/redisreceiver/internal/metadata/generated_metrics_test.go b/receiver/redisreceiver/internal/metadata/generated_metrics_test.go index 96dfe2712b85..82d698794747 100644 --- a/receiver/redisreceiver/internal/metadata/generated_metrics_test.go +++ b/receiver/redisreceiver/internal/metadata/generated_metrics_test.go @@ -188,6 +188,8 @@ func TestMetricsBuilder(t *testing.T) { rb := mb.NewResourceBuilder() rb.SetRedisVersion("redis.version-val") + rb.SetServerAddress("server.address-val") + rb.SetServerPort("server.port-val") res := rb.Emit() metrics := mb.Emit(WithResource(res)) diff --git a/receiver/redisreceiver/internal/metadata/generated_resource.go b/receiver/redisreceiver/internal/metadata/generated_resource.go index 24919b5b8310..a4b8a2495eac 100644 --- a/receiver/redisreceiver/internal/metadata/generated_resource.go +++ b/receiver/redisreceiver/internal/metadata/generated_resource.go @@ -28,6 +28,20 @@ func (rb *ResourceBuilder) SetRedisVersion(val string) { } } +// SetServerAddress sets provided value as "server.address" attribute. +func (rb *ResourceBuilder) SetServerAddress(val string) { + if rb.config.ServerAddress.Enabled { + rb.res.Attributes().PutStr("server.address", val) + } +} + +// SetServerPort sets provided value as "server.port" attribute. +func (rb *ResourceBuilder) SetServerPort(val string) { + if rb.config.ServerPort.Enabled { + rb.res.Attributes().PutStr("server.port", val) + } +} + // Emit returns the built resource and resets the internal builder state. func (rb *ResourceBuilder) Emit() pcommon.Resource { r := rb.res diff --git a/receiver/redisreceiver/internal/metadata/generated_resource_test.go b/receiver/redisreceiver/internal/metadata/generated_resource_test.go index 5aa1c95c3a68..39f77dbb710d 100644 --- a/receiver/redisreceiver/internal/metadata/generated_resource_test.go +++ b/receiver/redisreceiver/internal/metadata/generated_resource_test.go @@ -14,6 +14,8 @@ func TestResourceBuilder(t *testing.T) { cfg := loadResourceAttributesConfig(t, test) rb := NewResourceBuilder(cfg) rb.SetRedisVersion("redis.version-val") + rb.SetServerAddress("server.address-val") + rb.SetServerPort("server.port-val") res := rb.Emit() assert.Equal(t, 0, rb.Emit().Attributes().Len()) // Second call should return empty Resource @@ -22,7 +24,7 @@ func TestResourceBuilder(t *testing.T) { case "default": assert.Equal(t, 1, res.Attributes().Len()) case "all_set": - assert.Equal(t, 1, res.Attributes().Len()) + assert.Equal(t, 3, res.Attributes().Len()) case "none_set": assert.Equal(t, 0, res.Attributes().Len()) return @@ -35,6 +37,16 @@ func TestResourceBuilder(t *testing.T) { if ok { assert.EqualValues(t, "redis.version-val", val.Str()) } + val, ok = res.Attributes().Get("server.address") + assert.Equal(t, test == "all_set", ok) + if ok { + assert.EqualValues(t, "server.address-val", val.Str()) + } + val, ok = res.Attributes().Get("server.port") + assert.Equal(t, test == "all_set", ok) + if ok { + assert.EqualValues(t, "server.port-val", val.Str()) + } }) } } diff --git a/receiver/redisreceiver/internal/metadata/testdata/config.yaml b/receiver/redisreceiver/internal/metadata/testdata/config.yaml index 33972c5b607c..a392e0078154 100644 --- a/receiver/redisreceiver/internal/metadata/testdata/config.yaml +++ b/receiver/redisreceiver/internal/metadata/testdata/config.yaml @@ -72,6 +72,10 @@ all_set: resource_attributes: redis.version: enabled: true + server.address: + enabled: true + server.port: + enabled: true none_set: metrics: redis.clients.blocked: @@ -145,3 +149,7 @@ none_set: resource_attributes: redis.version: enabled: false + server.address: + enabled: false + server.port: + enabled: false diff --git a/receiver/redisreceiver/metadata.yaml b/receiver/redisreceiver/metadata.yaml index 6a6aa03e0a0a..b4ac8273884e 100644 --- a/receiver/redisreceiver/metadata.yaml +++ b/receiver/redisreceiver/metadata.yaml @@ -13,6 +13,14 @@ resource_attributes: description: Redis server's version. enabled: true type: string + server.address: + description: Redis server's address + enabled: false + type: string + server.port: + description: Redis server's port + enabled: false + type: string attributes: state: diff --git a/receiver/redisreceiver/redis_scraper.go b/receiver/redisreceiver/redis_scraper.go index 76531be35e69..e735f21ec9d1 100644 --- a/receiver/redisreceiver/redis_scraper.go +++ b/receiver/redisreceiver/redis_scraper.go @@ -23,11 +23,12 @@ import ( // Runs intermittently, fetching info from Redis, creating metrics/datapoints, // and feeding them to a metricsConsumer. type redisScraper struct { - client client - redisSvc *redisSvc - settings component.TelemetrySettings - mb *metadata.MetricsBuilder - uptime time.Duration + client client + redisSvc *redisSvc + settings component.TelemetrySettings + mb *metadata.MetricsBuilder + uptime time.Duration + configInfo configInfo } const redisMaxDbs = 16 // Maximum possible number of redis databases @@ -48,11 +49,16 @@ func newRedisScraper(cfg *Config, settings receiver.CreateSettings) (scraperhelp } func newRedisScraperWithClient(client client, settings receiver.CreateSettings, cfg *Config) (scraperhelper.Scraper, error) { + configInfo, err := newConfigInfo(cfg) + if err != nil { + return nil, err + } rs := &redisScraper{ - client: client, - redisSvc: newRedisSvc(client), - settings: settings.TelemetrySettings, - mb: metadata.NewMetricsBuilder(cfg.MetricsBuilderConfig, settings), + client: client, + redisSvc: newRedisSvc(client), + settings: settings.TelemetrySettings, + mb: metadata.NewMetricsBuilder(cfg.MetricsBuilderConfig, settings), + configInfo: configInfo, } return scraperhelper.NewScraper( metadata.Type, @@ -96,6 +102,8 @@ func (rs *redisScraper) Scrape(context.Context) (pmetric.Metrics, error) { rs.recordCmdMetrics(now, inf) rb := rs.mb.NewResourceBuilder() rb.SetRedisVersion(rs.getRedisVersion(inf)) + rb.SetServerAddress(rs.configInfo.Address) + rb.SetServerPort(rs.configInfo.Port) return rs.mb.Emit(metadata.WithResource(rb.Emit())), nil } diff --git a/receiver/redisreceiver/redis_scraper_test.go b/receiver/redisreceiver/redis_scraper_test.go index c3f55d518b32..49379ded9cdc 100644 --- a/receiver/redisreceiver/redis_scraper_test.go +++ b/receiver/redisreceiver/redis_scraper_test.go @@ -21,6 +21,7 @@ func TestRedisRunnable(t *testing.T) { settings := receivertest.NewNopCreateSettings() settings.Logger = logger cfg := createDefaultConfig().(*Config) + cfg.Endpoint = "localhost:6379" rs := &redisScraper{mb: metadata.NewMetricsBuilder(cfg.MetricsBuilderConfig, settings)} runner, err := newRedisScraperWithClient(newFakeClient(), settings, cfg) require.NoError(t, err) @@ -35,6 +36,13 @@ func TestRedisRunnable(t *testing.T) { assert.Equal(t, "otelcol/redisreceiver", il.Name()) } +func TestNewReceiver_invalid_endpoint(t *testing.T) { + c := createDefaultConfig().(*Config) + _, err := createMetricsReceiver(context.Background(), receivertest.NewNopCreateSettings(), c, nil) + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid endpoint") +} + func TestNewReceiver_invalid_auth_error(t *testing.T) { c := createDefaultConfig().(*Config) c.TLS = configtls.TLSClientSetting{