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.
measure-sh · Dec 11, 2024 · 2f385bd · 2f385bd
1 parent ebfe147
commit 2f385bd
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 12 deletions.
diff --git 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
@@ -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
@@ -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.
 
 </details>
 
-### 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.
 
 </details>
 
-### 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
@@ -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)