From d7242450a4e8da9b975bb9aca38afb97c28bc253 Mon Sep 17 00:00:00 2001
From: odysseu <55583423+odysseu@users.noreply.github.com>
Date: Tue, 4 Jun 2024 14:55:35 +0200
Subject: [PATCH] Enable monitoring shinyproxy via prometheus (#67)

* added skip_existing flag, bumped to 1.6.0

* added servicemonitor

* Added specific service for prometheus's servicemonitor

* add code chunk langauges

* Shinyproxy monitor doc

* Update configmap.yaml

* more parameters for configuring monitoring

* only minor version is enough
---
 charts/shinyproxy/Chart.yaml                  | 10 +--
 charts/shinyproxy/README.md                   | 45 +++++++++--
 charts/shinyproxy/templates/configmap.yaml    |  9 +++
 .../shinyproxy/templates/service-metrics.yaml | 18 +++++
 .../shinyproxy/templates/servicemonitor.yaml  | 21 +++++
 charts/shinyproxy/values.yaml                 | 80 +++++++++++--------
 6 files changed, 137 insertions(+), 46 deletions(-)
 create mode 100644 charts/shinyproxy/templates/service-metrics.yaml
 create mode 100644 charts/shinyproxy/templates/servicemonitor.yaml

diff --git a/charts/shinyproxy/Chart.yaml b/charts/shinyproxy/Chart.yaml
index 5e22ad0..211ca58 100644
--- a/charts/shinyproxy/Chart.yaml
+++ b/charts/shinyproxy/Chart.yaml
@@ -15,15 +15,15 @@ type: application
 # This is the chart version. This version number should be incremented each time you make changes
 # to the chart and its templates, including the app version.
 # Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 2.0.0
+version: 2.1.0
 
 # This is the version number of the application being deployed. This version number should be
 # incremented each time you make changes to the application. Versions are not expected to
 # follow Semantic Versioning. They should reflect the version the application is using.
 # It is recommended to use it with quotes.
-appVersion: "3.0.2"
+appVersion: "3.1.0"
 
 dependencies:
-  - name: library-chart
-    version: 2.0.20
-    repository: https://inseefrlab.github.io/helm-charts-datascience
+- name: library-chart
+  version: 1.5.21
+  repository: https://inseefrlab.github.io/helm-charts-interactive-services
diff --git a/charts/shinyproxy/README.md b/charts/shinyproxy/README.md
index bd3734f..d23f997 100644
--- a/charts/shinyproxy/README.md
+++ b/charts/shinyproxy/README.md
@@ -3,9 +3,9 @@
 This helm chart allow to deploy a customised shinyproxy app.
 Security
 
-With or without oidc, if using oidc pay attention to deploy yourself a secret that contains:
+If using oidc for authentication, you must first apply a secret that contains:
 
-```
+```yaml
 apiVersion: v1
 kind: Secret
 metadata:
@@ -17,11 +17,11 @@ stringData:
 
 ## App configuration
 
-You can customized the application.yml file in the apps helm values.
+You can customize the application.yml file in the apps' helm values.
 
-If you want to add some env variable you cas use container-env if you want to protect secret you can deploy yourself a secret like this, let say you have 2 applications:
+If you want to add some env variable you can use container-env if you want to protect secret you can deploy yourself a secret like this, let say you have 2 applications:
 
-```
+```yaml
 apiVersion: v1
 kind: Secret
 metadata:
@@ -35,6 +35,37 @@ stringData:
     VAR_EXAMPLE_3=value3
 ```
 
-This file will be injected in /opt/shinyproxy/conf/app1 and app2, then your application.yml configuration can specify ```container-env-file: /opt/shinyproxy/app1```
+This file will be injected in `/opt/shinyproxy/conf/app1` and `/opt/shinyproxy/conf/app2`, then your application.yml configuration can specify ```container-env-file: /opt/shinyproxy/app1```
+
+container-env and container-env-file can be mixed.
+
+## Monitor shinyproxy activity using prometheus
+
+As explained in the official [shinyproxy's docker image repository](https://shinyproxy.io/documentation/usage-statistics/#micrometer-prometheus), we can set up the Micrometer monitoring backend using Prometheus to fetch shinyproxy activity. By default, is it turned off.
+
+If you want to enable metrics of sinyproxy, put `monitor.enabled` to `true`.
+
+Then you'll want to chose an interval at which the shinyproxy logs to http://localhost:9090/actuator/prometheus in its pod.
+
+Each metric produced by shinyproxy can have a set prefix.
+
+Prometheus will discover the metrics if the service of shinyproxy uses [a specific `metadata.labels.release`](https://github.com/prometheus-operator/prometheus-operator/blob/main/Documentation/troubleshooting.md#using-textual-port-number-instead-of-port-name), this depends on your implementation of prometheus; by default it is empty.
+
+
+Here is an example when monitoring is enabled on shinyproxy :
+
+```yaml
+monitor:
+  enabled: true
+  metadata:
+    labels:
+      release: "prometheus-stack"
+  port:
+    number: 9090
+    name: monitor
+  interval: 10s
+  micrometer:
+    prefix: "shinyproxy"
+```
 
-container-env and container-env-file can be mixed.
\ No newline at end of file
+The _servicemonitor.yaml_ needs to fetch infos of a specific service (port, labels, ...). Since the _service.yaml_ we have is already used by the _ingress.yaml_, we made a second one called _service-metrics.yaml_ for ths purpose.
diff --git a/charts/shinyproxy/templates/configmap.yaml b/charts/shinyproxy/templates/configmap.yaml
index 6338438..e7c61e6 100644
--- a/charts/shinyproxy/templates/configmap.yaml
+++ b/charts/shinyproxy/templates/configmap.yaml
@@ -6,6 +6,15 @@ metadata:
     {{- include "library-chart.labels" . | nindent 4 }}
 data:
   application.yml: |-
+    {{ if .Values.monitor.enabled }}
+    usage-stats-micrometer-prefix: {{ .Values.monitor.micrometer.prefix }}
+    usage-stats-url: micrometer
+    management:
+      prometheus:
+        metrics:
+          export:
+            enabled: true
+    {{ end }}
     proxy:
       landing-page: {{ .Values.proxyConfig.landingPage }}
       container-backend: kubernetes
diff --git a/charts/shinyproxy/templates/service-metrics.yaml b/charts/shinyproxy/templates/service-metrics.yaml
new file mode 100644
index 0000000..e12e6f4
--- /dev/null
+++ b/charts/shinyproxy/templates/service-metrics.yaml
@@ -0,0 +1,18 @@
+{{- if .Values.monitor.enabled -}}
+apiVersion: v1
+kind: Service
+metadata:
+  name: {{ include "library-chart.fullname" . }}-metrics
+  labels:
+    {{- include "library-chart.labels" . | nindent 4 }}
+    servicename: {{ include "library-chart.fullname" . }}-metrics
+spec:
+  type: {{ .Values.service.type }}
+  ports:
+    - name: {{ .Values.monitor.port.name }}
+      port: 9090
+      targetPort: {{ .Values.monitor.port.number }}
+      protocol: TCP
+  selector:
+    {{- include "library-chart.selectorLabels" . | nindent 4 }}
+{{- end }}
\ No newline at end of file
diff --git a/charts/shinyproxy/templates/servicemonitor.yaml b/charts/shinyproxy/templates/servicemonitor.yaml
new file mode 100644
index 0000000..eb7fc43
--- /dev/null
+++ b/charts/shinyproxy/templates/servicemonitor.yaml
@@ -0,0 +1,21 @@
+{{- if .Values.monitor.enabled -}}
+apiVersion: monitoring.coreos.com/v1
+kind: ServiceMonitor
+metadata:
+  name: {{ include "library-chart.fullname" . }}-monitor
+  namespace:  {{ .Release.Namespace }}
+  labels:
+    release: {{ .Values.monitor.metadata.labels.release }}
+    app.kubernetes.io/name: {{ include "library-chart.fullname" . }}
+spec:
+  endpoints:
+  - port: {{ .Values.monitor.port.name }}
+    interval: {{ .Values.monitor.interval }}
+    path: /actuator/prometheus
+  namespaceSelector:
+    matchNames:
+    - {{ .Release.Namespace }}
+  selector:
+    matchLabels:
+      servicename: {{ include "library-chart.fullname" . }}-metrics
+{{- end }}
\ No newline at end of file
diff --git a/charts/shinyproxy/values.yaml b/charts/shinyproxy/values.yaml
index 4a8fa33..c520070 100644
--- a/charts/shinyproxy/values.yaml
+++ b/charts/shinyproxy/values.yaml
@@ -1,31 +1,31 @@
 proxyDeploy:
   image:
     name: openanalytics/shinyproxy
-    tag: 3.0.2
+    tag: 3.1.0
     pullPolicy: Always
   replicaCount: 1
   resources: {}
-    # We usually recommend not to specify default resources and to leave this as a conscious
-    # choice for the user. This also increases chances charts run on environments with little
-    # resources, such as Minikube. If you do want to specify resources, uncomment the following
-    # lines, adjust them as necessary, and remove the curly braces after 'resources:'.
-    # limits:
-    #   cpu: 100m
-    #   memory: 128Mi
-    # requests:
-    #   cpu: 100m
-    #   memory: 128Mi
+  # We usually recommend not to specify default resources and to leave this as a conscious
+  # choice for the user. This also increases chances charts run on environments with little
+  # resources, such as Minikube. If you do want to specify resources, uncomment the following
+  # lines, adjust them as necessary, and remove the curly braces after 'resources:'.
+  # limits:
+  #   cpu: 100m
+  #   memory: 128Mi
+  # requests:
+  #   cpu: 100m
+  #   memory: 128Mi
   podAnnotations: {}
   podLabels: {}
   podSecurityContext: {}
-    # fsGroup: 2000
+  # fsGroup: 2000
   securityContext: {}
-    # capabilities:
-    #   drop:
-    #   - ALL
-    # readOnlyRootFilesystem: true
-    # runAsNonRoot: true
-    # runAsUser: 1000
+  # capabilities:
+  #   drop:
+  #   - ALL
+  # readOnlyRootFilesystem: true
+  # runAsNonRoot: true
+  # runAsUser: 1000
   nodeSelector: {}
   tolerations: []
   affinity: {}
@@ -39,22 +39,22 @@ proxyConfig:
   authentication:
     type: none
     # type: openid
-    # openid:
-    #   authURL: ""
-    #   tokenURL: ""
-    #   jwksURL: ""
-    #   logoutURL: ""
-    #   usernameAttribute: ""
-    #   rolesClaim: ""
-    #   existingClientCredentialsSecretName: ""
+# openid:
+#   authURL: ""
+#   tokenURL: ""
+#   jwksURL: ""
+#   logoutURL: ""
+#   usernameAttribute: ""
+#   rolesClaim: ""
+#   existingClientCredentialsSecretName: ""
 
 apps:
-  - id: ""
-    display-name: ""
-    description: ""
-    container-image: ""
-    container-cmd: []
-    access-groups: []
+- id: ""
+  display-name: ""
+  description: ""
+  container-image: ""
+  container-cmd: []
+  access-groups: []
 
 
 existingAppsSecretName:
@@ -69,8 +69,8 @@ ingress:
   hostname: chart-example.local
 
 service:
-    type: ClusterIP
-    port: 8080
+  type: ClusterIP
+  port: 8080
 
 serviceAccount:
   # Specifies whether a service account should be created
@@ -80,3 +80,15 @@ serviceAccount:
   # The name of the service account to use.
   # If not set and create is true, a name is generated using the fullname template
   name: ""
+
+monitor:
+  enabled: false
+  metadata:
+    labels:
+      release: ""
+  port:
+    name: ""
+    number: 9090
+  interval: 10s
+  micrometer:
+    prefix: ""