Skip to content

Commit

Permalink
[receiver/hostmetrics] remove direction feature gate (open-telemetry#…
Browse files Browse the repository at this point in the history
…14959)

The following feature gates have been removed after being deprecated for a few versions:

- `receiver.hostmetricsreceiver.emitMetricsWithoutDirectionAttribute`
- `receiver.hostmetricsreceiver.emitMetricsWithDirectionAttribute`
  • Loading branch information
Alex Boten authored and shalper2 committed Dec 6, 2022
1 parent 80e5af8 commit e99d953
Show file tree
Hide file tree
Showing 27 changed files with 212 additions and 2,078 deletions.
16 changes: 16 additions & 0 deletions .chloggen/rm-direction-hostmetrics.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

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

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "remove direction feature gate"

# One or more tracking issues related to the change
issues: [14959]

# (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:
17 changes: 0 additions & 17 deletions receiver/hostmetricsreceiver/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,23 +141,6 @@ service:
receivers: [hostmetrics, hostmetrics/disk]
```

### Feature gate configurations

#### Transition from metrics with "direction" attribute

The proposal to change metrics from being reported with a `direction` attribute has been reverted in the specification. As a result, the
following feature gates will be removed in v0.62.0:

- **receiver.hostmetricsreceiver.emitMetricsWithoutDirectionAttribute**
- **receiver.hostmetricsreceiver.emitMetricsWithDirectionAttribute**

For additional information, see https://github.com/open-telemetry/opentelemetry-specification/issues/2726.

##### More information:

- https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/11815
- https://github.com/open-telemetry/opentelemetry-specification/pull/2617

[beta]: https://github.com/open-telemetry/opentelemetry-collector#beta
[contrib]: https://github.com/open-telemetry/opentelemetry-collector-releases/tree/main/distributions/otelcol-contrib
[core]: https://github.com/open-telemetry/opentelemetry-collector-releases/tree/main/distributions/otelcol
Expand Down
15 changes: 0 additions & 15 deletions receiver/hostmetricsreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ import (
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/featuregate"
"go.opentelemetry.io/collector/receiver/scraperhelper"
"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/hostmetricsreceiver/internal"
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/hostmetricsreceiver/internal/scraper/cpuscraper"
Expand Down Expand Up @@ -103,12 +101,6 @@ func createMetricsReceiver(
)
}

func logDeprecatedFeatureGateForDirection(log *zap.Logger, gateID string) {
log.Warn("WARNING: The " + gateID + " feature gate is deprecated and will be removed in the next release. The change to remove " +
"the direction attribute has been reverted in the specification. See https://github.com/open-telemetry/opentelemetry-specification/issues/2726 " +
"for additional details.")
}

func createAddScraperOptions(
ctx context.Context,
set component.ReceiverCreateSettings,
Expand All @@ -131,13 +123,6 @@ func createAddScraperOptions(
return nil, fmt.Errorf("host metrics scraper factory not found for key: %q", key)
}

if !featuregate.GetRegistry().IsEnabled(internal.EmitMetricsWithDirectionAttributeFeatureGateID) {
logDeprecatedFeatureGateForDirection(set.Logger, internal.EmitMetricsWithDirectionAttributeFeatureGateID)
}
if featuregate.GetRegistry().IsEnabled(internal.EmitMetricsWithoutDirectionAttributeFeatureGateID) {
logDeprecatedFeatureGateForDirection(set.Logger, internal.EmitMetricsWithoutDirectionAttributeFeatureGateID)
}

return scraperControllerOptions, nil
}

Expand Down
33 changes: 0 additions & 33 deletions receiver/hostmetricsreceiver/internal/scraper.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,42 +18,9 @@ import (
"context"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/featuregate"
"go.opentelemetry.io/collector/receiver/scraperhelper"
)

const (
EmitMetricsWithDirectionAttributeFeatureGateID = "receiver.hostmetricsreceiver.emitMetricsWithDirectionAttribute"
EmitMetricsWithoutDirectionAttributeFeatureGateID = "receiver.hostmetricsreceiver.emitMetricsWithoutDirectionAttribute"
)

var (
emitMetricsWithDirectionAttributeFeatureGate = featuregate.Gate{
ID: EmitMetricsWithDirectionAttributeFeatureGateID,
Enabled: true,
Description: "Some process host metrics reported are transitioning from being reported with a direction " +
"attribute to being reported with the direction included in the metric name to adhere to the " +
"OpenTelemetry specification. This feature gate controls emitting the old metrics with the direction " +
"attribute. For more details, see: " +
"https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/hostmetricsreceiver/README.md#feature-gate-configurations",
}

emitMetricsWithoutDirectionAttributeFeatureGate = featuregate.Gate{
ID: EmitMetricsWithoutDirectionAttributeFeatureGateID,
Enabled: false,
Description: "Some process host metrics reported are transitioning from being reported with a direction " +
"attribute to being reported with the direction included in the metric name to adhere to the " +
"OpenTelemetry specification. This feature gate controls emitting the new metrics without the direction " +
"attribute. For more details, see: " +
"https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/hostmetricsreceiver/README.md#feature-gate-configurations",
}
)

func init() {
featuregate.GetRegistry().MustRegister(emitMetricsWithDirectionAttributeFeatureGate)
featuregate.GetRegistry().MustRegister(emitMetricsWithoutDirectionAttributeFeatureGate)
}

// ScraperFactory can create a MetricScraper.
type ScraperFactory interface {
// CreateDefaultConfig creates the default configuration for the Scraper.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,11 @@ import (
"github.com/shirou/gopsutil/v3/disk"
"github.com/shirou/gopsutil/v3/host"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/featuregate"
"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/pmetric"
"go.opentelemetry.io/collector/receiver/scrapererror"

"github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/processor/filterset"
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/hostmetricsreceiver/internal"
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/hostmetricsreceiver/internal/scraper/diskscraper/internal/metadata"
)

Expand All @@ -50,17 +48,13 @@ type scraper struct {
excludeFS filterset.FilterSet

// for mocking
bootTime func() (uint64, error)
ioCounters func(names ...string) (map[string]disk.IOCountersStat, error)
emitMetricsWithDirectionAttribute bool
emitMetricsWithoutDirectionAttribute bool
bootTime func() (uint64, error)
ioCounters func(names ...string) (map[string]disk.IOCountersStat, error)
}

// newDiskScraper creates a Disk Scraper
func newDiskScraper(_ context.Context, settings component.ReceiverCreateSettings, cfg *Config) (*scraper, error) {
scraper := &scraper{settings: settings, config: cfg, bootTime: host.BootTime, ioCounters: disk.IOCounters}
scraper.emitMetricsWithDirectionAttribute = featuregate.GetRegistry().IsEnabled(internal.EmitMetricsWithDirectionAttributeFeatureGateID)
scraper.emitMetricsWithoutDirectionAttribute = featuregate.GetRegistry().IsEnabled(internal.EmitMetricsWithoutDirectionAttributeFeatureGateID)

var err error

Expand Down Expand Up @@ -116,27 +110,15 @@ func (s *scraper) scrape(_ context.Context) (pmetric.Metrics, error) {

func (s *scraper) recordDiskIOMetric(now pcommon.Timestamp, ioCounters map[string]disk.IOCountersStat) {
for device, ioCounter := range ioCounters {
if s.emitMetricsWithDirectionAttribute {
s.mb.RecordSystemDiskIoDataPoint(now, int64(ioCounter.ReadBytes), device, metadata.AttributeDirectionRead)
s.mb.RecordSystemDiskIoDataPoint(now, int64(ioCounter.WriteBytes), device, metadata.AttributeDirectionWrite)
}
if s.emitMetricsWithoutDirectionAttribute {
s.mb.RecordSystemDiskIoReadDataPoint(now, int64(ioCounter.ReadBytes), device)
s.mb.RecordSystemDiskIoWriteDataPoint(now, int64(ioCounter.WriteBytes), device)
}
s.mb.RecordSystemDiskIoDataPoint(now, int64(ioCounter.ReadBytes), device, metadata.AttributeDirectionRead)
s.mb.RecordSystemDiskIoDataPoint(now, int64(ioCounter.WriteBytes), device, metadata.AttributeDirectionWrite)
}
}

func (s *scraper) recordDiskOperationsMetric(now pcommon.Timestamp, ioCounters map[string]disk.IOCountersStat) {
for device, ioCounter := range ioCounters {
if s.emitMetricsWithDirectionAttribute {
s.mb.RecordSystemDiskOperationsDataPoint(now, int64(ioCounter.ReadCount), device, metadata.AttributeDirectionRead)
s.mb.RecordSystemDiskOperationsDataPoint(now, int64(ioCounter.WriteCount), device, metadata.AttributeDirectionWrite)
}
if s.emitMetricsWithoutDirectionAttribute {
s.mb.RecordSystemDiskOperationsReadDataPoint(now, int64(ioCounter.ReadCount), device)
s.mb.RecordSystemDiskOperationsWriteDataPoint(now, int64(ioCounter.WriteCount), device)
}
s.mb.RecordSystemDiskOperationsDataPoint(now, int64(ioCounter.ReadCount), device, metadata.AttributeDirectionRead)
s.mb.RecordSystemDiskOperationsDataPoint(now, int64(ioCounter.WriteCount), device, metadata.AttributeDirectionWrite)
}
}

Expand All @@ -148,14 +130,8 @@ func (s *scraper) recordDiskIOTimeMetric(now pcommon.Timestamp, ioCounters map[s

func (s *scraper) recordDiskOperationTimeMetric(now pcommon.Timestamp, ioCounters map[string]disk.IOCountersStat) {
for device, ioCounter := range ioCounters {
if s.emitMetricsWithDirectionAttribute {
s.mb.RecordSystemDiskOperationTimeDataPoint(now, float64(ioCounter.ReadTime)/1e3, device, metadata.AttributeDirectionRead)
s.mb.RecordSystemDiskOperationTimeDataPoint(now, float64(ioCounter.WriteTime)/1e3, device, metadata.AttributeDirectionWrite)
}
if s.emitMetricsWithoutDirectionAttribute {
s.mb.RecordSystemDiskOperationTimeReadDataPoint(now, float64(ioCounter.ReadTime)/1e3, device)
s.mb.RecordSystemDiskOperationTimeWriteDataPoint(now, float64(ioCounter.WriteTime)/1e3, device)
}
s.mb.RecordSystemDiskOperationTimeDataPoint(now, float64(ioCounter.ReadTime)/1e3, device, metadata.AttributeDirectionRead)
s.mb.RecordSystemDiskOperationTimeDataPoint(now, float64(ioCounter.WriteTime)/1e3, device, metadata.AttributeDirectionWrite)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,7 @@ func (s *scraper) recordDiskWeightedIOTimeMetric(now pcommon.Timestamp, ioCounte

func (s *scraper) recordDiskMergedMetric(now pcommon.Timestamp, ioCounters map[string]disk.IOCountersStat) {
for device, ioCounter := range ioCounters {
if s.emitMetricsWithDirectionAttribute {
s.mb.RecordSystemDiskMergedDataPoint(now, int64(ioCounter.MergedReadCount), device, metadata.AttributeDirectionRead)
s.mb.RecordSystemDiskMergedDataPoint(now, int64(ioCounter.MergedWriteCount), device, metadata.AttributeDirectionWrite)
}
if s.emitMetricsWithoutDirectionAttribute {
s.mb.RecordSystemDiskMergedReadDataPoint(now, int64(ioCounter.MergedReadCount), device)
s.mb.RecordSystemDiskMergedWriteDataPoint(now, int64(ioCounter.MergedWriteCount), device)
}
s.mb.RecordSystemDiskMergedDataPoint(now, int64(ioCounter.MergedReadCount), device, metadata.AttributeDirectionRead)
s.mb.RecordSystemDiskMergedDataPoint(now, int64(ioCounter.MergedWriteCount), device, metadata.AttributeDirectionWrite)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package diskscraper
import (
"context"
"errors"
"runtime"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -43,26 +42,12 @@ func TestScrape(t *testing.T) {
mutateScraper func(*scraper)
}

metricsWithDirection := 3
if runtime.GOOS == "linux" {
metricsWithDirection++
}

testCases := []testCase{
{
name: "Standard",
config: Config{Metrics: metadata.DefaultMetricsSettings()},
expectMetrics: metricsLen,
},
{
name: "With direction removed",
config: Config{Metrics: metadata.DefaultMetricsSettings()},
expectMetrics: metricsLen + metricsWithDirection,
mutateScraper: func(s *scraper) {
s.emitMetricsWithDirectionAttribute = false
s.emitMetricsWithoutDirectionAttribute = true
},
},
{
name: "Validate Start Time",
config: Config{Metrics: metadata.DefaultMetricsSettings()},
Expand Down Expand Up @@ -154,32 +139,16 @@ func TestScrape(t *testing.T) {
switch metric.Name() {
case "system.disk.io":
assertInt64DiskMetricValid(t, metric, true, test.expectedStartTime)
case "system.disk.io.read":
assertInt64DiskMetricValid(t, metric, false, test.expectedStartTime)
case "system.disk.io.write":
assertInt64DiskMetricValid(t, metric, false, test.expectedStartTime)
case "system.disk.io_time":
assertDoubleDiskMetricValid(t, metric, false, test.expectedStartTime)
case "system.disk.operation_time":
assertDoubleDiskMetricValid(t, metric, true, test.expectedStartTime)
case "system.disk.operation_time.read":
assertDoubleDiskMetricValid(t, metric, false, test.expectedStartTime)
case "system.disk.operation_time.write":
assertDoubleDiskMetricValid(t, metric, false, test.expectedStartTime)
case "system.disk.operations":
assertInt64DiskMetricValid(t, metric, true, test.expectedStartTime)
case "system.disk.operations.read":
assertInt64DiskMetricValid(t, metric, false, test.expectedStartTime)
case "system.disk.operations.write":
assertInt64DiskMetricValid(t, metric, false, test.expectedStartTime)
case "system.disk.weighted.io.time":
assertDoubleDiskMetricValid(t, metric, false, test.expectedStartTime)
case "system.disk.merged":
assertInt64DiskMetricValid(t, metric, true, test.expectedStartTime)
case "system.disk.merged.read":
assertInt64DiskMetricValid(t, metric, false, test.expectedStartTime)
case "system.disk.merged.write":
assertInt64DiskMetricValid(t, metric, false, test.expectedStartTime)
case "system.disk.pending_operations":
assertDiskPendingOperationsMetricValid(t, metric)
case "system.disk.weighted_io_time":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,12 @@ import (

"github.com/shirou/gopsutil/v3/host"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/featuregate"
"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/pmetric"
"go.opentelemetry.io/collector/receiver/scrapererror"
"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/processor/filterset"
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/hostmetricsreceiver/internal"
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/hostmetricsreceiver/internal/perfcounters"
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/hostmetricsreceiver/internal/scraper/diskscraper/internal/metadata"
)
Expand Down Expand Up @@ -65,16 +63,12 @@ type scraper struct {
skipScrape bool

// for mocking
bootTime func() (uint64, error)
emitMetricsWithDirectionAttribute bool
emitMetricsWithoutDirectionAttribute bool
bootTime func() (uint64, error)
}

// newDiskScraper creates a Disk Scraper
func newDiskScraper(_ context.Context, settings component.ReceiverCreateSettings, cfg *Config) (*scraper, error) {
scraper := &scraper{settings: settings, config: cfg, perfCounterScraper: &perfcounters.PerfLibScraper{}, bootTime: host.BootTime}
scraper.emitMetricsWithDirectionAttribute = featuregate.GetRegistry().IsEnabled(internal.EmitMetricsWithDirectionAttributeFeatureGateID)
scraper.emitMetricsWithoutDirectionAttribute = featuregate.GetRegistry().IsEnabled(internal.EmitMetricsWithoutDirectionAttributeFeatureGateID)

var err error

Expand Down Expand Up @@ -150,27 +144,15 @@ func (s *scraper) scrape(ctx context.Context) (pmetric.Metrics, error) {

func (s *scraper) recordDiskIOMetric(now pcommon.Timestamp, logicalDiskCounterValues []*perfcounters.CounterValues) {
for _, logicalDiskCounter := range logicalDiskCounterValues {
if s.emitMetricsWithDirectionAttribute {
s.mb.RecordSystemDiskIoDataPoint(now, logicalDiskCounter.Values[readBytesPerSec], logicalDiskCounter.InstanceName, metadata.AttributeDirectionRead)
s.mb.RecordSystemDiskIoDataPoint(now, logicalDiskCounter.Values[writeBytesPerSec], logicalDiskCounter.InstanceName, metadata.AttributeDirectionWrite)
}
if s.emitMetricsWithoutDirectionAttribute {
s.mb.RecordSystemDiskIoReadDataPoint(now, logicalDiskCounter.Values[readBytesPerSec], logicalDiskCounter.InstanceName)
s.mb.RecordSystemDiskIoWriteDataPoint(now, logicalDiskCounter.Values[writeBytesPerSec], logicalDiskCounter.InstanceName)
}
s.mb.RecordSystemDiskIoDataPoint(now, logicalDiskCounter.Values[readBytesPerSec], logicalDiskCounter.InstanceName, metadata.AttributeDirectionRead)
s.mb.RecordSystemDiskIoDataPoint(now, logicalDiskCounter.Values[writeBytesPerSec], logicalDiskCounter.InstanceName, metadata.AttributeDirectionWrite)
}
}

func (s *scraper) recordDiskOperationsMetric(now pcommon.Timestamp, logicalDiskCounterValues []*perfcounters.CounterValues) {
for _, logicalDiskCounter := range logicalDiskCounterValues {
if s.emitMetricsWithDirectionAttribute {
s.mb.RecordSystemDiskOperationsDataPoint(now, logicalDiskCounter.Values[readsPerSec], logicalDiskCounter.InstanceName, metadata.AttributeDirectionRead)
s.mb.RecordSystemDiskOperationsDataPoint(now, logicalDiskCounter.Values[writesPerSec], logicalDiskCounter.InstanceName, metadata.AttributeDirectionWrite)
}
if s.emitMetricsWithoutDirectionAttribute {
s.mb.RecordSystemDiskOperationsReadDataPoint(now, logicalDiskCounter.Values[readsPerSec], logicalDiskCounter.InstanceName)
s.mb.RecordSystemDiskOperationsWriteDataPoint(now, logicalDiskCounter.Values[writesPerSec], logicalDiskCounter.InstanceName)
}
s.mb.RecordSystemDiskOperationsDataPoint(now, logicalDiskCounter.Values[readsPerSec], logicalDiskCounter.InstanceName, metadata.AttributeDirectionRead)
s.mb.RecordSystemDiskOperationsDataPoint(now, logicalDiskCounter.Values[writesPerSec], logicalDiskCounter.InstanceName, metadata.AttributeDirectionWrite)
}
}

Expand All @@ -183,14 +165,8 @@ func (s *scraper) recordDiskIOTimeMetric(now pcommon.Timestamp, logicalDiskCount

func (s *scraper) recordDiskOperationTimeMetric(now pcommon.Timestamp, logicalDiskCounterValues []*perfcounters.CounterValues) {
for _, logicalDiskCounter := range logicalDiskCounterValues {
if s.emitMetricsWithDirectionAttribute {
s.mb.RecordSystemDiskOperationTimeDataPoint(now, float64(logicalDiskCounter.Values[avgDiskSecsPerRead])/1e7, logicalDiskCounter.InstanceName, metadata.AttributeDirectionRead)
s.mb.RecordSystemDiskOperationTimeDataPoint(now, float64(logicalDiskCounter.Values[avgDiskSecsPerWrite])/1e7, logicalDiskCounter.InstanceName, metadata.AttributeDirectionWrite)
}
if s.emitMetricsWithoutDirectionAttribute {
s.mb.RecordSystemDiskOperationTimeReadDataPoint(now, float64(logicalDiskCounter.Values[avgDiskSecsPerRead])/1e7, logicalDiskCounter.InstanceName)
s.mb.RecordSystemDiskOperationTimeWriteDataPoint(now, float64(logicalDiskCounter.Values[avgDiskSecsPerWrite])/1e7, logicalDiskCounter.InstanceName)
}
s.mb.RecordSystemDiskOperationTimeDataPoint(now, float64(logicalDiskCounter.Values[avgDiskSecsPerRead])/1e7, logicalDiskCounter.InstanceName, metadata.AttributeDirectionRead)
s.mb.RecordSystemDiskOperationTimeDataPoint(now, float64(logicalDiskCounter.Values[avgDiskSecsPerWrite])/1e7, logicalDiskCounter.InstanceName, metadata.AttributeDirectionWrite)
}
}

Expand Down
Loading

0 comments on commit e99d953

Please sign in to comment.