From 7da7dc138f53c3aa1d526d58618c1510f66d99ee Mon Sep 17 00:00:00 2001 From: Manuel Ganter Date: Fri, 6 Feb 2026 12:00:22 +0100 Subject: [PATCH] refactor(receiver): change query endpoint to filter by workflow and job Replace /api/v1/metrics/run/{runID} and /api/v1/metrics/repo/{org}/{repo} with /api/v1/metrics/repo/{org}/{repo}/{workflow}/{job} for more precise filtering by workflow and job name. Co-Authored-By: Claude Opus 4.5 --- internal/receiver/handler.go | 31 ++------ internal/receiver/handler_test.go | 102 ++++++++------------------ internal/receiver/store.go | 13 +--- internal/receiver/store_test.go | 117 ++++++++++-------------------- 4 files changed, 78 insertions(+), 185 deletions(-) diff --git a/internal/receiver/handler.go b/internal/receiver/handler.go index ff4edd0..a00e414 100644 --- a/internal/receiver/handler.go +++ b/internal/receiver/handler.go @@ -22,8 +22,7 @@ func NewHandler(store *Store, logger *slog.Logger) *Handler { // RegisterRoutes registers all HTTP routes on the given mux func (h *Handler) RegisterRoutes(mux *http.ServeMux) { mux.HandleFunc("POST /api/v1/metrics", h.handleReceiveMetrics) - mux.HandleFunc("GET /api/v1/metrics/run/{runID}", h.handleGetByRunID) - mux.HandleFunc("GET /api/v1/metrics/repo/{org}/{repo}", h.handleGetByRepository) + mux.HandleFunc("GET /api/v1/metrics/repo/{org}/{repo}/{workflow}/{job}", h.handleGetByWorkflowJob) mux.HandleFunc("GET /health", h.handleHealth) } @@ -58,33 +57,17 @@ func (h *Handler) handleReceiveMetrics(w http.ResponseWriter, r *http.Request) { _ = json.NewEncoder(w).Encode(map[string]any{"id": id, "status": "created"}) } -func (h *Handler) handleGetByRunID(w http.ResponseWriter, r *http.Request) { - runID := r.PathValue("runID") - if runID == "" { - http.Error(w, "run_id is required", http.StatusBadRequest) - return - } - - metrics, err := h.store.GetMetricsByRunID(runID) - if err != nil { - h.logger.Error("failed to get metrics", slog.String("error", err.Error())) - http.Error(w, "failed to get metrics", http.StatusInternalServerError) - return - } - - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(metrics) -} - -func (h *Handler) handleGetByRepository(w http.ResponseWriter, r *http.Request) { +func (h *Handler) handleGetByWorkflowJob(w http.ResponseWriter, r *http.Request) { org := r.PathValue("org") repo := r.PathValue("repo") - if org == "" || repo == "" { - http.Error(w, "org and repo are required", http.StatusBadRequest) + workflow := r.PathValue("workflow") + job := r.PathValue("job") + if org == "" || repo == "" || workflow == "" || job == "" { + http.Error(w, "org, repo, workflow and job are required", http.StatusBadRequest) return } - metrics, err := h.store.GetMetricsByRepository(org, repo) + metrics, err := h.store.GetMetricsByWorkflowJob(org, repo, workflow, job) if err != nil { h.logger.Error("failed to get metrics", slog.String("error", err.Error())) http.Error(w, "failed to get metrics", http.StatusInternalServerError) diff --git a/internal/receiver/handler_test.go b/internal/receiver/handler_test.go index a1845d2..fdd3894 100644 --- a/internal/receiver/handler_test.go +++ b/internal/receiver/handler_test.go @@ -98,81 +98,15 @@ func TestHandler_ReceiveMetrics_MissingRunID(t *testing.T) { } } -func TestHandler_GetByRunID(t *testing.T) { +func TestHandler_GetByWorkflowJob(t *testing.T) { h, cleanup := newTestHandler(t) defer cleanup() - // First, save a metric - payload := &MetricsPayload{ - Execution: ExecutionContext{ - Organization: "test-org", - Repository: "test-repo", - Workflow: "ci.yml", - Job: "build", - RunID: "run-get-test", - }, - Summary: summary.RunSummary{SampleCount: 5}, - } - if _, err := h.store.SaveMetric(payload); err != nil { - t.Fatalf("SaveMetric() error = %v", err) - } - - req := httptest.NewRequest(http.MethodGet, "/api/v1/metrics/run/run-get-test", nil) - rec := httptest.NewRecorder() - - mux := http.NewServeMux() - h.RegisterRoutes(mux) - mux.ServeHTTP(rec, req) - - if rec.Code != http.StatusOK { - t.Errorf("status = %d, want %d", rec.Code, http.StatusOK) - } - - var metrics []Metric - if err := json.NewDecoder(rec.Body).Decode(&metrics); err != nil { - t.Fatalf("failed to decode response: %v", err) - } - if len(metrics) != 1 { - t.Errorf("got %d metrics, want 1", len(metrics)) - } - if metrics[0].RunID != "run-get-test" { - t.Errorf("RunID = %q, want %q", metrics[0].RunID, "run-get-test") - } -} - -func TestHandler_GetByRunID_NotFound(t *testing.T) { - h, cleanup := newTestHandler(t) - defer cleanup() - - req := httptest.NewRequest(http.MethodGet, "/api/v1/metrics/run/nonexistent", nil) - rec := httptest.NewRecorder() - - mux := http.NewServeMux() - h.RegisterRoutes(mux) - mux.ServeHTTP(rec, req) - - if rec.Code != http.StatusOK { - t.Errorf("status = %d, want %d", rec.Code, http.StatusOK) - } - - var metrics []Metric - if err := json.NewDecoder(rec.Body).Decode(&metrics); err != nil { - t.Fatalf("failed to decode response: %v", err) - } - if len(metrics) != 0 { - t.Errorf("got %d metrics, want 0", len(metrics)) - } -} - -func TestHandler_GetByRepository(t *testing.T) { - h, cleanup := newTestHandler(t) - defer cleanup() - - // Save metrics for different repos + // Save metrics for different workflow/job combinations payloads := []*MetricsPayload{ - {Execution: ExecutionContext{Organization: "org-x", Repository: "repo-y", RunID: "r1"}}, - {Execution: ExecutionContext{Organization: "org-x", Repository: "repo-y", RunID: "r2"}}, - {Execution: ExecutionContext{Organization: "org-x", Repository: "repo-z", RunID: "r3"}}, + {Execution: ExecutionContext{Organization: "org-x", Repository: "repo-y", Workflow: "ci.yml", Job: "build", RunID: "r1"}}, + {Execution: ExecutionContext{Organization: "org-x", Repository: "repo-y", Workflow: "ci.yml", Job: "build", RunID: "r2"}}, + {Execution: ExecutionContext{Organization: "org-x", Repository: "repo-y", Workflow: "ci.yml", Job: "test", RunID: "r3"}}, } for _, p := range payloads { if _, err := h.store.SaveMetric(p); err != nil { @@ -180,7 +114,7 @@ func TestHandler_GetByRepository(t *testing.T) { } } - req := httptest.NewRequest(http.MethodGet, "/api/v1/metrics/repo/org-x/repo-y", nil) + req := httptest.NewRequest(http.MethodGet, "/api/v1/metrics/repo/org-x/repo-y/ci.yml/build", nil) rec := httptest.NewRecorder() mux := http.NewServeMux() @@ -200,6 +134,30 @@ func TestHandler_GetByRepository(t *testing.T) { } } +func TestHandler_GetByWorkflowJob_NotFound(t *testing.T) { + h, cleanup := newTestHandler(t) + defer cleanup() + + req := httptest.NewRequest(http.MethodGet, "/api/v1/metrics/repo/org/repo/workflow/job", nil) + rec := httptest.NewRecorder() + + mux := http.NewServeMux() + h.RegisterRoutes(mux) + mux.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Errorf("status = %d, want %d", rec.Code, http.StatusOK) + } + + var metrics []Metric + if err := json.NewDecoder(rec.Body).Decode(&metrics); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + if len(metrics) != 0 { + t.Errorf("got %d metrics, want 0", len(metrics)) + } +} + func TestHandler_Health(t *testing.T) { h, cleanup := newTestHandler(t) defer cleanup() diff --git a/internal/receiver/store.go b/internal/receiver/store.go index 85112a5..4ae970a 100644 --- a/internal/receiver/store.go +++ b/internal/receiver/store.go @@ -70,17 +70,10 @@ func (s *Store) SaveMetric(payload *MetricsPayload) (uint, error) { return metric.ID, nil } -// GetMetricsByRunID retrieves all metrics for a specific run -func (s *Store) GetMetricsByRunID(runID string) ([]Metric, error) { +// GetMetricsByWorkflowJob retrieves all metrics for a specific workflow and job +func (s *Store) GetMetricsByWorkflowJob(org, repo, workflow, job string) ([]Metric, error) { var metrics []Metric - result := s.db.Where("run_id = ?", runID).Order("received_at DESC").Find(&metrics) - return metrics, result.Error -} - -// GetMetricsByRepository retrieves all metrics for a specific repository -func (s *Store) GetMetricsByRepository(org, repo string) ([]Metric, error) { - var metrics []Metric - result := s.db.Where("organization = ? AND repository = ?", org, repo).Order("received_at DESC").Find(&metrics) + result := s.db.Where("organization = ? AND repository = ? AND workflow = ? AND job = ?", org, repo, workflow, job).Order("received_at DESC").Find(&metrics) return metrics, result.Error } diff --git a/internal/receiver/store_test.go b/internal/receiver/store_test.go index 8302535..44c4f17 100644 --- a/internal/receiver/store_test.go +++ b/internal/receiver/store_test.go @@ -55,84 +55,30 @@ func TestStore_SaveMetric(t *testing.T) { } } -func TestStore_GetMetricsByRunID(t *testing.T) { +func TestStore_GetMetricsByWorkflowJob(t *testing.T) { store := newTestStore(t) defer func() { _ = store.Close() }() - // Save two metrics with same run ID - for i := 0; i < 2; i++ { - payload := &MetricsPayload{ - Execution: ExecutionContext{ - Organization: "test-org", - Repository: "test-repo", - Workflow: "ci.yml", - Job: "build", - RunID: "run-456", - }, - Summary: summary.RunSummary{SampleCount: i + 1}, - } - if _, err := store.SaveMetric(payload); err != nil { - t.Fatalf("SaveMetric() error = %v", err) - } - } - - // Save one with different run ID - otherPayload := &MetricsPayload{ - Execution: ExecutionContext{RunID: "run-789"}, - Summary: summary.RunSummary{}, - } - if _, err := store.SaveMetric(otherPayload); err != nil { - t.Fatalf("SaveMetric() error = %v", err) - } - - metrics, err := store.GetMetricsByRunID("run-456") - if err != nil { - t.Fatalf("GetMetricsByRunID() error = %v", err) - } - if len(metrics) != 2 { - t.Errorf("GetMetricsByRunID() returned %d metrics, want 2", len(metrics)) - } - - for _, m := range metrics { - if m.RunID != "run-456" { - t.Errorf("GetMetricsByRunID() returned metric with RunID = %q, want %q", m.RunID, "run-456") - } - } -} - -func TestStore_GetMetricsByRunID_NotFound(t *testing.T) { - store := newTestStore(t) - defer func() { _ = store.Close() }() - - metrics, err := store.GetMetricsByRunID("nonexistent") - if err != nil { - t.Fatalf("GetMetricsByRunID() error = %v", err) - } - if len(metrics) != 0 { - t.Errorf("GetMetricsByRunID() returned %d metrics, want 0", len(metrics)) - } -} - -func TestStore_GetMetricsByRepository(t *testing.T) { - store := newTestStore(t) - defer func() { _ = store.Close() }() - - // Save metrics for different repos - repos := []struct { - org string - repo string + // Save metrics for different workflow/job combinations + payloads := []struct { + org string + repo string + workflow string + job string }{ - {"org-a", "repo-1"}, - {"org-a", "repo-1"}, - {"org-a", "repo-2"}, - {"org-b", "repo-1"}, + {"org-a", "repo-1", "ci.yml", "build"}, + {"org-a", "repo-1", "ci.yml", "build"}, + {"org-a", "repo-1", "ci.yml", "test"}, + {"org-a", "repo-1", "deploy.yml", "build"}, } - for i, r := range repos { + for i, p := range payloads { payload := &MetricsPayload{ Execution: ExecutionContext{ - Organization: r.org, - Repository: r.repo, + Organization: p.org, + Repository: p.repo, + Workflow: p.workflow, + Job: p.job, RunID: "run-" + string(rune('a'+i)), }, Summary: summary.RunSummary{}, @@ -142,22 +88,35 @@ func TestStore_GetMetricsByRepository(t *testing.T) { } } - metrics, err := store.GetMetricsByRepository("org-a", "repo-1") + metrics, err := store.GetMetricsByWorkflowJob("org-a", "repo-1", "ci.yml", "build") if err != nil { - t.Fatalf("GetMetricsByRepository() error = %v", err) + t.Fatalf("GetMetricsByWorkflowJob() error = %v", err) } if len(metrics) != 2 { - t.Errorf("GetMetricsByRepository() returned %d metrics, want 2", len(metrics)) + t.Errorf("GetMetricsByWorkflowJob() returned %d metrics, want 2", len(metrics)) } for _, m := range metrics { - if m.Organization != "org-a" || m.Repository != "repo-1" { - t.Errorf("GetMetricsByRepository() returned metric with org=%q repo=%q, want org-a/repo-1", - m.Organization, m.Repository) + if m.Organization != "org-a" || m.Repository != "repo-1" || m.Workflow != "ci.yml" || m.Job != "build" { + t.Errorf("GetMetricsByWorkflowJob() returned metric with org=%q repo=%q workflow=%q job=%q, want org-a/repo-1/ci.yml/build", + m.Organization, m.Repository, m.Workflow, m.Job) } } } +func TestStore_GetMetricsByWorkflowJob_NotFound(t *testing.T) { + store := newTestStore(t) + defer func() { _ = store.Close() }() + + metrics, err := store.GetMetricsByWorkflowJob("nonexistent", "repo", "workflow", "job") + if err != nil { + t.Fatalf("GetMetricsByWorkflowJob() error = %v", err) + } + if len(metrics) != 0 { + t.Errorf("GetMetricsByWorkflowJob() returned %d metrics, want 0", len(metrics)) + } +} + func TestStore_SaveMetric_PreservesPayload(t *testing.T) { store := newTestStore(t) defer func() { _ = store.Close() }() @@ -182,12 +141,12 @@ func TestStore_SaveMetric_PreservesPayload(t *testing.T) { t.Fatalf("SaveMetric() error = %v", err) } - metrics, err := store.GetMetricsByRunID("run-preserve") + metrics, err := store.GetMetricsByWorkflowJob("test-org", "test-repo", "build.yml", "test") if err != nil { - t.Fatalf("GetMetricsByRunID() error = %v", err) + t.Fatalf("GetMetricsByWorkflowJob() error = %v", err) } if len(metrics) != 1 { - t.Fatalf("GetMetricsByRunID() returned %d metrics, want 1", len(metrics)) + t.Fatalf("GetMetricsByWorkflowJob() returned %d metrics, want 1", len(metrics)) } m := metrics[0]