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

[receiver/hostmetrics] remove direction feature gate #14959

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
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