-
Notifications
You must be signed in to change notification settings - Fork 11
Add support for dynamically querying multiple tenants in trace domain #229
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import { StoreConfig } from '../korrel8r-client'; | ||
|
|
||
| export interface TraceContext { | ||
| namespace: string; | ||
| name: string; | ||
| tenant: string; | ||
| } | ||
|
|
||
| /** | ||
| * Extracts trace context (namespace, name, tenant) from the current URL. | ||
| * These parameters are set when the user navigates to the traces console. | ||
| */ | ||
| export const extractTraceContext = (): TraceContext | null => { | ||
| const searchParams = new URLSearchParams(window.location.search); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should use react-router functions to check the url path to see if the user is on a tracing page (we don't want to change the tracing context if say networking also just so happens to use these search params) and use |
||
|
|
||
| // Check if we're on a traces page with tenant information | ||
| const namespace = searchParams.get('namespace'); | ||
| const name = searchParams.get('name'); | ||
| const tenant = searchParams.get('tenant'); | ||
|
|
||
| // If any of these are missing, return null (not on traces page or no tenant selected) | ||
| if (!namespace || !name || !tenant) { | ||
| return null; | ||
|
Comment on lines
+17
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate/encode URL-derived context before building the Tempo endpoint.
Suggested fix (input guard + path encoding) export const extractTraceContext = (): TraceContext | null => {
const searchParams = new URLSearchParams(window.location.search);
+ const isDnsLabel = (value: string): boolean =>
+ /^[a-z0-9]([-a-z0-9]*[a-z0-9])?$/.test(value);
@@
- if (!namespace || !name || !tenant) {
+ if (!namespace || !name || !tenant || !isDnsLabel(namespace) || !isDnsLabel(name)) {
return null;
}
return { namespace, name, tenant };
};
@@
export const buildTraceStoreConfig = (context: TraceContext): StoreConfig => {
const { namespace, name, tenant } = context;
+ const encodedTenant = encodeURIComponent(tenant);
@@
- const tempoStackURL = `https://tempo-${name}-gateway.${namespace}.svc.cluster.local:8080/api/traces/v1/${tenant}/tempo/api/search`;
+ const tempoStackURL = `https://tempo-${name}-gateway.${namespace}.svc.cluster.local:8080/api/traces/v1/${encodedTenant}/tempo/api/search`;As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity." Also applies to: 37-37 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| return { namespace, name, tenant }; | ||
| }; | ||
|
|
||
| /** | ||
| * Builds a store configuration for the trace domain based on trace context. | ||
| */ | ||
| export const buildTraceStoreConfig = (context: TraceContext): StoreConfig => { | ||
| const { namespace, name, tenant } = context; | ||
|
|
||
| // Build the tempoStack URL with the tenant path | ||
| // Format: https://tempo-{name}-gateway.{namespace}.svc.cluster.local:8080/api/traces/v1/{tenant}/tempo/api/search | ||
| const tempoStackURL = `https://tempo-${name}-gateway.${namespace}.svc.cluster.local:8080/api/traces/v1/${tenant}/tempo/api/search`; | ||
|
|
||
| return { | ||
| domain: 'trace', | ||
| tempoStack: tempoStackURL, | ||
| certificateAuthority: '/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt', | ||
| }; | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be updated to align with the current korrel8r-client structure using tanstack-query, specifically as a mutation (see https://tanstack.com/query/latest/docs/framework/react/guides/mutations)