[GitHub] [airflow] sudeepgupta90 commented on a diff in pull request #31040: Added kube-prometheus-stack servicemonitor to gather metrics from Airflow statsd exporter

["sudeepgupta90 (via GitHub)" <gi...@apache.org>]

sudeepgupta90 commented on code in PR #31040:
URL: https://github.com/apache/airflow/pull/31040#discussion_r1183792504


##########
chart/templates/statsd/statsd-servicemonitor.yaml:
##########
@@ -0,0 +1,48 @@
+{{- if and .Values.statsd.enabled .Values.statsd.serviceMonitor.enabled }}
+apiVersion: monitoring.coreos.com/v1
+kind: ServiceMonitor
+metadata:
+  name: {{ .Release.Name }}
+  {{- if .Values.statsd.serviceMonitor.namespace }}
+  namespace: {{ .Values.statsd.serviceMonitor.namespace }}
+  {{- else }}
+  namespace: {{ .Release.Namespace }}
+  {{- end }}
+  {{- if .Values.statsd.serviceMonitor.labels }}
+  labels: 
+  {{- include "common.tplvalues.render" ( dict "value" .Values.statsd.serviceMonitor.labels "context" $ ) | nindent 4 }}

Review Comment:
   ```suggestion
     {{- .Values.statsd.serviceMonitor.labels | toYaml | nindent 4 }}
   ```



##########
chart/templates/statsd/statsd-servicemonitor.yaml:
##########
@@ -0,0 +1,48 @@
+{{- if and .Values.statsd.enabled .Values.statsd.serviceMonitor.enabled }}
+apiVersion: monitoring.coreos.com/v1
+kind: ServiceMonitor
+metadata:
+  name: {{ .Release.Name }}
+  {{- if .Values.statsd.serviceMonitor.namespace }}
+  namespace: {{ .Values.statsd.serviceMonitor.namespace }}
+  {{- else }}
+  namespace: {{ .Release.Namespace }}
+  {{- end }}
+  {{- if .Values.statsd.serviceMonitor.labels }}
+  labels: 
+  {{- include "common.tplvalues.render" ( dict "value" .Values.statsd.serviceMonitor.labels "context" $ ) | nindent 4 }}
+  {{- end }}    
+  {{- if .Values.commonAnnotations }}
+  annotations:

Review Comment:
   ```suggestion
     annotations:
       {{- .Values.commonAnnotations | toYaml | nindent 4}}
   ```



##########
chart/templates/_tplvalues.tpl:
##########
@@ -0,0 +1,13 @@
+{{/* vim: set filetype=mustache: */}}

Review Comment:
   What's the need for this special function? We can simply have the label values as is, no?



##########
chart/templates/statsd/statsd-servicemonitor.yaml:
##########
@@ -0,0 +1,48 @@
+{{- if and .Values.statsd.enabled .Values.statsd.serviceMonitor.enabled }}
+apiVersion: monitoring.coreos.com/v1
+kind: ServiceMonitor
+metadata:
+  name: {{ .Release.Name }}
+  {{- if .Values.statsd.serviceMonitor.namespace }}
+  namespace: {{ .Values.statsd.serviceMonitor.namespace }}
+  {{- else }}
+  namespace: {{ .Release.Namespace }}
+  {{- end }}
+  {{- if .Values.statsd.serviceMonitor.labels }}
+  labels: 
+  {{- include "common.tplvalues.render" ( dict "value" .Values.statsd.serviceMonitor.labels "context" $ ) | nindent 4 }}

Review Comment:
   Why would these label values have a template?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org