Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "feat(fe2): proper health probe endpoint - /api/status - [WBX-287]" #2091

Merged
merged 1 commit into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 0 additions & 28 deletions packages/frontend-2/lib/core/helpers/redis.ts

This file was deleted.

2 changes: 1 addition & 1 deletion packages/frontend-2/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@
"tailwindcss": "^3.4.1",
"type-fest": "^3.5.1",
"typescript": "^4.8.3",
"vue-tsc": "1.8.27",
"vue-tsc": "1.8.22",
"wait-on": "^6.0.1"
},
"engines": {
Expand Down
9 changes: 3 additions & 6 deletions packages/frontend-2/plugins/002-rum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { useCreateErrorLoggingTransport } from '~/lib/core/composables/error'
type PluginNuxtApp = Parameters<Plugin>[0]

async function initRumClient(app: PluginNuxtApp) {
const { enabled, keys, speckleServerVersion, baseUrl } = resolveInitParams()
const { enabled, keys, speckleServerVersion } = resolveInitParams()
const logger = useLogger()
const onAuthStateChange = useOnAuthStateChange()
const router = useRouter()
Expand All @@ -20,7 +20,6 @@ async function initRumClient(app: PluginNuxtApp) {
rg4js('enablePulse', true)
rg4js('boot')
rg4js('enableRum', true)
rg4js('withTags', [`baseUrl:${baseUrl}`, `version:${speckleServerVersion}`])

await onAuthStateChange(
(user, { resolveDistinctId }) => {
Expand Down Expand Up @@ -185,8 +184,7 @@ function resolveInitParams() {
logrocketAppId,
speckleServerVersion,
speedcurveId,
debugbearId,
baseUrl
debugbearId
}
} = useRuntimeConfig()
const raygun = raygunKey?.length ? raygunKey : null
Expand All @@ -203,8 +201,7 @@ function resolveInitParams() {
speedcurve,
debugbear
},
speckleServerVersion,
baseUrl
speckleServerVersion
}
}

Expand Down
32 changes: 21 additions & 11 deletions packages/frontend-2/plugins/004-redis.server.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Redis } from 'ioredis'
import { createRedis } from '~/lib/core/helpers/redis'

/**
* Re-using the same client for all SSR reqs (shouldn't be a problem)
Expand All @@ -10,20 +9,31 @@ let redis: InstanceType<typeof Redis> | undefined = undefined
* Provide redis (only in SSR)
*/
export default defineNuxtPlugin(async () => {
const { redisUrl } = useRuntimeConfig()
const logger = useLogger()

try {
const hasValidStatus =
redis && ['ready', 'connecting', 'reconnecting'].includes(redis.status)
if (!redis || !hasValidStatus) {
if (redis) {
await redis.quit()
}
if (redisUrl?.length) {
try {
const hasValidStatus =
redis && ['ready', 'connecting', 'reconnecting'].includes(redis.status)
if (!redis || !hasValidStatus) {
if (redis) {
await redis.quit()
}

redis = new Redis(redisUrl)

redis = await createRedis({ logger })
redis.on('error', (err) => {
logger.error(err, 'Redis error')
})

redis.on('end', () => {
logger.info('Redis disconnected from server')
})
}
} catch (e) {
logger.error(e, 'Redis setup failure')
}
} catch (e) {
logger.error(e, 'Redis setup failure')
}

const isValid = redis && redis.status === 'ready'
Expand Down
27 changes: 4 additions & 23 deletions packages/frontend-2/server/api/status.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,6 @@
import { ensureError } from '@speckle/shared'
import { createRedis } from '~/lib/core/helpers/redis'
import { useRequestId } from '~/lib/core/composables/server'

/**
* Check that the deployment is fine
*/

export default defineEventHandler(async () => {
let redisConnected = false

// Check that redis works
try {
const redis = await createRedis({ logger: useLogger() })
redisConnected = !!redis
} catch (e) {
const errMsg = ensureError(e).message
throw createError({
statusCode: 500,
fatal: true,
message: `Redis connection failed: ${errMsg}`
})
}

return { status: 'ok', redisConnected }
export default defineEventHandler((event) => {
const reqId = useRequestId({ event })
return { status: 'ok', reqId }
})
10 changes: 1 addition & 9 deletions packages/frontend-2/server/lib/core/helpers/observability.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { Observability } from '@speckle/shared'
import type { IncomingMessage } from 'node:http'
import { get } from 'lodash-es'
import type { Logger } from 'pino'
import type express from 'express'

const redactedReqHeaders = ['authorization', 'cookie']

Expand Down Expand Up @@ -45,7 +44,7 @@ export function serializeRequest(req: IncomingMessage) {
return {
id: req.id,
method: req.method,
path: getRequestPath(req),
path: req.url?.split('?')[0], // Remove query params which might be sensitive
// Allowlist useful headers
headers: Object.keys(req.headers).reduce((obj, key) => {
let valueToPrint = req.headers[key]
Expand All @@ -59,10 +58,3 @@ export function serializeRequest(req: IncomingMessage) {
}, {})
}
}

export const getRequestPath = (req: IncomingMessage | express.Request) => {
const path = ((get(req, 'originalUrl') || get(req, 'url') || '') as string).split(
'?'
)[0] as string
return path?.length ? path : null
}
23 changes: 11 additions & 12 deletions packages/frontend-2/server/middleware/001-logging.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Observability } from '@speckle/shared'
import { defineEventHandler, fromNodeMiddleware } from 'h3'
import { IncomingMessage, ServerResponse } from 'http'
import pino from 'pino'
Expand All @@ -8,10 +9,7 @@ import { randomUUID } from 'crypto'
import type { IncomingHttpHeaders } from 'http'
import { REQUEST_ID_HEADER } from '~~/server/lib/core/helpers/constants'
import { get } from 'lodash'
import {
serializeRequest,
getRequestPath
} from '~/server/lib/core/helpers/observability'
import { serializeRequest } from '~/server/lib/core/helpers/observability'

/**
* Server request logger
Expand All @@ -30,7 +28,10 @@ function determineRequestId(
const generateReqId: GenReqId = (req: IncomingMessage) =>
determineRequestId(req.headers)

const logger = useLogger()
const logger = Observability.getLogger(
useRuntimeConfig().public.logLevel,
useRuntimeConfig().public.logPretty
)

export const LoggingMiddleware = pinoHttp({
logger,
Expand All @@ -45,9 +46,8 @@ export const LoggingMiddleware = pinoHttp({
error: Error | undefined
) => {
// Mark some lower importance/spammy endpoints w/ 'debug' to reduce noise
const path = getRequestPath(req)
const shouldBeDebug =
['/metrics', '/health', '/api/status'].includes(path || '') ?? false
const path = req.url?.split('?')[0]
const shouldBeDebug = ['/metrics', '/health'].includes(path || '') ?? false

if (res.statusCode >= 400 && res.statusCode < 500) {
return 'info'
Expand All @@ -66,7 +66,7 @@ export const LoggingMiddleware = pinoHttp({
customSuccessObject(req, res, val: Record<string, unknown>) {
const isCompleted = !req.readableAborted && res.writableEnded
const requestStatus = isCompleted ? 'completed' : 'aborted'
const requestPath = getRequestPath(req) || 'unknown'
const requestPath = req.url?.split('?')[0] || 'unknown'
const appBindings = res.vueLoggerBindings || {}

return {
Expand All @@ -82,7 +82,7 @@ export const LoggingMiddleware = pinoHttp({
},
customErrorObject(req, res, err, val: Record<string, unknown>) {
const requestStatus = 'failed'
const requestPath = getRequestPath(req) || 'unknown'
const requestPath = req.url?.split('?')[0] || 'unknown'
const appBindings = res.vueLoggerBindings || {}

return {
Expand All @@ -107,10 +107,9 @@ export const LoggingMiddleware = pinoHttp({
const realRaw = get(res, 'raw.raw') as typeof res.raw
const isRequestCompleted = !!realRaw.writableEnded
const isRequestAborted = !isRequestCompleted
const statusCode = res.statusCode || res.raw.statusCode || realRaw.statusCode

return {
statusCode,
statusCode: res.raw.statusCode,
// Allowlist useful headers
headers: resRaw.headers,
isRequestAborted
Expand Down
6 changes: 0 additions & 6 deletions packages/frontend-2/server/tsconfig.json

This file was deleted.

29 changes: 0 additions & 29 deletions packages/frontend-2/server/utils/logger.ts

This file was deleted.

3 changes: 1 addition & 2 deletions packages/server/logging/expressLogging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,9 @@ export const LoggingExpressMiddleware = HttpLogger({
}
const serverRes = get(res, 'raw.raw') as ServerResponse
const auth = serverRes.req.context
const statusCode = res.statusCode || res.raw.statusCode || serverRes.statusCode

return {
statusCode,
statusCode: res.raw.statusCode,
// Allowlist useful headers
headers: Object.fromEntries(
Object.entries(resRaw.raw.headers).filter(
Expand Down
4 changes: 2 additions & 2 deletions utils/helm/speckle-server/templates/frontend_2/deployment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ spec:

livenessProbe:
httpGet:
path: /api/status
path: /health
port: www
failureThreshold: 3
initialDelaySeconds: 10
Expand All @@ -53,7 +53,7 @@ spec:
timeoutSeconds: 5
readinessProbe:
httpGet:
path: /api/status
path: /health
port: www
failureThreshold: 1
initialDelaySeconds: 5
Expand Down
13 changes: 0 additions & 13 deletions utils/helm/speckle-server/templates/redirect.ingress.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,4 @@ spec:
name: speckle-frontend
port:
name: www
{{- end }}
- pathType: Exact
path: "/api/status"
backend:
service:
{{- if .Values.frontend_2.enabled }}
name: speckle-frontend-2
port:
name: web
{{- else }}
name: speckle-frontend
port:
name: www
{{- end }}
Loading