Skip to content

Commit 6f042df

Browse files
99cardz99cardz
committed
refactor(graphite): remove unused graphiteDensityBucketSize function and related tests
1 parent 1a5dccc commit 6f042df

2 files changed

Lines changed: 1 addition & 284 deletions

File tree

‎tools/graphite.go‎

Lines changed: 1 addition & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -362,46 +362,6 @@ var ListGraphiteTags = mcpgrafana.MustTool(
362362
mcp.WithReadOnlyHintAnnotation(true),
363363
)
364364

365-
// graphiteDensityBucketSize returns an appropriate summarize bucket size for
366-
// the given from value. It targets roughly 12–48 buckets over the window;
367-
// if from cannot be parsed as a relative duration, "1h" is used.
368-
func graphiteDensityBucketSize(from string) string {
369-
if !strings.HasPrefix(from, "-") {
370-
return "1h"
371-
}
372-
s := from[1:]
373-
var n int
374-
var unit string
375-
if _, err := fmt.Sscanf(s, "%d%s", &n, &unit); err != nil {
376-
return "1h"
377-
}
378-
var d time.Duration
379-
switch unit {
380-
case "m", "min":
381-
d = time.Duration(n) * time.Minute
382-
case "h":
383-
d = time.Duration(n) * time.Hour
384-
case "d":
385-
d = time.Duration(n) * 24 * time.Hour
386-
case "w":
387-
d = time.Duration(n) * 7 * 24 * time.Hour
388-
default:
389-
return "1h"
390-
}
391-
switch {
392-
case d <= 3*time.Hour:
393-
return "5m"
394-
case d <= 24*time.Hour:
395-
return "30m"
396-
case d <= 72*time.Hour:
397-
return "1h"
398-
case d <= 14*24*time.Hour:
399-
return "6h"
400-
default:
401-
return "1d"
402-
}
403-
}
404-
405365
// computeSeriesDensity derives data-density statistics from the parsed
406366
// datapoints of a single Graphite series.
407367
func computeSeriesDensity(target string, pts []GraphiteDatapoint) *GraphiteSeriesDensity {
@@ -504,38 +464,6 @@ func queryGraphiteDensity(ctx context.Context, args QueryGraphiteDensityParams)
504464
until = "now"
505465
}
506466

507-
// Primary path: fetch summarize(isNonNull(<target>), <bucket>, "sum") to
508-
// obtain a compact per-bucket count of non-null points for each series.
509-
// This avoids downloading full-resolution data for long windows.
510-
bucketSize := graphiteDensityBucketSize(from)
511-
isNonNullTarget := fmt.Sprintf(`summarize(isNonNull(%s),"%s","sum")`, args.Target, bucketSize)
512-
513-
isNonNullParams := url.Values{}
514-
isNonNullParams.Set("target", isNonNullTarget)
515-
isNonNullParams.Set("from", from)
516-
isNonNullParams.Set("until", until)
517-
isNonNullParams.Set("format", "json")
518-
519-
var isNonNullCounts []int
520-
if data, fetchErr := client.doGet(ctx, "/render", isNonNullParams); fetchErr == nil {
521-
var raw []graphiteRawSeries
522-
if json.Unmarshal(data, &raw) == nil && len(raw) > 0 {
523-
isNonNullCounts = make([]int, len(raw))
524-
for i, s := range raw {
525-
for _, dp := range parseGraphiteDatapoints(s.Datapoints) {
526-
if dp.Value != nil {
527-
isNonNullCounts[i] += int(*dp.Value)
528-
}
529-
}
530-
}
531-
}
532-
}
533-
// isNonNullCounts is nil when isNonNull() is unsupported by this Graphite
534-
// version or the target matched no series; in both cases nonNullPoints is
535-
// computed from raw datapoints below.
536-
537-
// Always fetch the raw series: required for TotalPoints, LastSeen,
538-
// LongestGapSec, and EstimatedInterval.
539467
rawParams := url.Values{}
540468
rawParams.Set("target", args.Target)
541469
rawParams.Set("from", from)
@@ -552,23 +480,12 @@ func queryGraphiteDensity(ctx context.Context, args QueryGraphiteDensityParams)
552480
return nil, fmt.Errorf("parsing graphite render response: %w", err)
553481
}
554482

