From f1e1833c9fa423004eed406a6352ee03488ae82f Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Tue, 29 Oct 2024 16:07:02 +0100 Subject: [PATCH 01/11] fix typo in docu for makePrometheusConfigForServices func --- .../otelcollector/config/metric/agent/prometheus_receiver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/otelcollector/config/metric/agent/prometheus_receiver.go b/internal/otelcollector/config/metric/agent/prometheus_receiver.go index 21392c3be..df6428fba 100644 --- a/internal/otelcollector/config/metric/agent/prometheus_receiver.go +++ b/internal/otelcollector/config/metric/agent/prometheus_receiver.go @@ -32,7 +32,7 @@ func makePrometheusConfigForPods(opts BuildOptions) *PrometheusReceiver { return makePrometheusConfig(opts, "app-pods", RolePod, makePrometheusPodsRelabelConfigs) } -// makePrometheusConfigForPods creates a Prometheus configuration for scraping Services that are annotated with prometheus.io annotations. +// makePrometheusConfigForServices creates a Prometheus configuration for scraping Services that are annotated with prometheus.io annotations. func makePrometheusConfigForServices(opts BuildOptions) *PrometheusReceiver { return makePrometheusConfig(opts, "app-services", RoleEndpoints, makePrometheusEndpointsRelabelConfigs) } From 675ed2ef3a3e480712cc89cdd685c56a6b27935d Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Tue, 29 Oct 2024 16:09:22 +0100 Subject: [PATCH 02/11] fix typo in docu for makePrometheusConfig func --- .../otelcollector/config/metric/agent/prometheus_receiver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/otelcollector/config/metric/agent/prometheus_receiver.go b/internal/otelcollector/config/metric/agent/prometheus_receiver.go index df6428fba..1a7ed7041 100644 --- a/internal/otelcollector/config/metric/agent/prometheus_receiver.go +++ b/internal/otelcollector/config/metric/agent/prometheus_receiver.go @@ -38,7 +38,7 @@ func makePrometheusConfigForServices(opts BuildOptions) *PrometheusReceiver { } // makePrometheusConfig generates a Prometheus receiver configuration for scraping either annotated Pods or Services (based on the provided role and relabelConfigFn). -// If Istio is enabled, an additional scrape config is generated (prefixed with -secure) to scrape targets over HTTPS using Istio certificate. +// If Istio is enabled, an additional scrape config is generated (suffixed with -secure) to scrape targets over HTTPS using Istio certificate. // Istio certificate is expected to be mounted at the provided path using the proxy.istio.io/config annotation. // See more: https://istio.io/latest/docs/ops/integrations/prometheus/#tls-settings func makePrometheusConfig(opts BuildOptions, jobNamePrefix string, role Role, relabelConfigFn func(keepSecure bool) []RelabelConfig) *PrometheusReceiver { From 7d6ddcb043b6718e5126f94a25aa5695f9d8fa38 Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Tue, 29 Oct 2024 16:22:13 +0100 Subject: [PATCH 03/11] remove app-pods-secure scrape job --- .../otelcollector/config/metric/agent/prometheus_receiver.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/otelcollector/config/metric/agent/prometheus_receiver.go b/internal/otelcollector/config/metric/agent/prometheus_receiver.go index 1a7ed7041..b54a6cfb4 100644 --- a/internal/otelcollector/config/metric/agent/prometheus_receiver.go +++ b/internal/otelcollector/config/metric/agent/prometheus_receiver.go @@ -38,7 +38,7 @@ func makePrometheusConfigForServices(opts BuildOptions) *PrometheusReceiver { } // makePrometheusConfig generates a Prometheus receiver configuration for scraping either annotated Pods or Services (based on the provided role and relabelConfigFn). -// If Istio is enabled, an additional scrape config is generated (suffixed with -secure) to scrape targets over HTTPS using Istio certificate. +// If Istio is enabled, an additional scrape config is generated (suffixed with -secure) to scrape annotated Services over HTTPS using Istio certificate. // Istio certificate is expected to be mounted at the provided path using the proxy.istio.io/config annotation. // See more: https://istio.io/latest/docs/ops/integrations/prometheus/#tls-settings func makePrometheusConfig(opts BuildOptions, jobNamePrefix string, role Role, relabelConfigFn func(keepSecure bool) []RelabelConfig) *PrometheusReceiver { @@ -55,7 +55,8 @@ func makePrometheusConfig(opts BuildOptions, jobNamePrefix string, role Role, re httpScrapeConfig.RelabelConfigs = relabelConfigFn(false) config.Config.ScrapeConfigs = append(config.Config.ScrapeConfigs, httpScrapeConfig) - if opts.IstioEnabled { + // If Istio is enabled, generate an additional scrape config for scraping annotated Services (role == RoleEndpoints) over HTTPS + if opts.IstioEnabled && role == RoleEndpoints { httpsScrapeConfig := baseScrapeConfig httpsScrapeConfig.JobName = jobNamePrefix + "-secure" httpsScrapeConfig.RelabelConfigs = relabelConfigFn(true) From 3c10f73a5963bc29aefda8c06259fd5c2197af1a Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Tue, 29 Oct 2024 16:28:08 +0100 Subject: [PATCH 04/11] adjust unit tests afer removing app-pods-secure scrape job --- .../config/metric/agent/receivers_test.go | 1 - .../agent/testdata/config_istio_enabled.yaml | 47 ------------------- 2 files changed, 48 deletions(-) diff --git a/internal/otelcollector/config/metric/agent/receivers_test.go b/internal/otelcollector/config/metric/agent/receivers_test.go index a7949a30f..5ee4c7a5a 100644 --- a/internal/otelcollector/config/metric/agent/receivers_test.go +++ b/internal/otelcollector/config/metric/agent/receivers_test.go @@ -211,7 +211,6 @@ func TestReceivers(t *testing.T) { istioEnabled: true, expectedPodScrapeJobs: []string{ "app-pods", - "app-pods-secure", }, expectedServiceScrapeJobs: []string{ "app-services", diff --git a/internal/otelcollector/config/metric/agent/testdata/config_istio_enabled.yaml b/internal/otelcollector/config/metric/agent/testdata/config_istio_enabled.yaml index d3691789d..a53ed0550 100644 --- a/internal/otelcollector/config/metric/agent/testdata/config_istio_enabled.yaml +++ b/internal/otelcollector/config/metric/agent/testdata/config_istio_enabled.yaml @@ -209,53 +209,6 @@ receivers: action: replace kubernetes_sd_configs: - role: pod - - job_name: app-pods-secure - sample_limit: 50000 - scrape_interval: 30s - relabel_configs: - - source_labels: [__meta_kubernetes_pod_node_name] - regex: ${MY_NODE_NAME} - action: keep - - source_labels: [__meta_kubernetes_pod_annotation_prometheus_io_scrape] - regex: "true" - action: keep - - source_labels: [__meta_kubernetes_pod_phase] - regex: Pending|Succeeded|Failed - action: drop - - source_labels: [__meta_kubernetes_pod_container_init] - regex: (true) - action: drop - - source_labels: [__meta_kubernetes_pod_container_name] - regex: (istio-proxy) - action: drop - - source_labels: [__meta_kubernetes_pod_label_security_istio_io_tlsMode] - regex: (istio) - target_label: __scheme__ - replacement: https - action: replace - - source_labels: [__meta_kubernetes_pod_annotation_prometheus_io_scheme] - regex: (https?) - target_label: __scheme__ - action: replace - - source_labels: [__scheme__] - regex: (http) - action: drop - - source_labels: [__meta_kubernetes_pod_annotation_prometheus_io_path] - regex: (.+) - target_label: __metrics_path__ - action: replace - - source_labels: [__address__, __meta_kubernetes_pod_annotation_prometheus_io_port] - regex: ([^:]+)(?::\d+)?;(\d+) - target_label: __address__ - replacement: $$1:$$2 - action: replace - kubernetes_sd_configs: - - role: pod - tls_config: - ca_file: /etc/istio-output-certs/root-cert.pem - cert_file: /etc/istio-output-certs/cert-chain.pem - key_file: /etc/istio-output-certs/key.pem - insecure_skip_verify: true prometheus/app-services: config: scrape_configs: From 846832b4a0c5fb7590a8a2ee22e74017e5228160 Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Tue, 5 Nov 2024 23:56:11 +0100 Subject: [PATCH 05/11] ignore prometheus.io/scheme annotation for app-pods scrape job --- .../metric/agent/prometheus_receiver.go | 21 ++++++------------- .../agent/testdata/config_istio_enabled.yaml | 4 ---- .../testdata/config_istio_not_enabled.yaml | 4 ---- 3 files changed, 6 insertions(+), 23 deletions(-) diff --git a/internal/otelcollector/config/metric/agent/prometheus_receiver.go b/internal/otelcollector/config/metric/agent/prometheus_receiver.go index b54a6cfb4..4ed0f720d 100644 --- a/internal/otelcollector/config/metric/agent/prometheus_receiver.go +++ b/internal/otelcollector/config/metric/agent/prometheus_receiver.go @@ -71,28 +71,19 @@ func makePrometheusConfig(opts BuildOptions, jobNamePrefix string, role Role, re // They restrict Pods that are selected for scraping and set internal labels (__address__, __scheme__, etc.). // See more: https://prometheus.io/docs/prometheus/latest/configuration/configuration/#pod. // -// If requireHTTPS is true, only Pods with Istio sidecars or those explicitly marked with prometheus.io/scheme=http annotations are selected. -// If requireHTTPS is false, only Pods without Istio sidecars or those marked with prometheus.io/scheme=https annotation are selected. -func makePrometheusPodsRelabelConfigs(requireHTTPS bool) []RelabelConfig { - relabelConfigs := []RelabelConfig{ +// Only Pods without Istio sidecars are selected. +func makePrometheusPodsRelabelConfigs(_ bool) []RelabelConfig { + return []RelabelConfig{ keepIfRunningOnSameNode(NodeAffiliatedPod), keepIfScrapingEnabled(AnnotatedPod), dropIfPodNotRunning(), dropIfInitContainer(), dropIfIstioProxy(), inferSchemeFromIstioInjectedLabel(), - inferSchemeFromAnnotation(AnnotatedPod), - } - - if requireHTTPS { - relabelConfigs = append(relabelConfigs, dropIfSchemeHTTP()) - } else { - relabelConfigs = append(relabelConfigs, dropIfSchemeHTTPS()) - } - - return append(relabelConfigs, + dropIfSchemeHTTPS(), inferMetricsPathFromAnnotation(AnnotatedPod), - inferAddressFromAnnotation(AnnotatedPod)) + inferAddressFromAnnotation(AnnotatedPod), + } } // makePrometheusEndpointsRelabelConfigs generates a set of relabel configs for the Endpoint role type. diff --git a/internal/otelcollector/config/metric/agent/testdata/config_istio_enabled.yaml b/internal/otelcollector/config/metric/agent/testdata/config_istio_enabled.yaml index a53ed0550..8bcdfc6d6 100644 --- a/internal/otelcollector/config/metric/agent/testdata/config_istio_enabled.yaml +++ b/internal/otelcollector/config/metric/agent/testdata/config_istio_enabled.yaml @@ -191,10 +191,6 @@ receivers: target_label: __scheme__ replacement: https action: replace - - source_labels: [__meta_kubernetes_pod_annotation_prometheus_io_scheme] - regex: (https?) - target_label: __scheme__ - action: replace - source_labels: [__scheme__] regex: (https) action: drop diff --git a/internal/otelcollector/config/metric/agent/testdata/config_istio_not_enabled.yaml b/internal/otelcollector/config/metric/agent/testdata/config_istio_not_enabled.yaml index 3935e1964..d730ccae2 100644 --- a/internal/otelcollector/config/metric/agent/testdata/config_istio_not_enabled.yaml +++ b/internal/otelcollector/config/metric/agent/testdata/config_istio_not_enabled.yaml @@ -180,10 +180,6 @@ receivers: target_label: __scheme__ replacement: https action: replace - - source_labels: [__meta_kubernetes_pod_annotation_prometheus_io_scheme] - regex: (https?) - target_label: __scheme__ - action: replace - source_labels: [__scheme__] regex: (https) action: drop From 512f610c193ff616ae861e7c3bd397f7101ca2e2 Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Wed, 6 Nov 2024 00:21:35 +0100 Subject: [PATCH 06/11] update docs --- docs/user/04-metrics.md | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/docs/user/04-metrics.md b/docs/user/04-metrics.md index 898a93e00..03003eacf 100644 --- a/docs/user/04-metrics.md +++ b/docs/user/04-metrics.md @@ -313,14 +313,16 @@ spec: The Metric agent is configured with a generic scrape configuration, which uses annotations to specify the endpoints to scrape in the cluster. -For metrics ingestion to start automatically, simply apply the following annotations either to a Service that resolves your metrics port, or directly to the Pod: - -| Annotation Key | Example Values | Default Value | Description | -|------------------------------------|-------------------|-------------- |---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `prometheus.io/scrape` (mandatory) | `true`, `false` | none | Controls whether Prometheus automatically scrapes metrics from this target. | -| `prometheus.io/port` (mandatory) | `8080`, `9100` | none | Specifies the port where the metrics are exposed. | -| `prometheus.io/path` | `/metrics`, `/custom_metrics` | `/metrics` | Defines the HTTP path where Prometheus can find metrics data. | -| `prometheus.io/scheme` | `http`, `https` | If Istio is active, `https` is supported; otherwise, only `http` is available. The default scheme is `http` unless an Istio sidecar is present, denoted by the label `security.istio.io/tlsMode=istio`, in which case `https` becomes the default. | Determines the protocol used for scraping metrics — either HTTPS with mTLS or plain HTTP. | +For metrics ingestion to start automatically: +* If istio is not active, apply the annotations in the table below either to a Service that resolves your metrics port, or directly to the Pod. +* If istio is active, apply the annotations in the table below to a Service that resolves your metrics port. Applying the annotations directly to the Pod will not trigger metrics scraping if istio is active. + +| Annotation Key | Example Values | Default Value | Description | +|------------------------------------------------------------------|-------------------|-------------- |---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `prometheus.io/scrape` (mandatory) | `true`, `false` | none | Controls whether Prometheus automatically scrapes metrics from this target. | +| `prometheus.io/port` (mandatory) | `8080`, `9100` | none | Specifies the port where the metrics are exposed. | +| `prometheus.io/path` | `/metrics`, `/custom_metrics` | `/metrics` | Defines the HTTP path where Prometheus can find metrics data. | +| `prometheus.io/scheme` (only relevant when annotating a Service) | `http`, `https` | If Istio is active, `https` is supported; otherwise, only `http` is available. The default scheme is `http` unless an Istio sidecar is present, denoted by the label `security.istio.io/tlsMode=istio`, in which case `https` becomes the default. | Determines the protocol used for scraping metrics — either HTTPS with mTLS or plain HTTP. | If you're running the Pod targeted by a Service with Istio, Istio must be able to derive the [appProtocol](https://kubernetes.io/docs/concepts/services-networking/service/#application-protocol) from the Service port definition; otherwise the communication for scraping the metric endpoint cannot be established. You must either prefix the port name with the protocol like in `http-metrics`, or explicitly define the `appProtocol` attribute. From 9b31abfab4a581cc11e7d4b8e46ca43cd37771c1 Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Wed, 6 Nov 2024 21:26:23 +0100 Subject: [PATCH 07/11] adjust e2e tests --- .../istio/metrics_prometheus_input_test.go | 59 ++++++++++++++----- .../mocks/prommetricgen/metric_producer.go | 9 ++- 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/test/integration/istio/metrics_prometheus_input_test.go b/test/integration/istio/metrics_prometheus_input_test.go index 89568b572..c0995ccc2 100644 --- a/test/integration/istio/metrics_prometheus_input_test.go +++ b/test/integration/istio/metrics_prometheus_input_test.go @@ -49,10 +49,12 @@ var _ = Describe(suite.ID(), Label(suite.LabelIntegration), Ordered, func() { objs = append(objs, []client.Object{ httpsAnnotatedMetricProducer.Pod().WithSidecarInjection().WithPrometheusAnnotations(prommetricgen.SchemeHTTPS).K8sObject(), httpsAnnotatedMetricProducer.Service().WithPrometheusAnnotations(prommetricgen.SchemeHTTPS).K8sObject(), + httpAnnotatedMetricProducer.Pod().WithPrometheusAnnotations(prommetricgen.SchemeHTTP).K8sObject(), httpAnnotatedMetricProducer.Service().WithPrometheusAnnotations(prommetricgen.SchemeHTTP).K8sObject(), - unannotatedMetricProducer.Pod().WithPrometheusAnnotations(prommetricgen.SchemeHTTP).K8sObject(), - unannotatedMetricProducer.Service().WithPrometheusAnnotations(prommetricgen.SchemeHTTP).K8sObject(), + + unannotatedMetricProducer.Pod().WithPrometheusAnnotations(prommetricgen.SchemeNone).K8sObject(), + unannotatedMetricProducer.Service().WithPrometheusAnnotations(prommetricgen.SchemeNone).K8sObject(), }...) metricPipeline := testutils.NewMetricPipelineBuilder(). @@ -92,8 +94,8 @@ var _ = Describe(suite.ID(), Label(suite.LabelIntegration), Ordered, func() { // here we are discovering the same metric-producer workload twice: once via the annotated service and once via the annotated pod // targets discovered via annotated pods must have no service label Context("Annotated pods", func() { - It("Should scrape if prometheus.io/scheme=https", func() { - podScrapedMetricsShouldBeDelivered(backendExportURL, httpsAnnotatedMetricProducerName) + It("Should NOT scrape if istio sidecar exists", func() { + podMetricsShouldNotBeDelivered(backendExportURL, httpsAnnotatedMetricProducerName) }) It("Should scrape if prometheus.io/scheme=http", func() { @@ -124,8 +126,27 @@ var _ = Describe(suite.ID(), Label(suite.LabelIntegration), Ordered, func() { }) }) -func podScrapedMetricsShouldBeDelivered(proxyURL, podName string) { +func podMetricsShouldNotBeDelivered(proxyURL, targetName string) { + Consistently(func(g Gomega) { + resp, err := proxyClient.Get(proxyURL) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(resp).To(HaveHTTPStatus(http.StatusOK)) + + bodyContent, err := io.ReadAll(resp.Body) + defer resp.Body.Close() + g.Expect(err).NotTo(HaveOccurred()) + + g.Expect(bodyContent).To(HaveFlatMetrics( + Not(ContainElement(SatisfyAll( + HaveName(BeElementOf(prommetricgen.MetricNames)), + Not(HaveMetricAttributes(HaveKeyWithValue("service", targetName))), + HaveResourceAttributes(HaveKeyWithValue("k8s.pod.name", targetName)), + ))), + )) + }, periodic.TelemetryConsistentlyTimeout, periodic.TelemetryInterval).Should(Succeed()) +} +func podScrapedMetricsShouldBeDelivered(proxyURL, targetName string) { Eventually(func(g Gomega) { resp, err := proxyClient.Get(proxyURL) g.Expect(err).NotTo(HaveOccurred()) @@ -136,11 +157,16 @@ func podScrapedMetricsShouldBeDelivered(proxyURL, podName string) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(bodyContent).To(HaveFlatMetrics( - SatisfyAll( - ContainElement(HaveResourceAttributes(HaveKeyWithValue("k8s.pod.name", podName))), - ContainElement(HaveName(BeElementOf(prommetricgen.MetricNames))), - ContainElement(HaveScopeName(ContainSubstring(InstrumentationScopePrometheus))), - ), + ContainElement(SatisfyAll( + HaveName(BeElementOf(prommetricgen.MetricNames)), + Not(HaveMetricAttributes(HaveKeyWithValue("service", targetName))), + HaveResourceAttributes(HaveKeyWithValue("k8s.pod.name", targetName)), + HaveScopeName(Equal(InstrumentationScopePrometheus)), + HaveScopeVersion(SatisfyAny( + Equal("main"), + MatchRegexp("[0-9]+.[0-9]+.[0-9]+"), + )), + )), )) }, periodic.TelemetryEventuallyTimeout, periodic.TelemetryInterval).Should(Succeed()) } @@ -156,10 +182,15 @@ func serviceScrapedMetricsShouldBeDelivered(proxyURL, serviceName string) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(bodyContent).To(HaveFlatMetrics( - SatisfyAll( - ContainElement(HaveName(BeElementOf(prommetricgen.MetricNames))), - ContainElement(HaveMetricAttributes(HaveKeyWithValue("service", serviceName))), - ), + ContainElement(SatisfyAll( + HaveName(BeElementOf(prommetricgen.MetricNames)), + HaveMetricAttributes(HaveKeyWithValue("service", serviceName)), + HaveScopeName(Equal(InstrumentationScopePrometheus)), + HaveScopeVersion(SatisfyAny( + Equal("main"), + MatchRegexp("[0-9]+.[0-9]+.[0-9]+"), + )), + )), )) }, periodic.TelemetryEventuallyTimeout, periodic.TelemetryInterval).Should(Succeed()) } diff --git a/test/testkit/mocks/prommetricgen/metric_producer.go b/test/testkit/mocks/prommetricgen/metric_producer.go index b832c7216..10578642c 100644 --- a/test/testkit/mocks/prommetricgen/metric_producer.go +++ b/test/testkit/mocks/prommetricgen/metric_producer.go @@ -28,6 +28,7 @@ type ScrapingScheme string const ( SchemeHTTP ScrapingScheme = "http" SchemeHTTPS ScrapingScheme = "https" + SchemeNone ScrapingScheme = "none" ) var ( @@ -149,12 +150,16 @@ func (p *Pod) WithLabel(key, value string) *Pod { } func makePrometheusAnnotations(scheme ScrapingScheme) map[string]string { - return map[string]string{ + annotations := map[string]string{ "prometheus.io/scrape": "true", "prometheus.io/path": metricsEndpoint, "prometheus.io/port": strconv.Itoa(int(metricsPort)), - "prometheus.io/scheme": string(scheme), } + if scheme != SchemeNone { + annotations["prometheus.io/scheme"] = string(scheme) + } + + return annotations } func (p *Pod) K8sObject() *corev1.Pod { From c9793e93412e858c0f0a4c45cc1783268db37697 Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Wed, 6 Nov 2024 23:41:55 +0100 Subject: [PATCH 08/11] update checks for delivery of pod metrics --- .../istio/metrics_prometheus_input_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/integration/istio/metrics_prometheus_input_test.go b/test/integration/istio/metrics_prometheus_input_test.go index c0995ccc2..b8ca1ee3d 100644 --- a/test/integration/istio/metrics_prometheus_input_test.go +++ b/test/integration/istio/metrics_prometheus_input_test.go @@ -126,27 +126,27 @@ var _ = Describe(suite.ID(), Label(suite.LabelIntegration), Ordered, func() { }) }) -func podMetricsShouldNotBeDelivered(proxyURL, targetName string) { +func podMetricsShouldNotBeDelivered(proxyURL, podName string) { Consistently(func(g Gomega) { resp, err := proxyClient.Get(proxyURL) g.Expect(err).NotTo(HaveOccurred()) g.Expect(resp).To(HaveHTTPStatus(http.StatusOK)) bodyContent, err := io.ReadAll(resp.Body) - defer resp.Body.Close() + resp.Body.Close() g.Expect(err).NotTo(HaveOccurred()) g.Expect(bodyContent).To(HaveFlatMetrics( Not(ContainElement(SatisfyAll( HaveName(BeElementOf(prommetricgen.MetricNames)), - Not(HaveMetricAttributes(HaveKeyWithValue("service", targetName))), - HaveResourceAttributes(HaveKeyWithValue("k8s.pod.name", targetName)), + Not(HaveMetricAttributes(HaveKey("service"))), + HaveResourceAttributes(HaveKeyWithValue("k8s.pod.name", podName)), ))), )) }, periodic.TelemetryConsistentlyTimeout, periodic.TelemetryInterval).Should(Succeed()) } -func podScrapedMetricsShouldBeDelivered(proxyURL, targetName string) { +func podScrapedMetricsShouldBeDelivered(proxyURL, podName string) { Eventually(func(g Gomega) { resp, err := proxyClient.Get(proxyURL) g.Expect(err).NotTo(HaveOccurred()) @@ -159,8 +159,8 @@ func podScrapedMetricsShouldBeDelivered(proxyURL, targetName string) { g.Expect(bodyContent).To(HaveFlatMetrics( ContainElement(SatisfyAll( HaveName(BeElementOf(prommetricgen.MetricNames)), - Not(HaveMetricAttributes(HaveKeyWithValue("service", targetName))), - HaveResourceAttributes(HaveKeyWithValue("k8s.pod.name", targetName)), + Not(HaveMetricAttributes(HaveKey("service"))), + HaveResourceAttributes(HaveKeyWithValue("k8s.pod.name", podName)), HaveScopeName(Equal(InstrumentationScopePrometheus)), HaveScopeVersion(SatisfyAny( Equal("main"), From d783e68abf0e7569a3202aa3ca40ede780205a2d Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Thu, 7 Nov 2024 02:32:37 +0100 Subject: [PATCH 09/11] increase consistently timeout in e2e test --- test/integration/istio/metrics_prometheus_input_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/istio/metrics_prometheus_input_test.go b/test/integration/istio/metrics_prometheus_input_test.go index b8ca1ee3d..b295abb22 100644 --- a/test/integration/istio/metrics_prometheus_input_test.go +++ b/test/integration/istio/metrics_prometheus_input_test.go @@ -143,7 +143,7 @@ func podMetricsShouldNotBeDelivered(proxyURL, podName string) { HaveResourceAttributes(HaveKeyWithValue("k8s.pod.name", podName)), ))), )) - }, periodic.TelemetryConsistentlyTimeout, periodic.TelemetryInterval).Should(Succeed()) + }, 3*periodic.TelemetryConsistentlyTimeout, periodic.TelemetryInterval).Should(Succeed()) } func podScrapedMetricsShouldBeDelivered(proxyURL, podName string) { From a3417646d1a5f59261bc7923f11f91b894ee87f9 Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Thu, 7 Nov 2024 14:27:26 +0100 Subject: [PATCH 10/11] update metrics docu --- docs/user/04-metrics.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/user/04-metrics.md b/docs/user/04-metrics.md index 03003eacf..fc5497806 100644 --- a/docs/user/04-metrics.md +++ b/docs/user/04-metrics.md @@ -313,9 +313,9 @@ spec: The Metric agent is configured with a generic scrape configuration, which uses annotations to specify the endpoints to scrape in the cluster. -For metrics ingestion to start automatically: -* If istio is not active, apply the annotations in the table below either to a Service that resolves your metrics port, or directly to the Pod. -* If istio is active, apply the annotations in the table below to a Service that resolves your metrics port. Applying the annotations directly to the Pod will not trigger metrics scraping if istio is active. +For metrics ingestion to start automatically, use the annotations of the following table. +If an Istio sidecar is present, apply them to a Service that resolves your metrics port. +Only if Istio sidecar is not present, you can alternatively apply the annotations directly to the Pod. | Annotation Key | Example Values | Default Value | Description | |------------------------------------------------------------------|-------------------|-------------- |---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| From d17d0eb28ed34e58443191a37966165426c13d5d Mon Sep 17 00:00:00 2001 From: Mostafa Shorim Date: Thu, 7 Nov 2024 15:21:22 +0100 Subject: [PATCH 11/11] remove dropIfIstioProxy from pods relabel config and furether refactoring --- .../metric/agent/prometheus_receiver.go | 49 +++++++++++-------- .../config/metric/agent/receivers.go | 2 +- .../agent/testdata/config_istio_enabled.yaml | 3 -- .../testdata/config_istio_not_enabled.yaml | 3 -- 4 files changed, 30 insertions(+), 27 deletions(-) diff --git a/internal/otelcollector/config/metric/agent/prometheus_receiver.go b/internal/otelcollector/config/metric/agent/prometheus_receiver.go index 4ed0f720d..63edb3c20 100644 --- a/internal/otelcollector/config/metric/agent/prometheus_receiver.go +++ b/internal/otelcollector/config/metric/agent/prometheus_receiver.go @@ -23,43 +23,53 @@ const ( ) const ( - scrapeInterval = 30 * time.Second - sampleLimit = 50000 + scrapeInterval = 30 * time.Second + sampleLimit = 50000 + appPodsJobName = "app-pods" + appServicesJobName = "app-services" + appServicesSecureJobName = "app-services-secure" ) // makePrometheusConfigForPods creates a Prometheus configuration for scraping Pods that are annotated with prometheus.io annotations. -func makePrometheusConfigForPods(opts BuildOptions) *PrometheusReceiver { - return makePrometheusConfig(opts, "app-pods", RolePod, makePrometheusPodsRelabelConfigs) -} +func makePrometheusConfigForPods() *PrometheusReceiver { + var config PrometheusReceiver -// makePrometheusConfigForServices creates a Prometheus configuration for scraping Services that are annotated with prometheus.io annotations. -func makePrometheusConfigForServices(opts BuildOptions) *PrometheusReceiver { - return makePrometheusConfig(opts, "app-services", RoleEndpoints, makePrometheusEndpointsRelabelConfigs) + scrapeConfig := ScrapeConfig{ + ScrapeInterval: scrapeInterval, + SampleLimit: sampleLimit, + KubernetesDiscoveryConfigs: []KubernetesDiscoveryConfig{{Role: RolePod}}, + JobName: appPodsJobName, + RelabelConfigs: makePrometheusPodsRelabelConfigs(), + } + + config.Config.ScrapeConfigs = append(config.Config.ScrapeConfigs, scrapeConfig) + + return &config } -// makePrometheusConfig generates a Prometheus receiver configuration for scraping either annotated Pods or Services (based on the provided role and relabelConfigFn). -// If Istio is enabled, an additional scrape config is generated (suffixed with -secure) to scrape annotated Services over HTTPS using Istio certificate. +// makePrometheusConfigForServices creates a Prometheus configuration for scraping Services that are annotated with prometheus.io annotations. +// If Istio is enabled, an additional scrape job config is generated (suffixed with -secure) to scrape annotated Services over HTTPS using Istio certificate. // Istio certificate is expected to be mounted at the provided path using the proxy.istio.io/config annotation. // See more: https://istio.io/latest/docs/ops/integrations/prometheus/#tls-settings -func makePrometheusConfig(opts BuildOptions, jobNamePrefix string, role Role, relabelConfigFn func(keepSecure bool) []RelabelConfig) *PrometheusReceiver { +func makePrometheusConfigForServices(opts BuildOptions) *PrometheusReceiver { var config PrometheusReceiver baseScrapeConfig := ScrapeConfig{ ScrapeInterval: scrapeInterval, SampleLimit: sampleLimit, - KubernetesDiscoveryConfigs: []KubernetesDiscoveryConfig{{Role: role}}, + KubernetesDiscoveryConfigs: []KubernetesDiscoveryConfig{{Role: RoleEndpoints}}, } httpScrapeConfig := baseScrapeConfig - httpScrapeConfig.JobName = jobNamePrefix - httpScrapeConfig.RelabelConfigs = relabelConfigFn(false) + httpScrapeConfig.JobName = appServicesJobName + httpScrapeConfig.RelabelConfigs = makePrometheusEndpointsRelabelConfigs(false) config.Config.ScrapeConfigs = append(config.Config.ScrapeConfigs, httpScrapeConfig) - // If Istio is enabled, generate an additional scrape config for scraping annotated Services (role == RoleEndpoints) over HTTPS - if opts.IstioEnabled && role == RoleEndpoints { + // If Istio is enabled, generate an additional scrape config for scraping annotated Services over HTTPS + if opts.IstioEnabled { httpsScrapeConfig := baseScrapeConfig - httpsScrapeConfig.JobName = jobNamePrefix + "-secure" - httpsScrapeConfig.RelabelConfigs = relabelConfigFn(true) + httpsScrapeConfig.JobName = appServicesSecureJobName + httpsScrapeConfig.RelabelConfigs = makePrometheusEndpointsRelabelConfigs(true) httpsScrapeConfig.TLSConfig = makeTLSConfig(opts.IstioCertPath) config.Config.ScrapeConfigs = append(config.Config.ScrapeConfigs, httpsScrapeConfig) } @@ -72,13 +82,12 @@ func makePrometheusConfig(opts BuildOptions, jobNamePrefix string, role Role, re // See more: https://prometheus.io/docs/prometheus/latest/configuration/configuration/#pod. // // Only Pods without Istio sidecars are selected. -func makePrometheusPodsRelabelConfigs(_ bool) []RelabelConfig { +func makePrometheusPodsRelabelConfigs() []RelabelConfig { return []RelabelConfig{ keepIfRunningOnSameNode(NodeAffiliatedPod), keepIfScrapingEnabled(AnnotatedPod), dropIfPodNotRunning(), dropIfInitContainer(), - dropIfIstioProxy(), inferSchemeFromIstioInjectedLabel(), dropIfSchemeHTTPS(), inferMetricsPathFromAnnotation(AnnotatedPod), diff --git a/internal/otelcollector/config/metric/agent/receivers.go b/internal/otelcollector/config/metric/agent/receivers.go index cf6237495..4d3970c0b 100644 --- a/internal/otelcollector/config/metric/agent/receivers.go +++ b/internal/otelcollector/config/metric/agent/receivers.go @@ -11,7 +11,7 @@ func makeReceiversConfig(inputs inputSources, opts BuildOptions) Receivers { var receiversConfig Receivers if inputs.prometheus { - receiversConfig.PrometheusAppPods = makePrometheusConfigForPods(opts) + receiversConfig.PrometheusAppPods = makePrometheusConfigForPods() receiversConfig.PrometheusAppServices = makePrometheusConfigForServices(opts) } diff --git a/internal/otelcollector/config/metric/agent/testdata/config_istio_enabled.yaml b/internal/otelcollector/config/metric/agent/testdata/config_istio_enabled.yaml index 8bcdfc6d6..30b01b39c 100644 --- a/internal/otelcollector/config/metric/agent/testdata/config_istio_enabled.yaml +++ b/internal/otelcollector/config/metric/agent/testdata/config_istio_enabled.yaml @@ -183,9 +183,6 @@ receivers: - source_labels: [__meta_kubernetes_pod_container_init] regex: (true) action: drop - - source_labels: [__meta_kubernetes_pod_container_name] - regex: (istio-proxy) - action: drop - source_labels: [__meta_kubernetes_pod_label_security_istio_io_tlsMode] regex: (istio) target_label: __scheme__ diff --git a/internal/otelcollector/config/metric/agent/testdata/config_istio_not_enabled.yaml b/internal/otelcollector/config/metric/agent/testdata/config_istio_not_enabled.yaml index d730ccae2..3d6a346bb 100644 --- a/internal/otelcollector/config/metric/agent/testdata/config_istio_not_enabled.yaml +++ b/internal/otelcollector/config/metric/agent/testdata/config_istio_not_enabled.yaml @@ -172,9 +172,6 @@ receivers: - source_labels: [__meta_kubernetes_pod_container_init] regex: (true) action: drop - - source_labels: [__meta_kubernetes_pod_container_name] - regex: (istio-proxy) - action: drop - source_labels: [__meta_kubernetes_pod_label_security_istio_io_tlsMode] regex: (istio) target_label: __scheme__