From aefe3daba770bec4747ae76abe967fa0a552171d Mon Sep 17 00:00:00 2001 From: Anup Cowkur Date: Wed, 11 Dec 2024 14:52:59 +0530 Subject: [PATCH] fix(backend): make span name query param fixes #1575 spans with names like 'GET developer.android.com/...' cannot be passed as part of url path in Gin. It confuses the routing system and results in 404s. This commit fixes the issue by passing in span names as query params instead of as part of the path. --- backend/api/main.go | 4 ++-- backend/api/measure/app.go | 25 +++++++++++++++++++++++-- docs/api/dashboard/README.md | 16 ++++++++++------ frontend/dashboard/app/api/api_calls.ts | 9 +++++++-- 4 files changed, 42 insertions(+), 12 deletions(-) diff --git a/backend/api/main.go b/backend/api/main.go index 195beca80..6d5595d5d 100644 --- a/backend/api/main.go +++ b/backend/api/main.go @@ -111,8 +111,8 @@ func main() { apps.PATCH(":id/rename", measure.RenameApp) apps.POST(":id/shortFilters", measure.CreateShortFilters) apps.GET(":id/spans/roots/names", measure.GetRootSpanNames) - apps.GET(":id/spans/:spanName/instances", measure.GetSpanInstances) - apps.GET(":id/spans/:spanName/plot", measure.GetSpanMetricsPlot) + apps.GET(":id/spans/instances", measure.GetSpanInstances) + apps.GET(":id/spans/plot", measure.GetSpanMetricsPlot) apps.GET(":id/traces/:traceId", measure.GetTrace) } diff --git a/backend/api/measure/app.go b/backend/api/measure/app.go index 7f5ad9339..f701f182b 100644 --- a/backend/api/measure/app.go +++ b/backend/api/measure/app.go @@ -7,6 +7,7 @@ import ( "fmt" "math" "net/http" + "net/url" "sort" "strings" "time" @@ -5275,7 +5276,17 @@ func GetSpanInstances(c *gin.Context) { return } - spanName := c.Param("spanName") + rawSpanName := c.Query("span_name") + if rawSpanName == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "Missing span_name query param"}) + return + } + + spanName, err := url.QueryUnescape(rawSpanName) + if err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid span_name query param"}) + return + } af := filter.AppFilter{ AppID: id, @@ -5397,7 +5408,17 @@ func GetSpanMetricsPlot(c *gin.Context) { return } - spanName := c.Param("spanName") + rawSpanName := c.Query("span_name") + if rawSpanName == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "Missing span_name query param"}) + return + } + + spanName, err := url.QueryUnescape(rawSpanName) + if err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid span_name query param"}) + return + } af := filter.AppFilter{ AppID: id, diff --git a/docs/api/dashboard/README.md b/docs/api/dashboard/README.md index cc330f901..b9b3bcc72 100644 --- a/docs/api/dashboard/README.md +++ b/docs/api/dashboard/README.md @@ -119,12 +119,12 @@ Find all the endpoints, resources and detailed documentation for Measure Dashboa - [Authorization \& Content Type](#authorization--content-type-21) - [Response Body](#response-body-21) - [Status Codes \& Troubleshooting](#status-codes--troubleshooting-21) - - [GET `/apps/:id/spans/:spanName/instances`](#get-appsidspanspannameinstances) + - [GET `/apps/:id/spans/instances`](#get-appsidspansinstances) - [Usage Notes](#usage-notes-22) - [Authorization \& Content Type](#authorization--content-type-22) - [Response Body](#response-body-22) - [Status Codes \& Troubleshooting](#status-codes--troubleshooting-22) - - [GET `/apps/:id/spans/:spanName/plot`](#get-appsidspansspannameplot) + - [GET `/apps/:id/spans/plot`](#get-appsidspansplot) - [Usage Notes](#usage-notes-23) - [Authorization \& Content Type](#authorization--content-type-23) - [Response Body](#response-body-23) @@ -219,8 +219,8 @@ Find all the endpoints, resources and detailed documentation for Measure Dashboa - [**PATCH `/apps/:id/settings`**](#patch-appsidsettings) - Update an app's settings. - [**POST `/apps/:id/shortFilters`**](#post-appsidshortfilters) - Create a shortcode to represent a combination of various app filters. - [**GET `/apps/:id/spans/roots/names`**](#get-appsidspansrootsnames) - Fetch an app's root span names list with optional filters. -- [**GET `/apps/:id/spans/:spanName/instances`**](#get-appsidspanspannameinstances) - Fetch an span's list of instances with optional filters. -- [**GET `/apps/:id/spans/:spanName/plot`**](#get-appsidspansspannameplot) - Fetch an span's metrics plot with optional filters. +- [**GET `/apps/:id/spans/instances`**](#get-appsidspansinstances) - Fetch an span's list of instances with optional filters. +- [**GET `/apps/:id/spans/plot`**](#get-appsidspansplot) - Fetch an span's metrics plot with optional filters. - [**GET `/apps/:id/traces/:traceId`**](#get-appsidtracestraceid) - Fetch a trace. ### GET `/apps/:id/journey` @@ -3603,14 +3603,16 @@ List of HTTP status codes for success and failures. -### GET `/apps/:id/spans/:spanName/instances` +### GET `/apps/:id/spans/instances` Fetch an span's list of instances with optional filters. #### Usage Notes - App's UUID must be passed in the URI +- Span name for which instances list is being fetched must be passed as a query param - Accepted query parameters + - `span_name` (_required_) - Name of the span for which instances list is being fetched. - `from` (_optional_) - ISO8601 timestamp to include sessions after this time. - `to` (_optional_) - ISO8601 timestamp to include sessions before this time. - `versions` (_optional_) - List of comma separated version identifier strings to return only matching sessions. @@ -3740,14 +3742,16 @@ List of HTTP status codes for success and failures. -### GET `/apps/:id/spans/:spanName/plot` +### GET `/apps/:id/spans/plot` Fetch an span's metrics plot with optional filters. #### Usage Notes - App's UUID must be passed in the URI +- Span name for which metrics plot is being fetched must be passed as a query param - Accepted query parameters + - `span_name` (_required_) - Name of the span for which metrics plot is being fetched. - `from` (_optional_) - ISO8601 timestamp to include sessions after this time. - `to` (_optional_) - ISO8601 timestamp to include sessions before this time. - `versions` (_optional_) - List of comma separated version identifier strings to return only matching sessions. diff --git a/frontend/dashboard/app/api/api_calls.ts b/frontend/dashboard/app/api/api_calls.ts index e08b69181..f7501ed0f 100644 --- a/frontend/dashboard/app/api/api_calls.ts +++ b/frontend/dashboard/app/api/api_calls.ts @@ -846,6 +846,11 @@ async function applyGenericFiltersToUrl(url: string, filters: Filters, keyId: st searchParams.append('anr', '1') } + // Append span name if needed + if (filters.rootSpanName !== "") { + searchParams.append('span_name', encodeURIComponent(filters.rootSpanName)) + } + // Append span statuses if needed if (filters.spanStatuses.length > 0) { filters.spanStatuses.forEach((v) => { @@ -956,7 +961,7 @@ export const fetchRootSpanNamesFromServer = async (selectedApp: typeof emptyApp, export const fetchSpanInstancesFromServer = async (filters: Filters, limit: number, offset: number, router: AppRouterInstance) => { const origin = process.env.NEXT_PUBLIC_API_BASE_URL - var url = `${origin}/apps/${filters.app.id}/spans/${filters.rootSpanName}/instances?` + var url = `${origin}/apps/${filters.app.id}/spans/instances?` url = await applyGenericFiltersToUrl(url, filters, null, null, limit, offset) @@ -979,7 +984,7 @@ export const fetchSpanInstancesFromServer = async (filters: Filters, limit: numb export const fetchSpanMetricsPlotFromServer = async (filters: Filters, router: AppRouterInstance) => { const origin = process.env.NEXT_PUBLIC_API_BASE_URL - var url = `${origin}/apps/${filters.app.id}/spans/${filters.rootSpanName}/plot?` + var url = `${origin}/apps/${filters.app.id}/spans/plot?` url = await applyGenericFiltersToUrl(url, filters, null, null, null, null)