From a5a3ad630581aeeaee40cfd4e45c63199d0ff982 Mon Sep 17 00:00:00 2001 From: MiniDigger | Martin Date: Sat, 7 Jan 2023 15:17:01 +0100 Subject: [PATCH] feat(backend+chart): trace db calls and http requests using micrometer, opentelemetry and zipkin (closes #1072) --- backend/pom.xml | 14 +++ .../io/papermc/hangar/config/JDBIConfig.java | 16 --- .../hangar/config/ManagementConfig.java | 101 ++++++++++++++++++ .../io/papermc/hangar/config/WebConfig.java | 11 +- .../hangar/service/ValidationService.java | 2 +- backend/src/main/resources/application.yml | 14 ++- chart/templates/_helpers.tpl | 7 ++ chart/templates/deployment-zipkin.yaml | 74 +++++++++++++ chart/templates/ingress-zipkin.yaml | 44 ++++++++ chart/templates/secret-hangar-zipkin.yaml | 10 ++ chart/templates/service-zipkin.yaml | 18 ++++ chart/templates/serviceaccount-zipkin.yaml | 12 +++ chart/values.yaml | 80 ++++++++++++++ 13 files changed, 381 insertions(+), 22 deletions(-) create mode 100644 backend/src/main/java/io/papermc/hangar/config/ManagementConfig.java create mode 100644 chart/templates/deployment-zipkin.yaml create mode 100644 chart/templates/ingress-zipkin.yaml create mode 100644 chart/templates/secret-hangar-zipkin.yaml create mode 100644 chart/templates/service-zipkin.yaml create mode 100644 chart/templates/serviceaccount-zipkin.yaml diff --git a/backend/pom.xml b/backend/pom.xml index 73080bdee..8db30701d 100644 --- a/backend/pom.xml +++ b/backend/pom.xml @@ -123,6 +123,10 @@ org.springframework.boot spring-boot-starter-validation + + org.springframework.boot + spring-boot-starter-aop + org.apache.commons @@ -244,6 +248,16 @@ ${datafaker.version} + + + io.micrometer + micrometer-tracing-bridge-otel + + + io.opentelemetry + opentelemetry-exporter-zipkin + + org.springframework.boot diff --git a/backend/src/main/java/io/papermc/hangar/config/JDBIConfig.java b/backend/src/main/java/io/papermc/hangar/config/JDBIConfig.java index 9f6ca978d..460b69bf5 100644 --- a/backend/src/main/java/io/papermc/hangar/config/JDBIConfig.java +++ b/backend/src/main/java/io/papermc/hangar/config/JDBIConfig.java @@ -5,18 +5,14 @@ import io.papermc.hangar.db.customtypes.JSONB; import io.papermc.hangar.db.customtypes.JobState; import io.papermc.hangar.db.customtypes.PGLoggedAction; import io.papermc.hangar.db.customtypes.RoleCategory; -import java.sql.SQLException; import java.util.List; import java.util.UUID; -import java.util.logging.Logger; import javax.sql.DataSource; import org.jdbi.v3.core.Jdbi; import org.jdbi.v3.core.mapper.ColumnMapper; import org.jdbi.v3.core.mapper.RowMapper; import org.jdbi.v3.core.mapper.RowMapperFactory; import org.jdbi.v3.core.spi.JdbiPlugin; -import org.jdbi.v3.core.statement.SqlLogger; -import org.jdbi.v3.core.statement.StatementContext; import org.jdbi.v3.postgres.PostgresPlugin; import org.jdbi.v3.postgres.PostgresTypes; import org.jdbi.v3.sqlobject.SqlObjectPlugin; @@ -48,20 +44,8 @@ public class JDBIConfig { @Bean public Jdbi jdbi(final DataSource dataSource, final List jdbiPlugins, final List> rowMappers, final List rowMapperFactories, final List> columnMappers) { - final SqlLogger myLogger = new SqlLogger() { - @Override - public void logException(final StatementContext context, final SQLException ex) { - Logger.getLogger("sql").info("sql: " + context.getRenderedSql()); - } - - @Override - public void logAfterExecution(final StatementContext context) { - Logger.getLogger("sql").info("sql ae: " + context.getRenderedSql()); - } - }; final TransactionAwareDataSourceProxy dataSourceProxy = new TransactionAwareDataSourceProxy(dataSource); final Jdbi jdbi = Jdbi.create(dataSourceProxy); - // jdbi.setSqlLogger(myLogger); // for debugging sql statements final PostgresTypes config = jdbi.getConfig(PostgresTypes.class); jdbiPlugins.forEach(jdbi::installPlugin); diff --git a/backend/src/main/java/io/papermc/hangar/config/ManagementConfig.java b/backend/src/main/java/io/papermc/hangar/config/ManagementConfig.java new file mode 100644 index 000000000..3d1012903 --- /dev/null +++ b/backend/src/main/java/io/papermc/hangar/config/ManagementConfig.java @@ -0,0 +1,101 @@ +package io.papermc.hangar.config; + +import io.micrometer.observation.Observation; +import io.micrometer.observation.ObservationRegistry; +import io.micrometer.observation.aop.ObservedAspect; +import java.lang.reflect.Method; +import java.sql.SQLException; +import java.time.temporal.ChronoUnit; +import org.jdbi.v3.core.Jdbi; +import org.jdbi.v3.core.spi.JdbiPlugin; +import org.jdbi.v3.core.statement.SqlLogger; +import org.jdbi.v3.core.statement.StatementContext; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.boot.actuate.web.exchanges.InMemoryHttpExchangeRepository; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Lazy; + +import static io.micrometer.observation.Observation.createNotStarted; + +@Configuration(proxyBeanMethods = false) +public class ManagementConfig { + + private static final Logger sqlLog = LoggerFactory.getLogger("sql"); + + private final ObservationRegistry observationRegistry; + + private final boolean logSql = false; // for debugging sql statements + + public ManagementConfig(@Lazy final ObservationRegistry observationRegistry) { + this.observationRegistry = observationRegistry; + } + + @Bean + public InMemoryHttpExchangeRepository inMemoryHttpExchangeRepository() { + return new InMemoryHttpExchangeRepository(); + } + + @Bean + public ObservedAspect observedAspect() { + return new ObservedAspect(this.observationRegistry); + } + + @Bean + public JdbiPlugin observationPlugin() { + return new JdbiPlugin() { + @Override + public void customizeJdbi(final Jdbi jdbi) { + final SqlLogger myLogger = new SqlLogger() { + @Override + public void logBeforeExecution(final StatementContext context) { + if (ManagementConfig.this.logSql) { + sqlLog.info("sql be: " + context.getRenderedSql()); + } + + final Observation observation = createNotStarted(this.getObservationName(context), ManagementConfig.this.observationRegistry); + observation.start(); + observation.highCardinalityKeyValue("hangar.sql.rendered", context.getRenderedSql()); + observation.highCardinalityKeyValue("hangar.sql.binding", context.getBinding().toString()); + context.define("observation", observation); + } + + @Override + public void logAfterExecution(final StatementContext context) { + if (ManagementConfig.this.logSql) { + sqlLog.info("sql ae: " + context.getRenderedSql() + ", took " + context.getElapsedTime(ChronoUnit.MILLIS) + "ms"); + } + + final Object attr = context.getAttribute("observation"); + if (attr instanceof Observation observation) { + observation.stop(); + } else { + sqlLog.warn("No observation for " + this.getObservationName(context)); + } + } + + @Override + public void logException(final StatementContext context, final SQLException ex) { + if (ManagementConfig.this.logSql) { + sqlLog.info("sql e: " + context.getRenderedSql() + ", " + ex.getMessage()); + } + final Object attr = context.getAttribute("observation"); + if (attr instanceof Observation observation) { + observation.error(ex); + observation.stop(); + } else { + sqlLog.warn("No observation for " + this.getObservationName(context)); + } + } + + private String getObservationName(final StatementContext context) { + final Method method = context.getExtensionMethod().getMethod(); + return method.getDeclaringClass().getSimpleName() + "#" + method.getName(); + } + }; + jdbi.setSqlLogger(myLogger); + } + }; + } +} diff --git a/backend/src/main/java/io/papermc/hangar/config/WebConfig.java b/backend/src/main/java/io/papermc/hangar/config/WebConfig.java index 0df2663f8..4ff95a510 100644 --- a/backend/src/main/java/io/papermc/hangar/config/WebConfig.java +++ b/backend/src/main/java/io/papermc/hangar/config/WebConfig.java @@ -23,6 +23,7 @@ import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.web.client.RestTemplateBuilder; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.convert.converter.Converter; @@ -146,14 +147,16 @@ public class WebConfig extends WebMvcConfigurationSupport { } @Bean - public RestTemplate restTemplate(final List> messageConverters) { + public RestTemplate restTemplate(final List> messageConverters, final RestTemplateBuilder builder) { final RestTemplate restTemplate; if (interceptorLogger.isDebugEnabled()) { final ClientHttpRequestFactory factory = new BufferingClientHttpRequestFactory(new SimpleClientHttpRequestFactory()); - restTemplate = new RestTemplate(factory); - restTemplate.setInterceptors(List.of(new LoggingInterceptor())); + restTemplate = builder + .requestFactory(() -> factory) + .interceptors(new LoggingInterceptor()) + .build(); } else { - restTemplate = new RestTemplate(); + restTemplate = builder.build(); } this.addDefaultHttpMessageConverters(messageConverters); restTemplate.setMessageConverters(messageConverters); diff --git a/backend/src/main/java/io/papermc/hangar/service/ValidationService.java b/backend/src/main/java/io/papermc/hangar/service/ValidationService.java index 1922b583b..80ebf5bb9 100644 --- a/backend/src/main/java/io/papermc/hangar/service/ValidationService.java +++ b/backend/src/main/java/io/papermc/hangar/service/ValidationService.java @@ -11,7 +11,7 @@ import org.springframework.stereotype.Service; @Service public class ValidationService { - private static final Set BANNED_ROUTES = Set.of("api", "authors", "linkout", "logged-out", "new", "unread", "notifications", "staff", "admin", "organizations", "tools", "recommended", "null", "undefined", "privacy", "terms", "tos", "settings"); + private static final Set BANNED_ROUTES = Set.of("actuator", "admin", "api", "authors", "guidelines", "markdown","neworganization", "linkout", "logged-out", "new", "notifications", "null", "organizations", "privacy", "recommended", "settings", "staff", "terms", "tools", "tos", "undefined", "unread", "version"); private final HangarConfig config; public ValidationService(final HangarConfig config) { diff --git a/backend/src/main/resources/application.yml b/backend/src/main/resources/application.yml index 0fae4ad85..daf0c32e0 100644 --- a/backend/src/main/resources/application.yml +++ b/backend/src/main/resources/application.yml @@ -5,6 +5,8 @@ server: port: 8080 spring: + application: + name: "Hangar Backend" ############ # DataBase # ############ @@ -30,7 +32,6 @@ spring: WRITE_DATES_AS_TIMESTAMPS: false date-format: com.fasterxml.jackson.databind.util.StdDateFormat - cloud: aws: s3: @@ -42,6 +43,17 @@ spring: springdoc: pathsToMatch: "/api/v1/**" +management: + tracing: + sampling: + probability: 1.0 + endpoints: + enabled-by-default: true + web: + exposure: + include: "health" + # include: "*" # for local you can include all for more info + ############# # Fake User # ############# diff --git a/chart/templates/_helpers.tpl b/chart/templates/_helpers.tpl index 021c93c98..0580e9c33 100644 --- a/chart/templates/_helpers.tpl +++ b/chart/templates/_helpers.tpl @@ -69,3 +69,10 @@ Create the name of the service account to use {{- end }} {{- end }} +{{- define "hangar.zipkin.serviceAccountName" -}} +{{- if .Values.zipkin.serviceAccount.create }} +{{- default (printf "%s-zipkin" (include "hangar.fullname" .)) .Values.zipkin.serviceAccount.name }} +{{- else }} +{{- default "default" .Values.zipkin.serviceAccount.name }} +{{- end }} +{{- end }} diff --git a/chart/templates/deployment-zipkin.yaml b/chart/templates/deployment-zipkin.yaml new file mode 100644 index 000000000..44c220995 --- /dev/null +++ b/chart/templates/deployment-zipkin.yaml @@ -0,0 +1,74 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: {{ include "hangar.fullname" . }}-zipkin + labels: + {{- include "hangar.labels" . | nindent 4 }} +spec: + replicas: 1 + strategy: + type: RollingUpdate + rollingUpdate: + maxSurge: 1 + maxUnavailable: 1 + selector: + matchLabels: + {{- include "hangar.selectorLabels" . | nindent 6 }} + app.kubernetes.io/component: "zipkin" + template: + metadata: + annotations: + checksum/config: {{ include (print $.Template.BasePath "/secret-hangar-zipkin.yaml") . | sha256sum }} + {{- with .Values.backend.podAnnotations }} + {{- toYaml . | nindent 8 }} + {{- end }} + labels: + {{- include "hangar.selectorLabels" . | nindent 8 }} + app.kubernetes.io/component: "zipkin" + spec: + {{- with .Values.zipkin.imagePullSecrets }} + imagePullSecrets: + {{- toYaml . | nindent 8 }} + {{- end }} + serviceAccountName: {{ include "hangar.zipkin.serviceAccountName" . }} + securityContext: + {{- toYaml .Values.zipkin.podSecurityContext | nindent 8 }} + containers: + - name: {{ .Chart.Name }} + securityContext: + {{- toYaml .Values.zipkin.securityContext | nindent 12 }} + image: "{{ .Values.zipkin.image.repository }}:{{ .Values.zipkin.image.tag | default .Chart.AppVersion }}" + imagePullPolicy: {{ .Values.zipkin.image.pullPolicy }} + ports: + - name: http + containerPort: 9411 + protocol: TCP + livenessProbe: + httpGet: + path: /health + port: http + initialDelaySeconds: 10 + periodSeconds: 10 + readinessProbe: + httpGet: + path: /health + port: http + initialDelaySeconds: 5 + periodSeconds: 5 + envFrom: + - secretRef: + name: hangar-zipkin + resources: + {{- toYaml .Values.zipkin.resources | nindent 12 }} + {{- with .Values.zipkin.nodeSelector }} + nodeSelector: + {{- toYaml . | nindent 8 }} + {{- end }} + {{- with .Values.zipkin.affinity }} + affinity: + {{- toYaml . | nindent 8 }} + {{- end }} + {{- with .Values.zipkin.tolerations }} + tolerations: + {{- toYaml . | nindent 8 }} + {{- end }} diff --git a/chart/templates/ingress-zipkin.yaml b/chart/templates/ingress-zipkin.yaml new file mode 100644 index 000000000..4f0e2f309 --- /dev/null +++ b/chart/templates/ingress-zipkin.yaml @@ -0,0 +1,44 @@ +{{- if .Values.zipkin.ingress.enabled -}} +{{- $fullName := include "hangar.fullname" . -}} +{{- $svcPort := .Values.zipkin.service.port -}} +{{- if and .Values.zipkin.ingress.className (not (semverCompare ">=1.18-0" .Capabilities.KubeVersion.GitVersion)) }} + {{- if not (hasKey .Values.zipkin.ingress.annotations "kubernetes.io/ingress.class") }} + {{- $_ := set .Values.zipkin.ingress.annotations "kubernetes.io/ingress.class" .Values.zipkin.ingress.className}} + {{- end }} +{{- end }} +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: {{ $fullName }} + labels: + {{- include "hangar.labels" . | nindent 4 }} + {{- with .Values.zipkin.ingress.annotations }} + annotations: + {{- toYaml . | nindent 4 }} + {{- end }} +spec: + ingressClassName: {{ .Values.zipkin.ingress.className }} + {{- if .Values.zipkin.ingress.tls }} + tls: + {{- range .Values.zipkin.ingress.tls }} + - hosts: + {{- range .hosts }} + - {{ . | quote }} + {{- end }} + secretName: {{ .secretName }} + {{- end }} + {{- end }} + rules: + - host: {{ .Values.zipkin.ingress.host | quote }} + http: + paths: + {{- range .Values.zipkin.ingress.paths }} + - path: {{ .path }} + pathType: {{ .pathType }} + backend: + service: + name: {{ $fullName }}-zipkin + port: + number: {{ $svcPort }} + {{- end }} +{{- end }} diff --git a/chart/templates/secret-hangar-zipkin.yaml b/chart/templates/secret-hangar-zipkin.yaml new file mode 100644 index 000000000..f8055504d --- /dev/null +++ b/chart/templates/secret-hangar-zipkin.yaml @@ -0,0 +1,10 @@ +apiVersion: v1 +kind: Secret +metadata: + name: hangar-zipkin + labels: + {{- include "hangar.labels" . | nindent 4 }} +type: Opaque +stringData: + TEST: "{{ .Values.zipkin.config.test }}" + STORAGE_TYPE: mem diff --git a/chart/templates/service-zipkin.yaml b/chart/templates/service-zipkin.yaml new file mode 100644 index 000000000..cf0bc7cea --- /dev/null +++ b/chart/templates/service-zipkin.yaml @@ -0,0 +1,18 @@ +apiVersion: v1 +kind: Service +metadata: + name: {{ include "hangar.fullname" . }}-zipkin + labels: + {{- include "hangar.labels" . | nindent 4 }} + annotations: + service.kubernetes.io/topology-aware-hints: "auto" +spec: + type: {{ .Values.zipkin.service.type }} + ports: + - port: {{ .Values.zipkin.service.port }} + targetPort: http + protocol: TCP + name: http + selector: + {{- include "hangar.selectorLabels" . | nindent 4 }} + app.kubernetes.io/component: "zipkin" diff --git a/chart/templates/serviceaccount-zipkin.yaml b/chart/templates/serviceaccount-zipkin.yaml new file mode 100644 index 000000000..18ad1e311 --- /dev/null +++ b/chart/templates/serviceaccount-zipkin.yaml @@ -0,0 +1,12 @@ +{{- if .Values.zipkin.serviceAccount.create -}} +apiVersion: v1 +kind: ServiceAccount +metadata: + name: {{ include "hangar.zipkin.serviceAccountName" . }} + labels: + {{- include "hangar.labels" . | nindent 4 }} + {{- with .Values.zipkin.serviceAccount.annotations }} + annotations: + {{- toYaml . | nindent 4 }} + {{- end }} +{{- end }} diff --git a/chart/values.yaml b/chart/values.yaml index e210a2727..69ebcfdf0 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -211,3 +211,83 @@ backend: cdnEndpoint: "" cdnIncludeBucket: true announcement: "This is a staging server for testing purposes. Data could be deleted at any time. That said, signups are open, please test stuff and report and feedback on github or discord!" + +zipkin: + image: + repository: ghcr.io/openzipkin/zipkin + pullPolicy: Always + # Overrides the image tag whose default is the chart appVersion. + tag: "latest" + + imagePullSecrets: [] + nameOverride: "" + fullnameOverride: "" + + ingress: + enabled: false + className: "" + annotations: { } + # kubernetes.io/ingress.class: nginx + # kubernetes.io/tls-acme: "true" + host: hangar.test + tls: + - secretName: hangar-tls + hosts: + - hangar.test + paths: + - path: /zipkin + pathType: ImplementationSpecific + + serviceAccount: + # Specifies whether a service account should be created + create: true + # Annotations to add to the service account + annotations: {} + # The name of the service account to use. + # If not set and create is true, a name is generated using the fullname template + name: "" + + podAnnotations: {} + + podSecurityContext: + fsGroup: 1000 + runAsNonRoot: true + runAsUser: 1000 + + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL + + service: + type: ClusterIP + port: 9411 + + 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 + + autoscaling: + enabled: false + minReplicas: 1 + maxReplicas: 100 + targetCPUUtilizationPercentage: 80 + # targetMemoryUtilizationPercentage: 80 + + nodeSelector: {} + + tolerations: [] + + affinity: {} + + config: + test: "TEST"