From ac9154ed55f214aca8b63b26ea3b36198565a8ea Mon Sep 17 00:00:00 2001 From: Ravi Kumar L Date: Tue, 23 Jun 2026 01:07:55 +0200 Subject: [PATCH] fix(langfuse): harden tenant fanout config --- helm/librechat/templates/_helpers.tpl | 12 +++ helm/librechat/templates/configmap-env.yaml | 1 + .../templates/langfuse-fanout-configmap.yaml | 4 + .../templates/langfuse-fanout-deployment.yaml | 1 + .../tests/langfuse_fanout_selector_test.sh | 19 ++++- helm/librechat/values.yaml | 2 + .../__tests__/run-summarization.test.ts | 53 ++++++++++++ packages/api/src/langfuse/config.ts | 10 +-- packages/api/src/langfuse/destinations.ts | 4 +- packages/api/src/langfuse/feedback.spec.ts | 83 +++++++++++++++++++ .../api/src/langfuse/tenantDestinations.ts | 3 +- packages/api/src/langfuse/utils.ts | 26 ++++-- 12 files changed, 200 insertions(+), 18 deletions(-) diff --git a/helm/librechat/templates/_helpers.tpl b/helm/librechat/templates/_helpers.tpl index 561ced36cd..9cf16894f1 100755 --- a/helm/librechat/templates/_helpers.tpl +++ b/helm/librechat/templates/_helpers.tpl @@ -74,6 +74,18 @@ Langfuse fanout collector labels. {{- include "librechat.standardLabels" (dict "root" . "selectorLabels" (include "librechat.langfuseFanout.selectorLabels" .)) }} {{- end }} +{{/* +Validate Langfuse fanout destination keys. LibreChat normalizes destination +keys to lowercase before putting them on trace attributes, so Helm values must +already use the same lowercase key shape for collector routing to match. +*/}} +{{- define "librechat.langfuseFanout.validateDestinationKey" -}} +{{- $name := printf "%v" . -}} +{{- if not (regexMatch "^[a-z][a-z0-9_-]*$" $name) -}} +{{- fail (printf "langfuseFanout.tenant.destinations key %q is invalid; use lowercase keys matching ^[a-z][a-z0-9_-]*$ so LibreChat trace attributes match collector routes" $name) -}} +{{- end -}} +{{- end }} + {{/* RAG Selector labels */}} diff --git a/helm/librechat/templates/configmap-env.yaml b/helm/librechat/templates/configmap-env.yaml index 5f97c644f5..12742d69ea 100755 --- a/helm/librechat/templates/configmap-env.yaml +++ b/helm/librechat/templates/configmap-env.yaml @@ -33,6 +33,7 @@ data: {{- if and .Values.langfuseFanout.enabled (not (hasKey $configEnv "LANGFUSE_FANOUT_TENANT_DESTINATIONS")) }} {{- $tenantDestinations := list }} {{- range $name, $destination := .Values.langfuseFanout.tenant.destinations }} + {{- include "librechat.langfuseFanout.validateDestinationKey" $name }} {{- $tenantDestinations = append $tenantDestinations (printf "%s=%s" $name $destination.baseUrl) }} {{- end }} LANGFUSE_FANOUT_TENANT_DESTINATIONS: {{ join "," $tenantDestinations | quote }} diff --git a/helm/librechat/templates/langfuse-fanout-configmap.yaml b/helm/librechat/templates/langfuse-fanout-configmap.yaml index d8ff5c7777..bba55a42ea 100644 --- a/helm/librechat/templates/langfuse-fanout-configmap.yaml +++ b/helm/librechat/templates/langfuse-fanout-configmap.yaml @@ -27,6 +27,7 @@ data: error_mode: ignore table: {{- range $name, $_destination := .Values.langfuseFanout.tenant.destinations }} + {{- include "librechat.langfuseFanout.validateDestinationKey" $name }} - context: span condition: attributes["librechat.langfuse.destination"] == {{ $name | quote }} pipelines: [traces/tenant_{{ $name }}] @@ -52,6 +53,7 @@ data: timeout: ${env:LANGFUSE_FANOUT_BATCH_TIMEOUT} send_batch_size: ${env:LANGFUSE_FANOUT_BATCH_SEND_SIZE} {{- range $name, $_destination := .Values.langfuseFanout.tenant.destinations }} + {{- include "librechat.langfuseFanout.validateDestinationKey" $name }} batch/by_auth_{{ $name }}: timeout: ${env:LANGFUSE_FANOUT_BATCH_TIMEOUT} send_batch_size: ${env:LANGFUSE_FANOUT_BATCH_SEND_SIZE} @@ -68,6 +70,7 @@ data: Authorization: "${env:LANGFUSE_FANOUT_CENTRAL_AUTH_HEADER}" x-langfuse-ingestion-version: "4" {{- range $name, $_destination := .Values.langfuseFanout.tenant.destinations }} + {{- include "librechat.langfuseFanout.validateDestinationKey" $name }} otlphttp/tenant_{{ $name }}: endpoint: "${env:LANGFUSE_FANOUT_TENANT_{{ $name | upper | replace "-" "_" }}_BASE_URL}/api/public/otel" auth: @@ -88,6 +91,7 @@ data: processors: [memory_limiter, filter/tenant_export] exporters: [routing/langfuse_tenant_destination] {{- range $name, $_destination := .Values.langfuseFanout.tenant.destinations }} + {{- include "librechat.langfuseFanout.validateDestinationKey" $name }} traces/tenant_{{ $name }}: receivers: [routing/langfuse_tenant_destination] processors: [attributes/drop_librechat_routing, batch/by_auth_{{ $name }}] diff --git a/helm/librechat/templates/langfuse-fanout-deployment.yaml b/helm/librechat/templates/langfuse-fanout-deployment.yaml index 8ac8f04272..84c360f4bb 100644 --- a/helm/librechat/templates/langfuse-fanout-deployment.yaml +++ b/helm/librechat/templates/langfuse-fanout-deployment.yaml @@ -41,6 +41,7 @@ spec: name: {{ required "langfuseFanout.central.authHeaderSecret.name is required when langfuseFanout.enabled=true" .Values.langfuseFanout.central.authHeaderSecret.name | quote }} key: {{ .Values.langfuseFanout.central.authHeaderSecret.key | quote }} {{- range $name, $destination := .Values.langfuseFanout.tenant.destinations }} + {{- include "librechat.langfuseFanout.validateDestinationKey" $name }} - name: LANGFUSE_FANOUT_TENANT_{{ $name | upper | replace "-" "_" }}_BASE_URL value: {{ $destination.baseUrl | quote }} {{- end }} diff --git a/helm/librechat/tests/langfuse_fanout_selector_test.sh b/helm/librechat/tests/langfuse_fanout_selector_test.sh index 63af2b29f3..fc3987373f 100755 --- a/helm/librechat/tests/langfuse_fanout_selector_test.sh +++ b/helm/librechat/tests/langfuse_fanout_selector_test.sh @@ -11,7 +11,8 @@ CHART_DIR="$(cd "${SCRIPT_DIR}/.." && pwd)" REPO_ROOT="$(cd "${CHART_DIR}/../.." && pwd)" RENDER_CHART_DIR="$(mktemp -d -t librechat-fanout-chart.XXXXXX)" RENDERED_FILE="$(mktemp -t librechat-fanout-render.XXXXXX)" -trap 'rm -rf "${RENDER_CHART_DIR}"; rm -f "${RENDERED_FILE}"' EXIT +INVALID_RENDER_ERROR="$(mktemp -t librechat-fanout-invalid-key.XXXXXX)" +trap 'rm -rf "${RENDER_CHART_DIR}"; rm -f "${RENDERED_FILE}" "${INVALID_RENDER_ERROR}"' EXIT if ! command -v helm >/dev/null 2>&1; then echo "FAIL: helm not on PATH" >&2 @@ -39,6 +40,22 @@ helm template librechat "${RENDER_CHART_DIR}" \ --show-only templates/langfuse-fanout-configmap.yaml \ > "${RENDERED_FILE}" +if helm template librechat "${RENDER_CHART_DIR}" \ + --set langfuseFanout.enabled=true \ + --set langfuseFanout.central.authHeaderSecret.name=langfuse-central \ + --set langfuseFanout.tenant.destinations.EU.baseUrl=https://cloud.langfuse.com \ + --show-only templates/langfuse-fanout-configmap.yaml \ + > /dev/null 2> "${INVALID_RENDER_ERROR}"; then + echo "FAIL: Helm accepted invalid uppercase Langfuse fanout destination key" >&2 + exit 1 +fi + +if ! grep -q 'langfuseFanout.tenant.destinations key "EU" is invalid' "${INVALID_RENDER_ERROR}"; then + echo "FAIL: invalid destination key render did not explain the key contract" >&2 + cat "${INVALID_RENDER_ERROR}" >&2 + exit 1 +fi + if ! command -v node >/dev/null 2>&1; then echo "FAIL: node not on PATH" >&2 exit 1 diff --git a/helm/librechat/values.yaml b/helm/librechat/values.yaml index f4e59f3660..88e7150a61 100755 --- a/helm/librechat/values.yaml +++ b/helm/librechat/values.yaml @@ -300,6 +300,8 @@ langfuseFanout: name: "" key: LANGFUSE_FANOUT_CENTRAL_AUTH_HEADER tenant: + # Destination map keys are emitted by LibreChat as trace attributes and + # matched by the collector. Use lowercase keys matching ^[a-z][a-z0-9_-]*$. destinations: eu: baseUrl: https://cloud.langfuse.com diff --git a/packages/api/src/agents/__tests__/run-summarization.test.ts b/packages/api/src/agents/__tests__/run-summarization.test.ts index fcb780b91d..e251bfdf8a 100644 --- a/packages/api/src/agents/__tests__/run-summarization.test.ts +++ b/packages/api/src/agents/__tests__/run-summarization.test.ts @@ -210,6 +210,7 @@ beforeEach(() => { delete process.env.LANGFUSE_HOST; delete process.env.LANGFUSE_FANOUT_ENABLED; delete process.env.LANGFUSE_FANOUT_COLLECTOR_URL; + delete process.env.LANGFUSE_FANOUT_TENANT_BASE_URL; delete process.env.LANGFUSE_FANOUT_TENANT_DESTINATIONS; delete process.env.LANGFUSE_FANOUT_TENANT_EXPORT_DISABLED; }); @@ -1243,6 +1244,7 @@ describe('Langfuse run config', () => { process.env.LANGFUSE_BASE_URL = 'https://central.langfuse.example'; process.env.LANGFUSE_FANOUT_ENABLED = 'true'; process.env.LANGFUSE_FANOUT_COLLECTOR_URL = 'http://collector-from-env:4318'; + process.env.LANGFUSE_FANOUT_TENANT_BASE_URL = 'https://cloud.langfuse.com'; const callArgs = await callAndCaptureRunConfig({ tenantId: 'tenant-1', @@ -1597,6 +1599,37 @@ describe('Langfuse run config', () => { }); }); + it('uses central env Langfuse config when tenant fanout.enabled is the string false', async () => { + process.env.LANGFUSE_PUBLIC_KEY = 'pk-central'; + process.env.LANGFUSE_SECRET_KEY = 'sk-central'; + process.env.LANGFUSE_BASE_URL = 'https://central.langfuse.example'; + process.env.LANGFUSE_FANOUT_ENABLED = 'true'; + process.env.LANGFUSE_FANOUT_COLLECTOR_URL = 'http://collector-from-env:4318'; + + const callArgs = await callAndCaptureRunConfig({ + tenantId: 'tenant-1', + appConfig: { + langfuse: { + publicKey: 'pk-tenant-1', + secretKey: 'sk-tenant-1', + baseUrl: 'https://cloud.langfuse.com', + fanout: { + enabled: 'false', + }, + }, + } as unknown as AppConfig, + }); + + expect(callArgs.langfuse).toEqual({ + deterministicTraceId: true, + publicKey: 'pk-central', + secretKey: 'sk-central', + baseUrl: 'https://central.langfuse.example', + metadata: { 'librechat.tenant.id': 'tenant-1' }, + tags: ['tenant:tenant-1'], + }); + }); + it('honors tenant Langfuse enabled=false as a tracing opt-out', async () => { const callArgs = await callAndCaptureRunConfig({ tenantId: 'tenant-1', @@ -1616,6 +1649,26 @@ describe('Langfuse run config', () => { tags: ['tenant:tenant-1'], }); }); + + it('honors tenant Langfuse enabled as the string false', async () => { + const callArgs = await callAndCaptureRunConfig({ + tenantId: 'tenant-1', + appConfig: { + langfuse: { + enabled: 'false', + publicKey: 'pk-tenant-1', + secretKey: 'sk-tenant-1', + }, + } as unknown as AppConfig, + }); + + expect(callArgs.langfuse).toEqual({ + deterministicTraceId: true, + enabled: false, + metadata: { 'librechat.tenant.id': 'tenant-1' }, + tags: ['tenant:tenant-1'], + }); + }); }); // --------------------------------------------------------------------------- diff --git a/packages/api/src/langfuse/config.ts b/packages/api/src/langfuse/config.ts index 5f4fdb700d..efa64b0c7b 100644 --- a/packages/api/src/langfuse/config.ts +++ b/packages/api/src/langfuse/config.ts @@ -1,8 +1,8 @@ import type { AppConfig } from '@librechat/data-schemas'; import type { RunConfig } from '@librechat/agents'; import { resolveLangfuseTenantDestination } from './tenantDestinations'; +import { isTrueEnv, normalizeBoolean } from './utils'; import { normalizeString } from '~/utils/text'; -import { isTrueEnv } from './utils'; type LangfuseRunConfig = NonNullable; type LangfuseAppConfig = NonNullable; @@ -21,10 +21,8 @@ export function isLangfuseTenantExportEnabled(): boolean { } export function isLangfuseFanoutEnabled(fanout?: LangfuseFanoutConfig): boolean { - return ( - fanout?.enabled !== false && - (fanout?.enabled === true || isTrueEnv(process.env.LANGFUSE_FANOUT_ENABLED)) - ); + const enabled = normalizeBoolean(fanout?.enabled); + return enabled !== false && (enabled === true || isTrueEnv(process.env.LANGFUSE_FANOUT_ENABLED)); } function mergeTraceMetadata( @@ -83,7 +81,7 @@ export function buildLangfuseConfig({ langfuse.tags = tags; } - if (config?.enabled === false) { + if (normalizeBoolean(config?.enabled) === false) { return { ...langfuse, enabled: false, diff --git a/packages/api/src/langfuse/destinations.ts b/packages/api/src/langfuse/destinations.ts index c4ea0155c8..0b0bc4dd45 100644 --- a/packages/api/src/langfuse/destinations.ts +++ b/packages/api/src/langfuse/destinations.ts @@ -1,8 +1,8 @@ import type { AppConfig } from '@librechat/data-schemas'; import type { LangfuseFanoutConfig } from './config'; import { isLangfuseFanoutEnabled, isLangfuseTenantExportEnabled } from './config'; +import { isFalseEnv, normalizeBoolean, toBasicAuthorization } from './utils'; import { resolveLangfuseTenantDestination } from './tenantDestinations'; -import { isFalseEnv, toBasicAuthorization } from './utils'; import { normalizeString } from '~/utils/text'; const DEFAULT_BASE_URL = 'https://cloud.langfuse.com'; @@ -67,7 +67,7 @@ function getTenantScoreDestination(appConfig?: AppConfig): LangfuseScoreDestinat } const config = appConfig?.langfuse; - if (config?.enabled === false) { + if (normalizeBoolean(config?.enabled) === false) { return undefined; } const fanout = config?.fanout as LangfuseFanoutConfig | undefined; diff --git a/packages/api/src/langfuse/feedback.spec.ts b/packages/api/src/langfuse/feedback.spec.ts index 71dd410af6..75278d38df 100644 --- a/packages/api/src/langfuse/feedback.spec.ts +++ b/packages/api/src/langfuse/feedback.spec.ts @@ -165,6 +165,7 @@ describe('Langfuse feedback scores', () => { langfuse: { publicKey: 'tenant-public-key', secretKey: 'tenant-secret-key', + baseUrl: 'http://tenant-langfuse:3000', }, } as AppConfig, }); @@ -204,6 +205,31 @@ describe('Langfuse feedback scores', () => { }); }); + it('skips tenant feedback scores when tenant keys are configured without a tenant base URL', async () => { + enableTenantFanout(); + process.env.LANGFUSE_BASE_URL = 'http://central-langfuse:3000'; + process.env.LANGFUSE_FANOUT_TENANT_BASE_URL = 'http://tenant-langfuse:3000'; + const { sendFeedbackScore } = await loadFeedback(); + + await sendFeedbackScore({ + traceId: 'trace-id', + feedback: { rating: 'thumbsUp' }, + appConfig: appConfigWithLangfuse({ + publicKey: 'tenant-public-key', + secretKey: 'tenant-secret-key', + }), + }); + + expect(getFetchMock()).toHaveBeenCalledTimes(1); + expect(getFetchMock()).toHaveBeenCalledWith( + 'http://central-langfuse:3000/api/public/scores', + expect.objectContaining({ + method: 'POST', + headers: expect.objectContaining({ Authorization: getCentralAuthorization() }), + }), + ); + }); + it('posts tenant feedback scores to the configured destination for the tenant base URL', async () => { enableTenantFanout(); process.env.LANGFUSE_BASE_URL = 'http://central-langfuse:3000'; @@ -269,6 +295,7 @@ describe('Langfuse feedback scores', () => { langfuse: { publicKey: 'tenant-public-key', secretKey: 'tenant-secret-key', + baseUrl: 'http://tenant-langfuse:3000', }, } as AppConfig, }); @@ -307,6 +334,7 @@ describe('Langfuse feedback scores', () => { appConfig: appConfigWithLangfuse({ publicKey: 'tenant-public-key', secretKey: 'tenant-secret-key', + baseUrl: 'http://tenant-langfuse:3000', }), }); @@ -345,6 +373,32 @@ describe('Langfuse feedback scores', () => { ); }); + it('skips tenant scores when tenant Langfuse enabled is the string false', async () => { + enableTenantFanout(); + process.env.LANGFUSE_BASE_URL = 'http://central-langfuse:3000'; + const { sendFeedbackScore } = await loadFeedback(); + + await sendFeedbackScore({ + traceId: 'trace-id', + feedback: { rating: 'thumbsUp' }, + appConfig: appConfigWithLangfuse({ + enabled: 'false', + publicKey: 'tenant-public-key', + secretKey: 'tenant-secret-key', + baseUrl: 'https://cloud.langfuse.com', + } as unknown as AppConfig['langfuse']), + }); + + expect(getFetchMock()).toHaveBeenCalledTimes(1); + expect(getFetchMock()).toHaveBeenCalledWith( + 'http://central-langfuse:3000/api/public/scores', + expect.objectContaining({ + method: 'POST', + headers: expect.objectContaining({ Authorization: getCentralAuthorization() }), + }), + ); + }); + it('skips tenant scores when tenant fanout export is disabled but keeps central scores', async () => { enableTenantFanout(); process.env.LANGFUSE_BASE_URL = 'http://central-langfuse:3000'; @@ -512,6 +566,32 @@ describe('Langfuse feedback scores', () => { ); }); + it('skips tenant scores when tenant fanout enabled is the string false', async () => { + enableTenantFanout(); + process.env.LANGFUSE_BASE_URL = 'http://central-langfuse:3000'; + const { sendFeedbackScore } = await loadFeedback(); + + await sendFeedbackScore({ + traceId: 'trace-id', + feedback: { rating: 'thumbsUp' }, + appConfig: appConfigWithLangfuse({ + publicKey: 'tenant-public-key', + secretKey: 'tenant-secret-key', + baseUrl: 'https://cloud.langfuse.com', + fanout: { enabled: 'false' }, + } as unknown as AppConfig['langfuse']), + }); + + expect(getFetchMock()).toHaveBeenCalledTimes(1); + expect(getFetchMock()).toHaveBeenCalledWith( + 'http://central-langfuse:3000/api/public/scores', + expect.objectContaining({ + method: 'POST', + headers: expect.objectContaining({ Authorization: getCentralAuthorization() }), + }), + ); + }); + it('skips tenant scores when fanout has no collector URL', async () => { process.env.LANGFUSE_FANOUT_ENABLED = 'true'; process.env.LANGFUSE_BASE_URL = 'http://central-langfuse:3000'; @@ -578,6 +658,7 @@ describe('Langfuse feedback scores', () => { appConfig: appConfigWithLangfuse({ publicKey: 'tenant-public-key', secretKey: 'tenant-secret-key', + baseUrl: 'http://tenant-langfuse:3000', }), }), ).rejects.toThrow('langfuse central score create failed: score create 500: central down'); @@ -606,6 +687,7 @@ describe('Langfuse feedback scores', () => { appConfig: appConfigWithLangfuse({ publicKey: 'tenant-public-key', secretKey: 'tenant-secret-key', + baseUrl: 'http://tenant-langfuse:3000', }), }), ).rejects.toThrow('langfuse tenant score create failed: score create 503: tenant down'); @@ -636,6 +718,7 @@ describe('Langfuse feedback scores', () => { appConfig: appConfigWithLangfuse({ publicKey: 'tenant-public-key', secretKey: 'tenant-secret-key', + baseUrl: 'http://tenant-langfuse:3000', }), }), ).rejects.toThrow( diff --git a/packages/api/src/langfuse/tenantDestinations.ts b/packages/api/src/langfuse/tenantDestinations.ts index 3b3de6dfe5..85a733aede 100644 --- a/packages/api/src/langfuse/tenantDestinations.ts +++ b/packages/api/src/langfuse/tenantDestinations.ts @@ -99,8 +99,7 @@ export function getLangfuseTenantDestinations(): LangfuseTenantDestination[] { export function resolveLangfuseTenantDestination( baseUrl: unknown, ): LangfuseTenantDestination | undefined { - const normalizedBaseUrl = - normalizeBaseUrl(baseUrl) ?? normalizeBaseUrl(process.env[LEGACY_TENANT_BASE_URL_ENV]); + const normalizedBaseUrl = normalizeBaseUrl(baseUrl); if (!normalizedBaseUrl) { return undefined; diff --git a/packages/api/src/langfuse/utils.ts b/packages/api/src/langfuse/utils.ts index 7f0b14a05e..064b1e9d80 100644 --- a/packages/api/src/langfuse/utils.ts +++ b/packages/api/src/langfuse/utils.ts @@ -5,16 +5,28 @@ export function toBasicAuthorization(publicKey: string, secretKey: string): stri const TRUE_ENV_VALUES = new Set(['1', 'true', 'yes', 'on']); const FALSE_ENV_VALUES = new Set(['0', 'false', 'no', 'off']); -export function isTrueEnv(value?: string | boolean | null): boolean { +export function normalizeBoolean(value: unknown): boolean | undefined { if (typeof value === 'boolean') { return value; } - return typeof value === 'string' && TRUE_ENV_VALUES.has(value.trim().toLowerCase()); + if (typeof value !== 'string') { + return undefined; + } + + const normalized = value.trim().toLowerCase(); + if (TRUE_ENV_VALUES.has(normalized)) { + return true; + } + if (FALSE_ENV_VALUES.has(normalized)) { + return false; + } + return undefined; } -export function isFalseEnv(value?: string | boolean | null): boolean { - if (typeof value === 'boolean') { - return !value; - } - return typeof value === 'string' && FALSE_ENV_VALUES.has(value.trim().toLowerCase()); +export function isTrueEnv(value: unknown): boolean { + return normalizeBoolean(value) === true; +} + +export function isFalseEnv(value: unknown): boolean { + return normalizeBoolean(value) === false; }