Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[receivers/redis] Remove service_name config and type datapoint label #3808

Merged
merged 1 commit into from
Jun 22, 2021
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ The OpenTelemetry Collector Contrib contains everything in the [opentelemetry-co

- `f5cloud` exporter (#3509):
- Renamed the config 'auth' field to 'f5cloud_auth'. This will prevent a config field name collision when [Support for Custom Exporter Authenticators as Extensions](https://github.com/open-telemetry/opentelemetry-collector/pull/3128) is ready to be integrated.
- `redis` receiver (#3808)
- removed configuration `service_name`. Use resource processor or `resource_attributes` setting if using `receivercreator`
- removed `type` label and set instrumentation library name to `otelcol/redis` as other receivers do

## 💡 Enhancements 💡

Expand Down
10 changes: 6 additions & 4 deletions receiver/receivercreator/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,10 @@ receivers:
# Static receiver-specific config.
password: secret
# Dynamic configuration value.
service_name: `pod.labels["service_name"]`
collection_interval: `pod.annotations["collection_interval"]`
resource_attributes:
# Dynamic configuration value.
service.name: `pod.labels["service_name"]`

redis/2:
# Set a resource attribute based on endpoint value.
Expand All @@ -170,9 +173,8 @@ receivers:
redis/on_host:
# If this rule matches an instance of this receiver will be started.
rule: type == "port" && port == 6379 && is_ipv6 == true
config:
service_name: redis_on_host

resource_attributes:
service.name: redis_on_host

processors:
exampleprocessor:
Expand Down
5 changes: 0 additions & 5 deletions receiver/redisreceiver/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ The following settings are required:

- `endpoint` (no default): The hostname and port of the Redis instance,
separated by a colon.
- `service_name` (no default): The logical name of the Redis server. This
value will be added as a `service_name` Resource label and may end up as a
dimension on exported metrics, depending on the exporter.

The following settings are optional:

Expand All @@ -66,7 +63,6 @@ Example:
receivers:
redis:
endpoint: "localhost:6379"
service_name: "my-test-redis"
collection_interval: 10s
password: $REDIS_PASSWORD
```
Expand All @@ -80,7 +76,6 @@ configuration like the following:
receivers:
redis:
endpoint: "localhost:6379"
service_name: "my-test-redis"
collection_interval: 10s
password: $REDIS_PASSWORD
```
Expand Down
3 changes: 0 additions & 3 deletions receiver/redisreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ type Config struct {
Endpoint string `mapstructure:"endpoint"`
// The duration between Redis metric fetches.
CollectionInterval time.Duration `mapstructure:"collection_interval"`
// The logical name of the Redis server. This value will be added as a
// "service.name" Resource label.
ServiceName string `mapstructure:"service_name"`

// TODO allow users to add additional resource key value pairs?

Expand Down
12 changes: 0 additions & 12 deletions receiver/redisreceiver/pdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,6 @@ import (
"go.opentelemetry.io/collector/consumer/pdata"
)

func newResourceMetrics(ms pdata.MetricSlice, serviceName string) pdata.ResourceMetrics {
rm := pdata.NewResourceMetrics()
r := rm.Resource()
rattrs := r.Attributes()
rattrs.Insert("type", pdata.NewAttributeValueString(typeStr))
rattrs.Insert("service.name", pdata.NewAttributeValueString(serviceName))
ilm := pdata.NewInstrumentationLibraryMetrics()
rm.InstrumentationLibraryMetrics().Append(ilm)
ms.CopyTo(ilm.Metrics())
return rm
}

func buildKeyspaceTriplet(k *keyspace, t *timeBundle) pdata.MetricSlice {
ms := pdata.NewMetricSlice()
ms.Resize(3)
Expand Down
39 changes: 20 additions & 19 deletions receiver/redisreceiver/pdata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,8 @@ import (
"go.opentelemetry.io/collector/consumer/pdata"
)

func TestServiceName(t *testing.T) {
const serviceName = "foo-service"
rm := testGetMetricData(t, usedMemory(), serviceName)
const key = "service.name"
attrVal, ok := rm.Resource().Attributes().Get(key)
assert.True(t, ok)
assert.Equal(t, serviceName, attrVal.StringVal())
}

func TestMemoryMetric(t *testing.T) {
rm := testGetMetricData(t, usedMemory(), "")
rm := testGetMetricData(t, usedMemory())

const metricName = "redis/memory/used"
const desc = "Total number of bytes allocated by Redis using its allocator"
Expand All @@ -42,10 +33,11 @@ func TestMemoryMetric(t *testing.T) {

ilms := rm.InstrumentationLibraryMetrics()
assert.Equal(t, 1, ilms.Len())
assert.Equal(t, 2, rm.Resource().Attributes().Len())
assert.Equal(t, 0, rm.Resource().Attributes().Len())
ilm := ilms.At(0)
ms := ilm.Metrics()
m := ms.At(0)
assert.Equal(t, "otelcol/redis", ilm.InstrumentationLibrary().Name())
assert.Equal(t, metricName, m.Name())
assert.Equal(t, desc, m.Description())
assert.Equal(t, units, m.Unit())
Expand All @@ -54,7 +46,7 @@ func TestMemoryMetric(t *testing.T) {
}

func TestUptimeInSeconds(t *testing.T) {
pdm := testGetMetric(t, uptimeInSeconds(), "")
pdm := testGetMetric(t, uptimeInSeconds())
const units = "s"
v := 104946

Expand All @@ -63,7 +55,7 @@ func TestUptimeInSeconds(t *testing.T) {
}

func TestUsedCpuSys(t *testing.T) {
pdm := testGetMetricData(t, usedCPUSys(), "")
pdm := testGetMetricData(t, usedCPUSys())
const units = "s"
v := 185.649184

Expand Down Expand Up @@ -171,6 +163,15 @@ func TestNewPDM(t *testing.T) {
assert.Equal(t, serverStartTime, pdm.DoubleSum().DataPoints().At(0).StartTimestamp())
}

func newResourceMetrics(ms pdata.MetricSlice) pdata.ResourceMetrics {
rm := pdata.NewResourceMetrics()
ilm := pdata.NewInstrumentationLibraryMetrics()
ilm.InstrumentationLibrary().SetName("otelcol/redis")
rm.InstrumentationLibraryMetrics().Append(ilm)
ms.CopyTo(ilm.Metrics())
return rm
}

func testFetchMetrics(redisMetrics []*redisMetric) (pdata.MetricSlice, []error, error) {
svc := newRedisSvc(newFakeClient())
info, err := svc.info()
Expand All @@ -181,28 +182,28 @@ func testFetchMetrics(redisMetrics []*redisMetric) (pdata.MetricSlice, []error,
return ms, warnings, nil
}

func testGetMetric(t *testing.T, redisMetric *redisMetric, serverName string) pdata.Metric {
rm := testGetMetricData(t, redisMetric, serverName)
func testGetMetric(t *testing.T, redisMetric *redisMetric) pdata.Metric {
rm := testGetMetricData(t, redisMetric)
pdm := rm.InstrumentationLibraryMetrics().At(0).Metrics().At(0)
return pdm
}

func testGetMetricData(t *testing.T, metric *redisMetric, serverName string) pdata.ResourceMetrics {
rm, warnings, err := testGetMetricDataErr(metric, serverName)
func testGetMetricData(t *testing.T, metric *redisMetric) pdata.ResourceMetrics {
rm, warnings, err := testGetMetricDataErr(metric)
require.Nil(t, err)
require.Nil(t, warnings)
return rm
}

func testGetMetricDataErr(metric *redisMetric, serverName string) (pdata.ResourceMetrics, []error, error) {
func testGetMetricDataErr(metric *redisMetric) (pdata.ResourceMetrics, []error, error) {
redisMetrics := []*redisMetric{metric}
svc := newRedisSvc(newFakeClient())
info, err := svc.info()
if err != nil {
return pdata.ResourceMetrics{}, nil, err
}
ms, warnings := info.buildFixedMetrics(redisMetrics, testTimeBundle())
rm := newResourceMetrics(ms, serverName)
rm := newResourceMetrics(ms)
return rm, warnings, nil
}

Expand Down
2 changes: 1 addition & 1 deletion receiver/redisreceiver/receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (r *redisReceiver) Start(ctx context.Context, host component.Host) error {
Addr: r.config.Endpoint,
Password: r.config.Password,
})
redisRunnable := newRedisRunnable(ctx, r.config.ID(), c, r.config.ServiceName, r.consumer, r.logger)
redisRunnable := newRedisRunnable(ctx, r.config.ID(), c, r.consumer, r.logger)
r.intervalRunner = interval.NewRunner(r.config.CollectionInterval, redisRunnable)

go func() {
Expand Down
7 changes: 1 addition & 6 deletions receiver/redisreceiver/redis_runnable.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,19 @@ type redisRunnable struct {
redisMetrics []*redisMetric
logger *zap.Logger
timeBundle *timeBundle
serviceName string
obsrecv *obsreport.Receiver
}

func newRedisRunnable(
ctx context.Context,
id config.ComponentID,
client client,
serviceName string,
metricsConsumer consumer.Metrics,
logger *zap.Logger,
) *redisRunnable {
return &redisRunnable{
id: id,
ctx: ctx,
serviceName: serviceName,
redisSvc: newRedisSvc(client),
metricsConsumer: metricsConsumer,
logger: logger,
Expand Down Expand Up @@ -100,10 +97,8 @@ func (r *redisRunnable) Run() error {

pdm := pdata.NewMetrics()
rm := pdm.ResourceMetrics().AppendEmpty()
resource := rm.Resource()
rattrs := resource.Attributes()
rattrs.InsertString("service.name", r.serviceName)
ilm := rm.InstrumentationLibraryMetrics().AppendEmpty()
ilm.InstrumentationLibrary().SetName("otelcol/" + typeStr)
fixedMS, warnings := inf.buildFixedMetrics(r.redisMetrics, r.timeBundle)
fixedMS.MoveAndAppendTo(ilm.Metrics())
if warnings != nil {
Expand Down
11 changes: 9 additions & 2 deletions receiver/redisreceiver/redis_runnable_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/consumer/consumertest"
Expand All @@ -27,11 +28,17 @@ import (
func TestRedisRunnable(t *testing.T) {
consumer := new(consumertest.MetricsSink)
logger, _ := zap.NewDevelopment()
runner := newRedisRunnable(context.Background(), config.NewID(typeStr), newFakeClient(), "", consumer, logger)
runner := newRedisRunnable(context.Background(), config.NewID(typeStr), newFakeClient(), consumer, logger)
err := runner.Setup()
require.Nil(t, err)
err = runner.Run()
require.Nil(t, err)
// + 6 because there are two keyspace entries each of which has three metrics
require.Equal(t, len(getDefaultRedisMetrics())+6, consumer.MetricsCount())
assert.Equal(t, len(getDefaultRedisMetrics())+6, consumer.MetricsCount())
md := consumer.AllMetrics()[0]
rm := md.ResourceMetrics().At(0)
ilm := rm.InstrumentationLibraryMetrics().At(0)
il := ilm.InstrumentationLibrary()
assert.Equal(t, "otelcol/redis", il.Name())

}