From 5de9937bff5d5904c65d6ffb728e5202d163bc60 Mon Sep 17 00:00:00 2001 From: Daniel Jaglowski Date: Fri, 27 Oct 2023 20:33:15 -0600 Subject: [PATCH] [chore][receiver/nginx] Remove stable feature gate (#28659) Resolves https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/4326 --- receiver/nginxreceiver/documentation.md | 14 ----- receiver/nginxreceiver/factory.go | 12 ---- receiver/nginxreceiver/go.mod | 2 +- .../internal/metadata/generated_config.go | 4 -- .../metadata/generated_config_test.go | 2 - .../internal/metadata/generated_metrics.go | 59 ------------------- .../metadata/generated_metrics_test.go | 19 ------ .../internal/metadata/temporary_metrics.go | 23 -------- .../internal/metadata/testdata/config.yaml | 4 -- receiver/nginxreceiver/metadata.yaml | 9 --- receiver/nginxreceiver/scraper.go | 2 +- 11 files changed, 2 insertions(+), 148 deletions(-) delete mode 100644 receiver/nginxreceiver/internal/metadata/temporary_metrics.go diff --git a/receiver/nginxreceiver/documentation.md b/receiver/nginxreceiver/documentation.md index 6cb26b8f49f3..3dd32a196cde 100644 --- a/receiver/nginxreceiver/documentation.md +++ b/receiver/nginxreceiver/documentation.md @@ -49,17 +49,3 @@ Total number of requests made to the server since it started | Unit | Metric Type | Value Type | Aggregation Temporality | Monotonic | | ---- | ----------- | ---------- | ----------------------- | --------- | | requests | Sum | Int | Cumulative | true | - -### temp.connections_current - -Temporary placeholder for old version of nginx.connections_current. See featuregate 'nginx.connections_as_sum'. - -| Unit | Metric Type | Value Type | -| ---- | ----------- | ---------- | -| connections | Gauge | Int | - -#### Attributes - -| Name | Description | Values | -| ---- | ----------- | ------ | -| state | The state of a connection | Str: ``active``, ``reading``, ``writing``, ``waiting`` | diff --git a/receiver/nginxreceiver/factory.go b/receiver/nginxreceiver/factory.go index 00e5cc83855c..98429beb21a3 100644 --- a/receiver/nginxreceiver/factory.go +++ b/receiver/nginxreceiver/factory.go @@ -10,24 +10,12 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/confighttp" "go.opentelemetry.io/collector/consumer" - "go.opentelemetry.io/collector/featuregate" "go.opentelemetry.io/collector/receiver" "go.opentelemetry.io/collector/receiver/scraperhelper" "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/nginxreceiver/internal/metadata" ) -const ( - connectionsAsSum = "receiver.nginx.emitConnectionsCurrentAsSum" -) - -var _ = featuregate.GlobalRegistry().MustRegister( - connectionsAsSum, - featuregate.StageStable, - featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/4326"), - featuregate.WithRegisterToVersion("v0.87.0"), -) - // NewFactory creates a factory for nginx receiver. func NewFactory() receiver.Factory { return receiver.NewFactory( diff --git a/receiver/nginxreceiver/go.mod b/receiver/nginxreceiver/go.mod index 36d9d2e2a0c6..b14caa8e6c08 100644 --- a/receiver/nginxreceiver/go.mod +++ b/receiver/nginxreceiver/go.mod @@ -15,7 +15,6 @@ require ( go.opentelemetry.io/collector/config/configtls v0.88.1-0.20231026220224-6405e152a2d9 go.opentelemetry.io/collector/confmap v0.88.1-0.20231026220224-6405e152a2d9 go.opentelemetry.io/collector/consumer v0.88.1-0.20231026220224-6405e152a2d9 - go.opentelemetry.io/collector/featuregate v1.0.0-rcv0017.0.20231026220224-6405e152a2d9 go.opentelemetry.io/collector/pdata v1.0.0-rcv0017.0.20231026220224-6405e152a2d9 go.opentelemetry.io/collector/receiver v0.88.1-0.20231026220224-6405e152a2d9 go.uber.org/zap v1.26.0 @@ -83,6 +82,7 @@ require ( go.opentelemetry.io/collector/config/internal v0.88.1-0.20231026220224-6405e152a2d9 // indirect go.opentelemetry.io/collector/extension v0.88.1-0.20231026220224-6405e152a2d9 // indirect go.opentelemetry.io/collector/extension/auth v0.88.1-0.20231026220224-6405e152a2d9 // indirect + go.opentelemetry.io/collector/featuregate v1.0.0-rcv0017.0.20231026220224-6405e152a2d9 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.45.0 // indirect go.opentelemetry.io/otel v1.19.0 // indirect go.opentelemetry.io/otel/metric v1.19.0 // indirect diff --git a/receiver/nginxreceiver/internal/metadata/generated_config.go b/receiver/nginxreceiver/internal/metadata/generated_config.go index e98c8ed60083..15bee241b292 100644 --- a/receiver/nginxreceiver/internal/metadata/generated_config.go +++ b/receiver/nginxreceiver/internal/metadata/generated_config.go @@ -29,7 +29,6 @@ type MetricsConfig struct { NginxConnectionsCurrent MetricConfig `mapstructure:"nginx.connections_current"` NginxConnectionsHandled MetricConfig `mapstructure:"nginx.connections_handled"` NginxRequests MetricConfig `mapstructure:"nginx.requests"` - TempConnectionsCurrent MetricConfig `mapstructure:"temp.connections_current"` } func DefaultMetricsConfig() MetricsConfig { @@ -46,9 +45,6 @@ func DefaultMetricsConfig() MetricsConfig { NginxRequests: MetricConfig{ Enabled: true, }, - TempConnectionsCurrent: MetricConfig{ - Enabled: true, - }, } } diff --git a/receiver/nginxreceiver/internal/metadata/generated_config_test.go b/receiver/nginxreceiver/internal/metadata/generated_config_test.go index e6d63d80c191..e406f9b2191c 100644 --- a/receiver/nginxreceiver/internal/metadata/generated_config_test.go +++ b/receiver/nginxreceiver/internal/metadata/generated_config_test.go @@ -30,7 +30,6 @@ func TestMetricsBuilderConfig(t *testing.T) { NginxConnectionsCurrent: MetricConfig{Enabled: true}, NginxConnectionsHandled: MetricConfig{Enabled: true}, NginxRequests: MetricConfig{Enabled: true}, - TempConnectionsCurrent: MetricConfig{Enabled: true}, }, }, }, @@ -42,7 +41,6 @@ func TestMetricsBuilderConfig(t *testing.T) { NginxConnectionsCurrent: MetricConfig{Enabled: false}, NginxConnectionsHandled: MetricConfig{Enabled: false}, NginxRequests: MetricConfig{Enabled: false}, - TempConnectionsCurrent: MetricConfig{Enabled: false}, }, }, }, diff --git a/receiver/nginxreceiver/internal/metadata/generated_metrics.go b/receiver/nginxreceiver/internal/metadata/generated_metrics.go index a33a0e8e3427..874747263aec 100644 --- a/receiver/nginxreceiver/internal/metadata/generated_metrics.go +++ b/receiver/nginxreceiver/internal/metadata/generated_metrics.go @@ -251,57 +251,6 @@ func newMetricNginxRequests(cfg MetricConfig) metricNginxRequests { return m } -type metricTempConnectionsCurrent struct { - data pmetric.Metric // data buffer for generated metric. - config MetricConfig // metric config provided by user. - capacity int // max observed number of data points added to the metric. -} - -// init fills temp.connections_current metric with initial data. -func (m *metricTempConnectionsCurrent) init() { - m.data.SetName("temp.connections_current") - m.data.SetDescription("Temporary placeholder for old version of nginx.connections_current. See featuregate 'nginx.connections_as_sum'.") - m.data.SetUnit("connections") - m.data.SetEmptyGauge() - m.data.Gauge().DataPoints().EnsureCapacity(m.capacity) -} - -func (m *metricTempConnectionsCurrent) recordDataPoint(start pcommon.Timestamp, ts pcommon.Timestamp, val int64, stateAttributeValue string) { - if !m.config.Enabled { - return - } - dp := m.data.Gauge().DataPoints().AppendEmpty() - dp.SetStartTimestamp(start) - dp.SetTimestamp(ts) - dp.SetIntValue(val) - dp.Attributes().PutStr("state", stateAttributeValue) -} - -// updateCapacity saves max length of data point slices that will be used for the slice capacity. -func (m *metricTempConnectionsCurrent) updateCapacity() { - if m.data.Gauge().DataPoints().Len() > m.capacity { - m.capacity = m.data.Gauge().DataPoints().Len() - } -} - -// emit appends recorded metric data to a metrics slice and prepares it for recording another set of data points. -func (m *metricTempConnectionsCurrent) emit(metrics pmetric.MetricSlice) { - if m.config.Enabled && m.data.Gauge().DataPoints().Len() > 0 { - m.updateCapacity() - m.data.MoveTo(metrics.AppendEmpty()) - m.init() - } -} - -func newMetricTempConnectionsCurrent(cfg MetricConfig) metricTempConnectionsCurrent { - m := metricTempConnectionsCurrent{config: cfg} - if cfg.Enabled { - m.data = pmetric.NewMetric() - m.init() - } - return m -} - // MetricsBuilder provides an interface for scrapers to report metrics while taking care of all the transformations // required to produce metric representation defined in metadata and user config. type MetricsBuilder struct { @@ -314,7 +263,6 @@ type MetricsBuilder struct { metricNginxConnectionsCurrent metricNginxConnectionsCurrent metricNginxConnectionsHandled metricNginxConnectionsHandled metricNginxRequests metricNginxRequests - metricTempConnectionsCurrent metricTempConnectionsCurrent } // metricBuilderOption applies changes to default metrics builder. @@ -337,7 +285,6 @@ func NewMetricsBuilder(mbc MetricsBuilderConfig, settings receiver.CreateSetting metricNginxConnectionsCurrent: newMetricNginxConnectionsCurrent(mbc.Metrics.NginxConnectionsCurrent), metricNginxConnectionsHandled: newMetricNginxConnectionsHandled(mbc.Metrics.NginxConnectionsHandled), metricNginxRequests: newMetricNginxRequests(mbc.Metrics.NginxRequests), - metricTempConnectionsCurrent: newMetricTempConnectionsCurrent(mbc.Metrics.TempConnectionsCurrent), } for _, op := range options { op(mb) @@ -398,7 +345,6 @@ func (mb *MetricsBuilder) EmitForResource(rmo ...ResourceMetricsOption) { mb.metricNginxConnectionsCurrent.emit(ils.Metrics()) mb.metricNginxConnectionsHandled.emit(ils.Metrics()) mb.metricNginxRequests.emit(ils.Metrics()) - mb.metricTempConnectionsCurrent.emit(ils.Metrics()) for _, op := range rmo { op(rm) @@ -439,11 +385,6 @@ func (mb *MetricsBuilder) RecordNginxRequestsDataPoint(ts pcommon.Timestamp, val mb.metricNginxRequests.recordDataPoint(mb.startTime, ts, val) } -// RecordTempConnectionsCurrentDataPoint adds a data point to temp.connections_current metric. -func (mb *MetricsBuilder) RecordTempConnectionsCurrentDataPoint(ts pcommon.Timestamp, val int64, stateAttributeValue AttributeState) { - mb.metricTempConnectionsCurrent.recordDataPoint(mb.startTime, ts, val, stateAttributeValue.String()) -} - // Reset resets metrics builder to its initial state. It should be used when external metrics source is restarted, // and metrics builder should update its startTime and reset it's internal state accordingly. func (mb *MetricsBuilder) Reset(options ...metricBuilderOption) { diff --git a/receiver/nginxreceiver/internal/metadata/generated_metrics_test.go b/receiver/nginxreceiver/internal/metadata/generated_metrics_test.go index d92058c3f72f..5a8f90a84fcd 100644 --- a/receiver/nginxreceiver/internal/metadata/generated_metrics_test.go +++ b/receiver/nginxreceiver/internal/metadata/generated_metrics_test.go @@ -71,10 +71,6 @@ func TestMetricsBuilder(t *testing.T) { allMetricsCount++ mb.RecordNginxRequestsDataPoint(ts, 1) - defaultMetricsCount++ - allMetricsCount++ - mb.RecordTempConnectionsCurrentDataPoint(ts, 1, AttributeStateActive) - res := pcommon.NewResource() metrics := mb.Emit(WithResource(res)) @@ -156,21 +152,6 @@ func TestMetricsBuilder(t *testing.T) { assert.Equal(t, ts, dp.Timestamp()) assert.Equal(t, pmetric.NumberDataPointValueTypeInt, dp.ValueType()) assert.Equal(t, int64(1), dp.IntValue()) - case "temp.connections_current": - assert.False(t, validatedMetrics["temp.connections_current"], "Found a duplicate in the metrics slice: temp.connections_current") - validatedMetrics["temp.connections_current"] = true - assert.Equal(t, pmetric.MetricTypeGauge, ms.At(i).Type()) - assert.Equal(t, 1, ms.At(i).Gauge().DataPoints().Len()) - assert.Equal(t, "Temporary placeholder for old version of nginx.connections_current. See featuregate 'nginx.connections_as_sum'.", ms.At(i).Description()) - assert.Equal(t, "connections", ms.At(i).Unit()) - dp := ms.At(i).Gauge().DataPoints().At(0) - assert.Equal(t, start, dp.StartTimestamp()) - assert.Equal(t, ts, dp.Timestamp()) - assert.Equal(t, pmetric.NumberDataPointValueTypeInt, dp.ValueType()) - assert.Equal(t, int64(1), dp.IntValue()) - attrVal, ok := dp.Attributes().Get("state") - assert.True(t, ok) - assert.EqualValues(t, "active", attrVal.Str()) } } }) diff --git a/receiver/nginxreceiver/internal/metadata/temporary_metrics.go b/receiver/nginxreceiver/internal/metadata/temporary_metrics.go deleted file mode 100644 index ea84845a3569..000000000000 --- a/receiver/nginxreceiver/internal/metadata/temporary_metrics.go +++ /dev/null @@ -1,23 +0,0 @@ -// Code generated by mdatagen. DO NOT EDIT. - -package metadata - -// WithCurrentConnectionsAsGauge sets the current connections metric as a gauge. -func WithCurrentConnectionsAsGauge() metricBuilderOption { - return func(mb *MetricsBuilder) { - if mb.metricNginxConnectionsCurrent.config.Enabled { - mb.metricNginxConnectionsCurrent.config.Enabled = false - mb.metricTempConnectionsCurrent.config.Enabled = true - mb.metricTempConnectionsCurrent.data.SetName(mb.metricNginxConnectionsCurrent.data.Name()) - mb.metricTempConnectionsCurrent.data.SetDescription(mb.metricNginxConnectionsCurrent.data.Description()) - } - } -} - -// WithCurrentConnectionsAsGaugeDisabled disables the current connections metric as a gauge. -// This is necessary because the metric must be enabled by default in order to be able to apply the other option. -func WithCurrentConnectionsAsGaugeDisabled() metricBuilderOption { - return func(mb *MetricsBuilder) { - mb.metricTempConnectionsCurrent.config.Enabled = false - } -} diff --git a/receiver/nginxreceiver/internal/metadata/testdata/config.yaml b/receiver/nginxreceiver/internal/metadata/testdata/config.yaml index 7612db967b5d..05f6368506a2 100644 --- a/receiver/nginxreceiver/internal/metadata/testdata/config.yaml +++ b/receiver/nginxreceiver/internal/metadata/testdata/config.yaml @@ -9,8 +9,6 @@ all_set: enabled: true nginx.requests: enabled: true - temp.connections_current: - enabled: true none_set: metrics: nginx.connections_accepted: @@ -21,5 +19,3 @@ none_set: enabled: false nginx.requests: enabled: false - temp.connections_current: - enabled: false diff --git a/receiver/nginxreceiver/metadata.yaml b/receiver/nginxreceiver/metadata.yaml index 53e5861ba6a0..c5cd9aa4987f 100644 --- a/receiver/nginxreceiver/metadata.yaml +++ b/receiver/nginxreceiver/metadata.yaml @@ -55,12 +55,3 @@ metrics: monotonic: false aggregation_temporality: cumulative attributes: [state] - -# Old version of metric, to be removed when featuregate is stable - temp.connections_current: - enabled: true # must be enabled by default in order to apply necessary MetricBuilder option - description: Temporary placeholder for old version of nginx.connections_current. See featuregate 'nginx.connections_as_sum'. - unit: connections - gauge: - value_type: int - attributes: [state] diff --git a/receiver/nginxreceiver/scraper.go b/receiver/nginxreceiver/scraper.go index 2335bff64d68..695b6518ed56 100644 --- a/receiver/nginxreceiver/scraper.go +++ b/receiver/nginxreceiver/scraper.go @@ -31,7 +31,7 @@ func newNginxScraper( settings receiver.CreateSettings, cfg *Config, ) *nginxScraper { - mb := metadata.NewMetricsBuilder(cfg.MetricsBuilderConfig, settings, metadata.WithCurrentConnectionsAsGaugeDisabled()) + mb := metadata.NewMetricsBuilder(cfg.MetricsBuilderConfig, settings) return &nginxScraper{ settings: settings.TelemetrySettings, cfg: cfg,