555-
// Only use isNonNull counts when the series count matches exactly.
556-
// A mismatch means summarize consolidated or skipped series differently,
557-
// so fall back to counting non-null points from raw datapoints.
558-
useIsNonNull := isNonNullCounts != nil && len(isNonNullCounts) == len(rawSeries)
559-
560483
result := &QueryGraphiteDensityResult{
561484
Series: make([]*GraphiteSeriesDensity, 0, len(rawSeries)),
562485
}
563-
for i, rs := range rawSeries {
486+
for _, rs := range rawSeries {
564487
pts := parseGraphiteDatapoints(rs.Datapoints)
565488
stats := computeSeriesDensity(rs.Target, pts)
566-
if useIsNonNull {
567-
stats.NonNullPoints = isNonNullCounts[i]
568-
if stats.TotalPoints > 0 {
569-
stats.FillRatio = float64(stats.NonNullPoints) / float64(stats.TotalPoints)
570-
}
571-
}
572489
result.Series = append(result.Series, stats)
573490
}
574491
return result, nil

‎tools/graphite_unit_test.go‎

Lines changed: 0 additions & 200 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"net/http"
77
"net/http/httptest"
88
"net/url"
9-
"strings"
109
"testing"
1110
"time"
1211

@@ -363,37 +362,6 @@ func TestGraphiteClient_DoGet_NonOKStatus(t *testing.T) {
363362
assert.Contains(t, err.Error(), "500")
364363
}
365364

366-
// --- graphiteDensityBucketSize ---
367-
368-
func TestGraphiteDensityBucketSize(t *testing.T) {
369-
tests := []struct {
370-
from string
371-
want string
372-
}{
373-
{"-30m", "5m"},
374-
{"-1h", "5m"},
375-
{"-3h", "5m"},
376-
{"-4h", "30m"},
377-
{"-12h", "30m"},
378-
{"-24h", "30m"},
379-
{"-48h", "1h"},
380-
{"-72h", "1h"},
381-
{"-7d", "6h"},
382-
{"-14d", "6h"},
383-
{"-30d", "1d"},
384-
{"-1w", "6h"},
385-
{"", "1h"},
386-
{"now", "1h"},
387-
{"2024-01-01T00:00:00Z", "1h"},
388-
{"-5x", "1h"}, // unknown unit
389-
}
390-
for _, tc := range tests {
391-
t.Run(tc.from, func(t *testing.T) {
392-
assert.Equal(t, tc.want, graphiteDensityBucketSize(tc.from))
393-
})
394-
}
395-
}
396-
397365
// --- computeSeriesDensity ---
398366

399367
func TestComputeSeriesDensity_AllNull(t *testing.T) {
@@ -607,171 +575,3 @@ func TestQueryGraphiteDensity_MixedCluster(t *testing.T) {
607575
assert.Equal(t, int64(60), s2.EstimatedInterval)
608576
}
609577

610-
func TestQueryGraphiteDensity_IsNonNullUsedWhenCountMatches(t *testing.T) {
611-
// The isNonNull+summarize call reports 3 non-null points; the raw series has
612-
// 2 non-null points (could differ due to resolution). When counts match,
613-
// the isNonNull value is used for NonNullPoints / FillRatio.
614-
isNonNullResp := []graphiteRawSeries{
615-
{
616-
Target: `summarize(isNonNull(a.b.c),"5m","sum")`,
617-
Datapoints: [][]json.RawMessage{
618-
{json.RawMessage("2"), json.RawMessage("1704067200")},
619-
{json.RawMessage("1"), json.RawMessage("1704067500")},
620-
},
621-
},
622-
}
623-
rawResp := []graphiteRawSeries{
624-
{
625-
Target: "a.b.c",
626-
Datapoints: [][]json.RawMessage{
627-
{json.RawMessage("1.0"), json.RawMessage("1704067200")},
628-
{json.RawMessage("null"), json.RawMessage("1704067260")},
629-
{json.RawMessage("2.0"), json.RawMessage("1704067320")},
630-
{json.RawMessage("null"), json.RawMessage("1704067380")},
631-
},
632-
},
633-
}
634-
635-
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
636-
w.Header().Set("Content-Type", "application/json")
637-
target := r.URL.Query().Get("target")
638-
if strings.HasPrefix(target, "summarize") {
639-
_ = json.NewEncoder(w).Encode(isNonNullResp)
640-
} else {
641-
_ = json.NewEncoder(w).Encode(rawResp)
642-
}
643-
}))
644-
t.Cleanup(ts.Close)
645-
646-
client := &GraphiteClient{httpClient: http.DefaultClient, baseURL: ts.URL}
647-
ctx := context.Background()
648-
649-
// Fetch isNonNull
650-
innParams := url.Values{}
651-
innParams.Set("target", `summarize(isNonNull(a.b.c),"5m","sum")`)
652-
innParams.Set("format", "json")
653-
innData, err := client.doGet(ctx, "/render", innParams)
654-
require.NoError(t, err)
655-
var innSeries []graphiteRawSeries
656-
require.NoError(t, json.Unmarshal(innData, &innSeries))
657-
658-
isNonNullCounts := make([]int, len(innSeries))
659-
for i, s := range innSeries {
660-
for _, dp := range parseGraphiteDatapoints(s.Datapoints) {
661-
if dp.Value != nil {
662-
isNonNullCounts[i] += int(*dp.Value)
663-
}
664-
}
665-
}
666-
assert.Equal(t, []int{3}, isNonNullCounts) // 2+1 buckets
667-
668-
// Fetch raw
669-
rawParams := url.Values{}
670-
rawParams.Set("target", "a.b.c")
671-
rawParams.Set("format", "json")
672-
rawData, err := client.doGet(ctx, "/render", rawParams)
673-
require.NoError(t, err)
674-
var raws []graphiteRawSeries
675-
require.NoError(t, json.Unmarshal(rawData, &raws))
676-
677-
// Counts match (1 == 1) → use isNonNull count
678-
useIsNonNull := len(isNonNullCounts) == len(raws)
679-
assert.True(t, useIsNonNull)
680-
681-
stats := computeSeriesDensity(raws[0].Target, parseGraphiteDatapoints(raws[0].Datapoints))
682-
if useIsNonNull {
683-
stats.NonNullPoints = isNonNullCounts[0]
684-
if stats.TotalPoints > 0 {
685-
stats.FillRatio = float64(stats.NonNullPoints) / float64(stats.TotalPoints)
686-
}
687-
}
688-
assert.Equal(t, 3, stats.NonNullPoints) // from isNonNull, not raw count (2)
689-
assert.InDelta(t, 0.75, stats.FillRatio, 1e-9) // 3/4
690-
}
691-
692-
func TestQueryGraphiteDensity_IsNonNullCountMismatch_FallsBackToRaw(t *testing.T) {
693-
// isNonNull returns 2 series; raw returns 3 → mismatch → use raw counts.
694-
isNonNullResp := []graphiteRawSeries{
695-
{Target: `summarize(isNonNull(a.b.1),"5m","sum")`,
696-
Datapoints: [][]json.RawMessage{{json.RawMessage("3"), json.RawMessage("1704067200")}}},
697-
{Target: `summarize(isNonNull(a.b.2),"5m","sum")`,
698-
Datapoints: [][]json.RawMessage{{json.RawMessage("null"), json.RawMessage("1704067200")}}},
699-
}
700-
rawResp := []graphiteRawSeries{
701-
{
702-
Target: "a.b.1",
703-
Datapoints: [][]json.RawMessage{
704-
{json.RawMessage("1.0"), json.RawMessage("1704067200")},
705-
{json.RawMessage("null"), json.RawMessage("1704067260")},
706-
},
707-
},
708-
{
709-
Target: "a.b.2",
710-
Datapoints: [][]json.RawMessage{
711-
{json.RawMessage("null"), json.RawMessage("1704067200")},
712-
{json.RawMessage("null"), json.RawMessage("1704067260")},
713-
},
714-
},
715-
{
716-
Target: "a.b.3", // extra series absent from isNonNull result
717-
Datapoints: [][]json.RawMessage{
718-
{json.RawMessage("2.0"), json.RawMessage("1704067200")},
719-
{json.RawMessage("3.0"), json.RawMessage("1704067260")},
720-
},
721-
},
722-
}
723-
724-
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
725-
w.Header().Set("Content-Type", "application/json")
726-
target := r.URL.Query().Get("target")
727-
if strings.HasPrefix(target, "summarize") {
728-
_ = json.NewEncoder(w).Encode(isNonNullResp)
729-
} else {
730-
_ = json.NewEncoder(w).Encode(rawResp)
731-
}
732-
}))
733-
t.Cleanup(ts.Close)
734-
735-
client := &GraphiteClient{httpClient: http.DefaultClient, baseURL: ts.URL}
736-
ctx := context.Background()
737-
738-
// Fetch isNonNull
739-
innParams := url.Values{}
740-
innParams.Set("target", `summarize(isNonNull(a.b.*),"5m","sum")`)
741-
innParams.Set("format", "json")
742-
innData, err := client.doGet(ctx, "/render", innParams)
743-
require.NoError(t, err)
744-
var innSeries []graphiteRawSeries
745-
require.NoError(t, json.Unmarshal(innData, &innSeries))
746-
747-
isNonNullCounts := make([]int, len(innSeries))
748-
for i, s := range innSeries {
749-
for _, dp := range parseGraphiteDatapoints(s.Datapoints) {
750-
if dp.Value != nil {
751-
isNonNullCounts[i] += int(*dp.Value)
752-
}
753-
}
754-
}
755-
756-
// Fetch raw
757-
rawParams := url.Values{}
758-
rawParams.Set("target", "a.b.*")
759-
rawParams.Set("format", "json")
760-
rawData, err := client.doGet(ctx, "/render", rawParams)
761-
require.NoError(t, err)
762-
var raws []graphiteRawSeries
763-
require.NoError(t, json.Unmarshal(rawData, &raws))
764-
765-
// 2 isNonNull vs 3 raw → mismatch → do NOT use isNonNull
766-
useIsNonNull := len(isNonNullCounts) == len(raws)
767-
assert.False(t, useIsNonNull)
768-
769-
// Compute stats from raw only
770-
var results []*GraphiteSeriesDensity
771-
for _, rs := range raws {
772-
results = append(results, computeSeriesDensity(rs.Target, parseGraphiteDatapoints(rs.Datapoints)))
773-
}
774-
assert.Equal(t, 1, results[0].NonNullPoints) // a.b.1: from raw
775-
assert.Equal(t, 0, results[1].NonNullPoints) // a.b.2: from raw
776-
assert.Equal(t, 2, results[2].NonNullPoints) // a.b.3: from raw
777-
}

0 commit comments

Comments
 (0)