Miretpl commented on code in PR #64902:
URL: https://github.com/apache/airflow/pull/64902#discussion_r3174884193
##########
chart/values.schema.json:
##########
@@ -8302,7 +8325,7 @@
}
},
"enabled": {
- "description": "Enable StatsD.",
+ "description": "Enable StatsD. When
OtelCollector.MetricsEnabled is true, [metrics] statsd_on is set to false in
the rendered Airflow config because Airflow can only export metrics to one
backend at a time.",
Review Comment:
```suggestion
"description": "Enable StatsD. When
`otelCollector.metricsEnabled` is true, [metrics] statsd_on is set to false in
the rendered Airflow config because Airflow can only export metrics to one
backend at a time.",
```
just for consistency with other fields
##########
chart/templates/otel-collector/otel-collector-deployment.yaml:
##########
@@ -0,0 +1,118 @@
+{{/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+*/}}
+
+################################
+## OTel Collector Deployment
+################################
+{{- if .Values.otelCollector.enabled }}
+{{- $nodeSelector := or .Values.otelCollector.nodeSelector
.Values.nodeSelector }}
+{{- $affinity := or .Values.otelCollector.affinity .Values.affinity }}
+{{- $tolerations := or .Values.otelCollector.tolerations .Values.tolerations }}
+{{- $topologySpreadConstraints := or
.Values.otelCollector.topologySpreadConstraints
.Values.topologySpreadConstraints }}
+{{- $revisionHistoryLimit := include "airflow.revisionHistoryLimit" (list
.Values.otelCollector.revisionHistoryLimit .Values.revisionHistoryLimit) }}
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: {{ include "airflow.fullname" . }}-otel-collector
+ labels:
+ tier: airflow
+ component: otel-collector
+ release: {{ .Release.Name }}
+ chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
+ heritage: {{ .Release.Service }}
+ {{- if or .Values.labels .Values.otelCollector.labels }}
+ {{- mustMerge .Values.otelCollector.labels .Values.labels | toYaml |
nindent 4 }}
+ {{- end }}
+ {{- with .Values.otelCollector.annotations }}
Review Comment:
I think I thought/saw that this annotations are used in the configmap file
🤔, but as I see it now, it looks good, so no need for any change here.
Regarding `configMapAnnotations` for ConfigMap itself, we could add it, but I
don't see it as required now
##########
chart/values.schema.json:
##########
@@ -8611,6 +8634,254 @@
}
}
},
+ "otelCollector": {
+ "description": "OpenTelemetry Collector settings.",
+ "type": "object",
+ "x-docsSection": "OpenTelemetry",
+ "additionalProperties": false,
+ "properties": {
+ "tracesEnabled": {
+ "description": "Send Airflow traces to the OTel Collector.
Sets `[traces] otel_on` in the rendered Airflow config and deploys the
collector when either `tracesEnabled` or `metricsEnabled` is true.",
+ "type": "boolean",
+ "default": true
+ },
+ "metricsEnabled": {
+ "description": "Send Airflow metrics to the OTel
Collector. Sets `[metrics] otel_on` in the rendered Airflow config and forces
`[metrics] statsd_on` to False because Airflow can only export metrics to one
backend at a time.",
+ "type": "boolean",
+ "default": false
+ },
+ "metricExportIntervalMs": {
+ "description": "Interval (in milliseconds) at which the
OTel SDK exports metrics to the collector. Sets the
`OTEL_METRIC_EXPORT_INTERVAL` env var on Airflow pods.",
+ "type": "integer",
+ "default": 30000
Review Comment:
```suggestion
"default": 30000,
"minimum": 0
```
for better validation. We could also add a similar config to e.g.
`terminationGracePeriodSeconds` or probes configs
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
