diff --git a/api/envoy/config/metrics/v3/metrics_service.proto b/api/envoy/config/metrics/v3/metrics_service.proto index 4bb6c77e66c23..e5fab870f8ee6 100644 --- a/api/envoy/config/metrics/v3/metrics_service.proto +++ b/api/envoy/config/metrics/v3/metrics_service.proto @@ -38,4 +38,9 @@ message MetricsServiceConfig { // Eventually (https://github.com/envoyproxy/envoy/issues/10968) if this value is not set, the // sink will take updates from the :ref:`MetricsResponse `. google.protobuf.BoolValue report_counters_as_deltas = 2; + + // If true, metrics will have their tags emitted as labels on the metrics objects sent to the MetricsService, + // and the tag extracted name will be used instead of the full name, which may contain values used by the tag + // extractor or additional tags added during stats creation. + bool emit_tags_as_labels = 4; } diff --git a/api/envoy/config/metrics/v4alpha/metrics_service.proto b/api/envoy/config/metrics/v4alpha/metrics_service.proto index edc0fcfc4d6e5..570bc2e9d7162 100644 --- a/api/envoy/config/metrics/v4alpha/metrics_service.proto +++ b/api/envoy/config/metrics/v4alpha/metrics_service.proto @@ -38,4 +38,9 @@ message MetricsServiceConfig { // Eventually (https://github.com/envoyproxy/envoy/issues/10968) if this value is not set, the // sink will take updates from the :ref:`MetricsResponse `. google.protobuf.BoolValue report_counters_as_deltas = 2; + + // If true, metrics will have their tags emitted as labels on the metrics objects sent to the MetricsService, + // and the tag extracted name will be used instead of the full name, which may contain values used by the tag + // extractor or additional tags added during stats creation. + bool emit_tags_as_labels = 4; } diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 9a08d9a7e57a1..444319d3da2be 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -33,5 +33,7 @@ Removed Config or Runtime New Features ------------ +* metric service: added support for sending metric tags as labels. This can be enabled by setting the :ref:`emit_tags_as_labels ` field to true. + Deprecated ---------- diff --git a/generated_api_shadow/envoy/config/metrics/v3/metrics_service.proto b/generated_api_shadow/envoy/config/metrics/v3/metrics_service.proto index 4bb6c77e66c23..e5fab870f8ee6 100644 --- a/generated_api_shadow/envoy/config/metrics/v3/metrics_service.proto +++ b/generated_api_shadow/envoy/config/metrics/v3/metrics_service.proto @@ -38,4 +38,9 @@ message MetricsServiceConfig { // Eventually (https://github.com/envoyproxy/envoy/issues/10968) if this value is not set, the // sink will take updates from the :ref:`MetricsResponse `. google.protobuf.BoolValue report_counters_as_deltas = 2; + + // If true, metrics will have their tags emitted as labels on the metrics objects sent to the MetricsService, + // and the tag extracted name will be used instead of the full name, which may contain values used by the tag + // extractor or additional tags added during stats creation. + bool emit_tags_as_labels = 4; } diff --git a/generated_api_shadow/envoy/config/metrics/v4alpha/metrics_service.proto b/generated_api_shadow/envoy/config/metrics/v4alpha/metrics_service.proto index edc0fcfc4d6e5..570bc2e9d7162 100644 --- a/generated_api_shadow/envoy/config/metrics/v4alpha/metrics_service.proto +++ b/generated_api_shadow/envoy/config/metrics/v4alpha/metrics_service.proto @@ -38,4 +38,9 @@ message MetricsServiceConfig { // Eventually (https://github.com/envoyproxy/envoy/issues/10968) if this value is not set, the // sink will take updates from the :ref:`MetricsResponse `. google.protobuf.BoolValue report_counters_as_deltas = 2; + + // If true, metrics will have their tags emitted as labels on the metrics objects sent to the MetricsService, + // and the tag extracted name will be used instead of the full name, which may contain values used by the tag + // extractor or additional tags added during stats creation. + bool emit_tags_as_labels = 4; } diff --git a/source/extensions/stat_sinks/metrics_service/config.cc b/source/extensions/stat_sinks/metrics_service/config.cc index 832daa3621016..a17539df4e6fc 100644 --- a/source/extensions/stat_sinks/metrics_service/config.cc +++ b/source/extensions/stat_sinks/metrics_service/config.cc @@ -39,7 +39,7 @@ MetricsServiceSinkFactory::createStatsSink(const Protobuf::Message& config, return std::make_unique>( - grpc_metrics_streamer, + grpc_metrics_streamer, sink_config.emit_tags_as_labels(), PROTOBUF_GET_WRAPPED_OR_DEFAULT(sink_config, report_counters_as_deltas, false)); } diff --git a/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.cc b/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.cc index 11734fe336fe2..14d0d2043bb48 100644 --- a/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.cc +++ b/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.cc @@ -82,10 +82,8 @@ MetricsPtr MetricsFlusher::flush(Stats::MetricSnapshot& snapshot) const { void MetricsFlusher::flushCounter(io::prometheus::client::MetricFamily& metrics_family, const Stats::MetricSnapshot::CounterSnapshot& counter_snapshot, int64_t snapshot_time_ms) const { - metrics_family.set_type(io::prometheus::client::MetricType::COUNTER); - metrics_family.set_name(counter_snapshot.counter_.get().name()); - auto* metric = metrics_family.add_metric(); - metric->set_timestamp_ms(snapshot_time_ms); + auto* metric = populateMetricsFamily(metrics_family, io::prometheus::client::MetricType::COUNTER, + snapshot_time_ms, counter_snapshot.counter_.get()); auto* counter_metric = metric->mutable_counter(); if (report_counters_as_deltas_) { counter_metric->set_value(counter_snapshot.delta_); @@ -96,10 +94,8 @@ void MetricsFlusher::flushCounter(io::prometheus::client::MetricFamily& metrics_ void MetricsFlusher::flushGauge(io::prometheus::client::MetricFamily& metrics_family, const Stats::Gauge& gauge, int64_t snapshot_time_ms) const { - metrics_family.set_type(io::prometheus::client::MetricType::GAUGE); - metrics_family.set_name(gauge.name()); - auto* metric = metrics_family.add_metric(); - metric->set_timestamp_ms(snapshot_time_ms); + auto* metric = populateMetricsFamily(metrics_family, io::prometheus::client::MetricType::GAUGE, + snapshot_time_ms, gauge); auto* gauge_metric = metric->mutable_gauge(); gauge_metric->set_value(gauge.value()); } @@ -113,10 +109,9 @@ void MetricsFlusher::flushHistogram(io::prometheus::client::MetricFamily& summar // performance. // Add summary information for histograms. - summary_metrics_family.set_type(io::prometheus::client::MetricType::SUMMARY); - summary_metrics_family.set_name(envoy_histogram.name()); - auto* summary_metric = summary_metrics_family.add_metric(); - summary_metric->set_timestamp_ms(snapshot_time_ms); + auto* summary_metric = + populateMetricsFamily(summary_metrics_family, io::prometheus::client::MetricType::SUMMARY, + snapshot_time_ms, envoy_histogram); auto* summary = summary_metric->mutable_summary(); const Stats::HistogramStatistics& hist_stats = envoy_histogram.intervalStatistics(); for (size_t i = 0; i < hist_stats.supportedQuantiles().size(); i++) { @@ -126,10 +121,9 @@ void MetricsFlusher::flushHistogram(io::prometheus::client::MetricFamily& summar } // Add bucket information for histograms. - histogram_metrics_family.set_type(io::prometheus::client::MetricType::HISTOGRAM); - histogram_metrics_family.set_name(envoy_histogram.name()); - auto* histogram_metric = histogram_metrics_family.add_metric(); - histogram_metric->set_timestamp_ms(snapshot_time_ms); + auto* histogram_metric = + populateMetricsFamily(histogram_metrics_family, io::prometheus::client::MetricType::HISTOGRAM, + snapshot_time_ms, envoy_histogram); auto* histogram = histogram_metric->mutable_histogram(); histogram->set_sample_count(hist_stats.sampleCount()); histogram->set_sample_sum(hist_stats.sampleSum()); @@ -139,6 +133,31 @@ void MetricsFlusher::flushHistogram(io::prometheus::client::MetricFamily& summar bucket->set_cumulative_count(hist_stats.computedBuckets()[i]); } } + +io::prometheus::client::Metric* +MetricsFlusher::populateMetricsFamily(io::prometheus::client::MetricFamily& metrics_family, + io::prometheus::client::MetricType type, + int64_t snapshot_time_ms, const Stats::Metric& metric) const { + metrics_family.set_type(type); + auto* prometheus_metric = metrics_family.add_metric(); + prometheus_metric->set_timestamp_ms(snapshot_time_ms); + + if (emit_labels_) { + // TODO(snowp): Look into the perf implication of this. We need to take a lock on the symbol + // table to stringify the StatNames, which could result in some lock contention. Consider + // caching the conversion between stat handle to extracted tags. + metrics_family.set_name(metric.tagExtractedName()); + for (const auto& tag : metric.tags()) { + auto* label = prometheus_metric->add_label(); + label->set_name(tag.name_); + label->set_value(tag.value_); + } + } else { + metrics_family.set_name(metric.name()); + } + + return prometheus_metric; +} } // namespace MetricsService } // namespace StatSinks } // namespace Extensions diff --git a/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.h b/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.h index 782d887caf7f7..7baef22c5bc07 100644 --- a/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.h +++ b/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.h @@ -82,8 +82,8 @@ using GrpcMetricsStreamerImplPtr = std::unique_ptr; class MetricsFlusher { public: - explicit MetricsFlusher(const bool report_counters_as_deltas) - : report_counters_as_deltas_(report_counters_as_deltas) {} + MetricsFlusher(bool report_counters_as_deltas, bool emit_labels) + : report_counters_as_deltas_(report_counters_as_deltas), emit_labels_(emit_labels) {} MetricsPtr flush(Stats::MetricSnapshot& snapshot) const; @@ -98,7 +98,13 @@ class MetricsFlusher { const Stats::ParentHistogram& envoy_histogram, int64_t snapshot_time_ms) const; + io::prometheus::client::Metric* + populateMetricsFamily(io::prometheus::client::MetricFamily& metrics_family, + io::prometheus::client::MetricType type, int64_t snapshot_time_ms, + const Stats::Metric& metric) const; + const bool report_counters_as_deltas_; + const bool emit_labels_; }; /** @@ -109,8 +115,9 @@ template class MetricsServiceSink : pu // MetricsService::Sink MetricsServiceSink( const GrpcMetricsStreamerSharedPtr& grpc_metrics_streamer, - const bool report_counters_as_deltas) - : flusher_(report_counters_as_deltas), grpc_metrics_streamer_(grpc_metrics_streamer) {} + bool report_counters_as_deltas, bool emit_labels) + : flusher_(report_counters_as_deltas, emit_labels), + grpc_metrics_streamer_(grpc_metrics_streamer) {} void flush(Stats::MetricSnapshot& snapshot) override { grpc_metrics_streamer_->send(flusher_.flush(snapshot)); diff --git a/test/extensions/stats_sinks/metrics_service/grpc_metrics_service_impl_test.cc b/test/extensions/stats_sinks/metrics_service/grpc_metrics_service_impl_test.cc index bafd9bc579606..3b77c71ceaed2 100644 --- a/test/extensions/stats_sinks/metrics_service/grpc_metrics_service_impl_test.cc +++ b/test/extensions/stats_sinks/metrics_service/grpc_metrics_service_impl_test.cc @@ -111,7 +111,7 @@ class MetricsServiceSinkTest : public testing::Test { TEST_F(MetricsServiceSinkTest, CheckSendCall) { MetricsServiceSink - sink(streamer_, false); + sink(streamer_, false, false); auto counter = std::make_shared>(); counter->name_ = "test_counter"; @@ -137,7 +137,7 @@ TEST_F(MetricsServiceSinkTest, CheckSendCall) { TEST_F(MetricsServiceSinkTest, CheckStatsCount) { MetricsServiceSink - sink(streamer_, false); + sink(streamer_, false, false); auto counter = std::make_shared>(); counter->name_ = "test_counter"; @@ -168,7 +168,7 @@ TEST_F(MetricsServiceSinkTest, CheckStatsCount) { TEST_F(MetricsServiceSinkTest, ReportCountersValues) { MetricsServiceSink - sink(streamer_, false); + sink(streamer_, false, false); auto counter = std::make_shared>(); counter->name_ = "test_counter"; @@ -187,7 +187,7 @@ TEST_F(MetricsServiceSinkTest, ReportCountersValues) { TEST_F(MetricsServiceSinkTest, ReportCountersAsDeltas) { MetricsServiceSink - sink(streamer_, true); + sink(streamer_, true, false); auto counter = std::make_shared>(); counter->name_ = "test_counter"; @@ -202,6 +202,86 @@ TEST_F(MetricsServiceSinkTest, ReportCountersAsDeltas) { sink.flush(snapshot_); } +// Test the behavior of tag emission based on the emit_tags_as_label flag. +TEST_F(MetricsServiceSinkTest, ReportMetricsWithTags) { + auto counter = std::make_shared>(); + counter->name_ = "full-counter-name"; + counter->value_ = 100; + counter->used_ = true; + counter->setTagExtractedName("tag-counter-name"); + counter->setTags({{"a", "b"}}); + snapshot_.counters_.push_back({1, *counter}); + + auto gauge = std::make_shared>(); + gauge->name_ = "full-gauge-name"; + gauge->value_ = 100; + gauge->used_ = true; + gauge->setTagExtractedName("tag-gauge-name"); + gauge->setTags({{"a", "b"}}); + snapshot_.gauges_.push_back({*gauge}); + + auto histogram = std::make_shared>(); + histogram->name_ = "full-histogram-name"; + histogram->used_ = true; + histogram->setTagExtractedName("tag-histogram-name"); + histogram->setTags({{"a", "b"}}); + snapshot_.histograms_.push_back({*histogram}); + + { + // When the emit_tags flag is false, we don't emit the tags and use the full name. + MetricsServiceSink + sink(streamer_, true, false); + + EXPECT_CALL(*streamer_, send(_)).WillOnce(Invoke([](MetricsPtr&& metrics) { + EXPECT_EQ(4, metrics->size()); + + EXPECT_EQ("full-counter-name", (*metrics)[0].name()); + EXPECT_EQ(0, (*metrics)[0].metric(0).label().size()); + + EXPECT_EQ("full-gauge-name", (*metrics)[1].name()); + EXPECT_EQ(0, (*metrics)[1].metric(0).label().size()); + + EXPECT_EQ("full-histogram-name", (*metrics)[2].name()); + EXPECT_EQ(0, (*metrics)[2].metric(0).label().size()); + + EXPECT_EQ("full-histogram-name", (*metrics)[3].name()); + EXPECT_EQ(0, (*metrics)[3].metric(0).label().size()); + })); + sink.flush(snapshot_); + } + + io::prometheus::client::LabelPair expected_label_pair; + expected_label_pair.set_name("a"); + expected_label_pair.set_value("b"); + + // When the emit_tags flag is true, we emit the tags as labels and use the tag extracted name. + MetricsServiceSink + sink(streamer_, true, true); + + EXPECT_CALL(*streamer_, send(_)).WillOnce(Invoke([&expected_label_pair](MetricsPtr&& metrics) { + EXPECT_EQ(4, metrics->size()); + + EXPECT_EQ("tag-counter-name", (*metrics)[0].name()); + EXPECT_EQ(1, (*metrics)[0].metric(0).label().size()); + EXPECT_TRUE(TestUtility::protoEqual(expected_label_pair, (*metrics)[0].metric(0).label()[0])); + + EXPECT_EQ("tag-gauge-name", (*metrics)[1].name()); + EXPECT_EQ(1, (*metrics)[1].metric(0).label().size()); + EXPECT_TRUE(TestUtility::protoEqual(expected_label_pair, (*metrics)[0].metric(0).label()[0])); + + EXPECT_EQ("tag-histogram-name", (*metrics)[2].name()); + EXPECT_EQ(1, (*metrics)[2].metric(0).label().size()); + EXPECT_TRUE(TestUtility::protoEqual(expected_label_pair, (*metrics)[0].metric(0).label()[0])); + + EXPECT_EQ("tag-histogram-name", (*metrics)[3].name()); + EXPECT_EQ(1, (*metrics)[3].metric(0).label().size()); + EXPECT_TRUE(TestUtility::protoEqual(expected_label_pair, (*metrics)[0].metric(0).label()[0])); + })); + sink.flush(snapshot_); +} + } // namespace } // namespace MetricsService } // namespace StatSinks