diff --git a/.chloggen/nginx-beta-featuregate.yaml b/.chloggen/nginx-beta-featuregate.yaml new file mode 100755 index 000000000000..f9a3e60058b2 --- /dev/null +++ b/.chloggen/nginx-beta-featuregate.yaml @@ -0,0 +1,20 @@ +# Use this changelog template to create an entry for release notes. +# If your change doesn't affect end users, such as a test fix or a tooling change, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. + +# 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: nginxreceiver + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Bump 'receiver.nginx.emitConnectionsCurrentAsSum' featuregate to beta (enabled by default) + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [4326] + +# (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: diff --git a/receiver/nginxreceiver/documentation.md b/receiver/nginxreceiver/documentation.md index 737d181fd87f..6cb26b8f49f3 100644 --- a/receiver/nginxreceiver/documentation.md +++ b/receiver/nginxreceiver/documentation.md @@ -24,9 +24,9 @@ The total number of accepted client connections The current number of nginx connections by state -| Unit | Metric Type | Value Type | -| ---- | ----------- | ---------- | -| connections | Gauge | Int | +| Unit | Metric Type | Value Type | Aggregation Temporality | Monotonic | +| ---- | ----------- | ---------- | ----------------------- | --------- | +| connections | Sum | Int | Cumulative | false | #### Attributes @@ -52,11 +52,11 @@ Total number of requests made to the server since it started ### temp.connections_current -Temporary placeholder for new version of nginx.connections_current. See featuregate 'nginx.connections_as_sum'. +Temporary placeholder for old version of nginx.connections_current. See featuregate 'nginx.connections_as_sum'. -| Unit | Metric Type | Value Type | Aggregation Temporality | Monotonic | -| ---- | ----------- | ---------- | ----------------------- | --------- | -| connections | Sum | Int | Cumulative | false | +| Unit | Metric Type | Value Type | +| ---- | ----------- | ---------- | +| connections | Gauge | Int | #### Attributes diff --git a/receiver/nginxreceiver/factory.go b/receiver/nginxreceiver/factory.go index a999b9fdc453..b9b83b39f798 100644 --- a/receiver/nginxreceiver/factory.go +++ b/receiver/nginxreceiver/factory.go @@ -23,7 +23,7 @@ const ( var connectorsAsSumGate = featuregate.GlobalRegistry().MustRegister( connectionsAsSum, - featuregate.StageAlpha, + featuregate.StageBeta, featuregate.WithRegisterDescription("When enabled, the receiver will emit the 'nginx.connections_current' metric as a nonmonotonic sum, rather than a gauge."), featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/4326"), featuregate.WithRegisterFromVersion("v0.78.0"), diff --git a/receiver/nginxreceiver/internal/metadata/generated_metrics.go b/receiver/nginxreceiver/internal/metadata/generated_metrics.go index 6053be37ffeb..f3d88852be96 100644 --- a/receiver/nginxreceiver/internal/metadata/generated_metrics.go +++ b/receiver/nginxreceiver/internal/metadata/generated_metrics.go @@ -107,15 +107,17 @@ func (m *metricNginxConnectionsCurrent) init() { m.data.SetName("nginx.connections_current") m.data.SetDescription("The current number of nginx connections by state") m.data.SetUnit("connections") - m.data.SetEmptyGauge() - m.data.Gauge().DataPoints().EnsureCapacity(m.capacity) + m.data.SetEmptySum() + m.data.Sum().SetIsMonotonic(false) + m.data.Sum().SetAggregationTemporality(pmetric.AggregationTemporalityCumulative) + m.data.Sum().DataPoints().EnsureCapacity(m.capacity) } func (m *metricNginxConnectionsCurrent) recordDataPoint(start pcommon.Timestamp, ts pcommon.Timestamp, val int64, stateAttributeValue string) { if !m.config.Enabled { return } - dp := m.data.Gauge().DataPoints().AppendEmpty() + dp := m.data.Sum().DataPoints().AppendEmpty() dp.SetStartTimestamp(start) dp.SetTimestamp(ts) dp.SetIntValue(val) @@ -124,14 +126,14 @@ func (m *metricNginxConnectionsCurrent) recordDataPoint(start pcommon.Timestamp, // updateCapacity saves max length of data point slices that will be used for the slice capacity. func (m *metricNginxConnectionsCurrent) updateCapacity() { - if m.data.Gauge().DataPoints().Len() > m.capacity { - m.capacity = m.data.Gauge().DataPoints().Len() + if m.data.Sum().DataPoints().Len() > m.capacity { + m.capacity = m.data.Sum().DataPoints().Len() } } // emit appends recorded metric data to a metrics slice and prepares it for recording another set of data points. func (m *metricNginxConnectionsCurrent) emit(metrics pmetric.MetricSlice) { - if m.config.Enabled && m.data.Gauge().DataPoints().Len() > 0 { + if m.config.Enabled && m.data.Sum().DataPoints().Len() > 0 { m.updateCapacity() m.data.MoveTo(metrics.AppendEmpty()) m.init() @@ -258,19 +260,17 @@ type metricTempConnectionsCurrent struct { // 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 new version of nginx.connections_current. See featuregate 'nginx.connections_as_sum'.") + m.data.SetDescription("Temporary placeholder for old version of nginx.connections_current. See featuregate 'nginx.connections_as_sum'.") m.data.SetUnit("connections") - m.data.SetEmptySum() - m.data.Sum().SetIsMonotonic(false) - m.data.Sum().SetAggregationTemporality(pmetric.AggregationTemporalityCumulative) - m.data.Sum().DataPoints().EnsureCapacity(m.capacity) + 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.Sum().DataPoints().AppendEmpty() + dp := m.data.Gauge().DataPoints().AppendEmpty() dp.SetStartTimestamp(start) dp.SetTimestamp(ts) dp.SetIntValue(val) @@ -279,14 +279,14 @@ func (m *metricTempConnectionsCurrent) recordDataPoint(start pcommon.Timestamp, // updateCapacity saves max length of data point slices that will be used for the slice capacity. func (m *metricTempConnectionsCurrent) updateCapacity() { - if m.data.Sum().DataPoints().Len() > m.capacity { - m.capacity = m.data.Sum().DataPoints().Len() + 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.Sum().DataPoints().Len() > 0 { + if m.config.Enabled && m.data.Gauge().DataPoints().Len() > 0 { m.updateCapacity() m.data.MoveTo(metrics.AppendEmpty()) m.init() diff --git a/receiver/nginxreceiver/internal/metadata/generated_metrics_test.go b/receiver/nginxreceiver/internal/metadata/generated_metrics_test.go index 5ecaf1639429..0b33b59cc177 100644 --- a/receiver/nginxreceiver/internal/metadata/generated_metrics_test.go +++ b/receiver/nginxreceiver/internal/metadata/generated_metrics_test.go @@ -116,11 +116,13 @@ func TestMetricsBuilder(t *testing.T) { case "nginx.connections_current": assert.False(t, validatedMetrics["nginx.connections_current"], "Found a duplicate in the metrics slice: nginx.connections_current") validatedMetrics["nginx.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, pmetric.MetricTypeSum, ms.At(i).Type()) + assert.Equal(t, 1, ms.At(i).Sum().DataPoints().Len()) assert.Equal(t, "The current number of nginx connections by state", ms.At(i).Description()) assert.Equal(t, "connections", ms.At(i).Unit()) - dp := ms.At(i).Gauge().DataPoints().At(0) + assert.Equal(t, false, ms.At(i).Sum().IsMonotonic()) + assert.Equal(t, pmetric.AggregationTemporalityCumulative, ms.At(i).Sum().AggregationTemporality()) + dp := ms.At(i).Sum().DataPoints().At(0) assert.Equal(t, start, dp.StartTimestamp()) assert.Equal(t, ts, dp.Timestamp()) assert.Equal(t, pmetric.NumberDataPointValueTypeInt, dp.ValueType()) @@ -159,13 +161,11 @@ func TestMetricsBuilder(t *testing.T) { 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.MetricTypeSum, ms.At(i).Type()) - assert.Equal(t, 1, ms.At(i).Sum().DataPoints().Len()) - assert.Equal(t, "Temporary placeholder for new version of nginx.connections_current. See featuregate 'nginx.connections_as_sum'.", ms.At(i).Description()) + 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()) - assert.Equal(t, false, ms.At(i).Sum().IsMonotonic()) - assert.Equal(t, pmetric.AggregationTemporalityCumulative, ms.At(i).Sum().AggregationTemporality()) - dp := ms.At(i).Sum().DataPoints().At(0) + 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()) diff --git a/receiver/nginxreceiver/internal/metadata/temporary_metrics.go b/receiver/nginxreceiver/internal/metadata/temporary_metrics.go index 0ea684cd3501..ea84845a3569 100644 --- a/receiver/nginxreceiver/internal/metadata/temporary_metrics.go +++ b/receiver/nginxreceiver/internal/metadata/temporary_metrics.go @@ -2,8 +2,8 @@ package metadata -// WithCurrentConnectionsAsSum sets the current connections metric as a sum. -func WithCurrentConnectionsAsSum() metricBuilderOption { +// 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 @@ -14,9 +14,9 @@ func WithCurrentConnectionsAsSum() metricBuilderOption { } } -// WithCurrentConnectionsAsSumDisabled disables the current connections metric as a sum. +// 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 WithCurrentConnectionsAsSumDisabled() metricBuilderOption { +func WithCurrentConnectionsAsGaugeDisabled() metricBuilderOption { return func(mb *MetricsBuilder) { mb.metricTempConnectionsCurrent.config.Enabled = false } diff --git a/receiver/nginxreceiver/metadata.yaml b/receiver/nginxreceiver/metadata.yaml index ff9b00126154..b74cb1a21ba4 100644 --- a/receiver/nginxreceiver/metadata.yaml +++ b/receiver/nginxreceiver/metadata.yaml @@ -44,23 +44,21 @@ metrics: monotonic: true aggregation: cumulative attributes: [] - -# Old version of metric, to be enabled when featuregate is stable nginx.connections_current: enabled: true description: The current number of nginx connections by state unit: connections - gauge: + sum: value_type: int + monotonic: false + aggregation: cumulative attributes: [state] -# New version of metric, to be enabled when featuregate is stable +# 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 new version of nginx.connections_current. See featuregate 'nginx.connections_as_sum'. + description: Temporary placeholder for old version of nginx.connections_current. See featuregate 'nginx.connections_as_sum'. unit: connections - sum: + gauge: value_type: int - monotonic: false - aggregation: cumulative attributes: [state] diff --git a/receiver/nginxreceiver/scraper.go b/receiver/nginxreceiver/scraper.go index 6074ce5098ac..c5b8b575f8de 100644 --- a/receiver/nginxreceiver/scraper.go +++ b/receiver/nginxreceiver/scraper.go @@ -33,9 +33,9 @@ func newNginxScraper( ) *nginxScraper { var mb *metadata.MetricsBuilder if connectorsAsSumGate.IsEnabled() { - mb = metadata.NewMetricsBuilder(cfg.MetricsBuilderConfig, settings, metadata.WithCurrentConnectionsAsSum()) + mb = metadata.NewMetricsBuilder(cfg.MetricsBuilderConfig, settings, metadata.WithCurrentConnectionsAsGaugeDisabled()) } else { - mb = metadata.NewMetricsBuilder(cfg.MetricsBuilderConfig, settings, metadata.WithCurrentConnectionsAsSumDisabled()) + mb = metadata.NewMetricsBuilder(cfg.MetricsBuilderConfig, settings, metadata.WithCurrentConnectionsAsGauge()) } return &nginxScraper{ settings: settings.TelemetrySettings, @@ -78,15 +78,15 @@ func (r *nginxScraper) scrape(context.Context) (pmetric.Metrics, error) { r.mb.RecordNginxConnectionsHandledDataPoint(now, stats.Connections.Handled) if connectorsAsSumGate.IsEnabled() { - r.mb.RecordTempConnectionsCurrentDataPoint(now, stats.Connections.Active, metadata.AttributeStateActive) - r.mb.RecordTempConnectionsCurrentDataPoint(now, stats.Connections.Reading, metadata.AttributeStateReading) - r.mb.RecordTempConnectionsCurrentDataPoint(now, stats.Connections.Writing, metadata.AttributeStateWriting) - r.mb.RecordTempConnectionsCurrentDataPoint(now, stats.Connections.Waiting, metadata.AttributeStateWaiting) - } else { r.mb.RecordNginxConnectionsCurrentDataPoint(now, stats.Connections.Active, metadata.AttributeStateActive) r.mb.RecordNginxConnectionsCurrentDataPoint(now, stats.Connections.Reading, metadata.AttributeStateReading) r.mb.RecordNginxConnectionsCurrentDataPoint(now, stats.Connections.Writing, metadata.AttributeStateWriting) r.mb.RecordNginxConnectionsCurrentDataPoint(now, stats.Connections.Waiting, metadata.AttributeStateWaiting) + } else { + r.mb.RecordTempConnectionsCurrentDataPoint(now, stats.Connections.Active, metadata.AttributeStateActive) + r.mb.RecordTempConnectionsCurrentDataPoint(now, stats.Connections.Reading, metadata.AttributeStateReading) + r.mb.RecordTempConnectionsCurrentDataPoint(now, stats.Connections.Writing, metadata.AttributeStateWriting) + r.mb.RecordTempConnectionsCurrentDataPoint(now, stats.Connections.Waiting, metadata.AttributeStateWaiting) } return r.mb.Emit(), nil diff --git a/receiver/nginxreceiver/scraper_test.go b/receiver/nginxreceiver/scraper_test.go index 2a7ebce149ab..ba5ad5509560 100644 --- a/receiver/nginxreceiver/scraper_test.go +++ b/receiver/nginxreceiver/scraper_test.go @@ -41,21 +41,22 @@ func TestScraper(t *testing.T) { expectedMetrics, err := golden.ReadMetrics(expectedFile) require.NoError(t, err) - require.NoError(t, pmetrictest.CompareMetrics(expectedMetrics, actualMetrics, pmetrictest.IgnoreStartTimestamp(), + require.NoError(t, pmetrictest.CompareMetrics(expectedMetrics, actualMetrics, + pmetrictest.IgnoreStartTimestamp(), pmetrictest.IgnoreMetricDataPointsOrder(), - pmetrictest.IgnoreMetricDataPointsOrder(), - pmetrictest.IgnoreTimestamp())) + pmetrictest.IgnoreTimestamp(), + pmetrictest.IgnoreMetricsOrder())) } -func TestScraperWithConnectionsAsSum(t *testing.T) { +func TestScraperWithConnectionsAsGauge(t *testing.T) { nginxMock := newMockServer(t) cfg := createDefaultConfig().(*Config) cfg.Endpoint = nginxMock.URL + "/status" require.NoError(t, component.ValidateConfig(cfg)) - require.NoError(t, featuregate.GlobalRegistry().Set(connectionsAsSum, true)) + require.NoError(t, featuregate.GlobalRegistry().Set(connectionsAsSum, false)) defer func() { - require.NoError(t, featuregate.GlobalRegistry().Set(connectionsAsSum, false)) + require.NoError(t, featuregate.GlobalRegistry().Set(connectionsAsSum, true)) }() scraper := newNginxScraper(receivertest.NewNopCreateSettings(), cfg) @@ -66,14 +67,15 @@ func TestScraperWithConnectionsAsSum(t *testing.T) { actualMetrics, err := scraper.scrape(context.Background()) require.NoError(t, err) - expectedFile := filepath.Join("testdata", "scraper", "expected_with_connections_as_sum.yaml") + expectedFile := filepath.Join("testdata", "scraper", "expected_with_connections_as_gauge.yaml") expectedMetrics, err := golden.ReadMetrics(expectedFile) require.NoError(t, err) - require.NoError(t, pmetrictest.CompareMetrics(expectedMetrics, actualMetrics, pmetrictest.IgnoreStartTimestamp(), - pmetrictest.IgnoreMetricDataPointsOrder(), + require.NoError(t, pmetrictest.CompareMetrics(expectedMetrics, actualMetrics, + pmetrictest.IgnoreStartTimestamp(), pmetrictest.IgnoreMetricDataPointsOrder(), - pmetrictest.IgnoreTimestamp(), pmetrictest.IgnoreMetricsOrder())) + pmetrictest.IgnoreTimestamp(), + pmetrictest.IgnoreMetricsOrder())) } func TestScraperError(t *testing.T) { diff --git a/receiver/nginxreceiver/testdata/integration/expected.yaml b/receiver/nginxreceiver/testdata/integration/expected.yaml index ecf0ec5405c9..9c5bc3463b5a 100644 --- a/receiver/nginxreceiver/testdata/integration/expected.yaml +++ b/receiver/nginxreceiver/testdata/integration/expected.yaml @@ -12,7 +12,8 @@ resourceMetrics: isMonotonic: true unit: connections - description: The current number of nginx connections by state - gauge: + sum: + aggregationTemporality: 2 dataPoints: - asInt: "1" attributes: @@ -38,6 +39,7 @@ resourceMetrics: value: stringValue: waiting timeUnixNano: "1643729289485220000" + isMonotonic: false name: nginx.connections_current unit: connections - description: The total number of handled connections. Generally, the parameter value is the same as nginx.connections_accepted unless some resource limits have been reached (for example, the worker_connections limit). diff --git a/receiver/nginxreceiver/testdata/scraper/expected.yaml b/receiver/nginxreceiver/testdata/scraper/expected.yaml index 0d523ba95e6c..b56d0becda04 100644 --- a/receiver/nginxreceiver/testdata/scraper/expected.yaml +++ b/receiver/nginxreceiver/testdata/scraper/expected.yaml @@ -12,7 +12,9 @@ resourceMetrics: isMonotonic: true unit: connections - description: The current number of nginx connections by state - gauge: + name: nginx.connections_current + sum: + aggregationTemporality: 2 dataPoints: - asInt: "291" attributes: @@ -38,7 +40,6 @@ resourceMetrics: value: stringValue: writing timeUnixNano: "1638471548185885000" - name: nginx.connections_current unit: connections - description: The total number of handled connections. Generally, the parameter value is the same as nginx.connections_accepted unless some resource limits have been reached (for example, the worker_connections limit). name: nginx.connections_handled diff --git a/receiver/nginxreceiver/testdata/scraper/expected_with_connections_as_sum.yaml b/receiver/nginxreceiver/testdata/scraper/expected_with_connections_as_gauge.yaml similarity index 97% rename from receiver/nginxreceiver/testdata/scraper/expected_with_connections_as_sum.yaml rename to receiver/nginxreceiver/testdata/scraper/expected_with_connections_as_gauge.yaml index b56d0becda04..0d523ba95e6c 100644 --- a/receiver/nginxreceiver/testdata/scraper/expected_with_connections_as_sum.yaml +++ b/receiver/nginxreceiver/testdata/scraper/expected_with_connections_as_gauge.yaml @@ -12,9 +12,7 @@ resourceMetrics: isMonotonic: true unit: connections - description: The current number of nginx connections by state - name: nginx.connections_current - sum: - aggregationTemporality: 2 + gauge: dataPoints: - asInt: "291" attributes: @@ -40,6 +38,7 @@ resourceMetrics: value: stringValue: writing timeUnixNano: "1638471548185885000" + name: nginx.connections_current unit: connections - description: The total number of handled connections. Generally, the parameter value is the same as nginx.connections_accepted unless some resource limits have been reached (for example, the worker_connections limit). name: nginx.connections_handled