Skip to content

Commit d515b62

Browse files
victoriametrics: fix seriesByTag name→__graphite__ mapping and quote-aware tag splitting
Two fixes for seriesByTag conversion to PromQL: 1. VictoriaMetrics: replace fragile strings.ReplaceAll("'name=","'__name__=") with a proper tag rename via new SeriesByTagToPromQLWithRenames helper. The "name" tag (Graphite metric path) is now correctly mapped to __graphite__ (matching how VM stores Graphite data), not __name__. This is consistent with the non-seriesByTag path which already uses {__graphite__="..."}. 2. SplitTagValues: use quote-aware comma splitting so that tag values containing commas (e.g. seriesByTag('desc=hello, world')) are not incorrectly split into separate arguments. Includes tests for SplitTagValues and SeriesByTagToPromQLWithRenames. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent beaa7db commit d515b62

3 files changed

Lines changed: 145 additions & 8 deletions

File tree

zipper/protocols/prometheus/helpers/helpers.go

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ func PromethizeTagValue(tagValue string) (string, types.Tag) {
160160
case idx+1 == len(tagValue): // != or = with empty value
161161
t.OP += "="
162162
case tagValue[idx+1] == '~':
163-
if len(t.OP) > 0 { // !=~
163+
if t.OP != "" { // !=~
164164
t.OP += "~"
165165
} else { // =~
166166
t.OP = "=~"
@@ -177,12 +177,37 @@ func PromethizeTagValue(tagValue string) (string, types.Tag) {
177177
return tagName, t
178178
}
179179

180-
// SplitTagValues - For given tag-value list converts it to more usable map[string]Tag, where string is TagName
180+
// splitWithQuotes splits s by delimiter, but skips delimiters inside single- or double-quoted regions.
181+
// This is needed because seriesByTag arguments are quoted ('tag=value') and tag values themselves may
182+
// contain commas which must not be treated as argument separators.
183+
func splitWithQuotes(s string, delimiter rune) []string {
184+
var result []string
185+
start := 0
186+
var quoteChar rune // 0 = outside quotes
187+
for i, c := range s {
188+
switch {
189+
case c == quoteChar:
190+
quoteChar = 0
191+
case quoteChar == 0 && (c == '\'' || c == '"'):
192+
quoteChar = c
193+
case quoteChar == 0 && c == delimiter:
194+
result = append(result, s[start:i])
195+
start = i + 1
196+
}
197+
}
198+
return append(result, s[start:])
199+
}
200+
201+
// SplitTagValues - For given tag-value list converts it to more usable map[string]Tag, where string is TagName.
202+
// Splits by comma but respects commas inside quoted strings (e.g. seriesByTag('tag=value','other=val,ue')).
181203
func SplitTagValues(query string) map[string]types.Tag {
182-
tags := strings.Split(query, ",")
204+
tags := splitWithQuotes(query, ',')
183205
result := make(map[string]types.Tag)
184206
for _, tvString := range tags {
185207
tvString = strings.TrimSpace(tvString)
208+
if len(tvString) < 2 {
209+
continue
210+
}
186211
name, tag := PromethizeTagValue(tvString[1 : len(tvString)-1])
187212
result[name] = tag
188213
}
@@ -210,13 +235,34 @@ func PromMetricToGraphite(metric map[string]string) string {
210235
return res.String()
211236
}
212237

213-
// SeriesByTagToPromQL converts graphite SeriesByTag to PromQL
214-
// will return step if __step__ is passed
238+
// SeriesByTagToPromQL converts graphite SeriesByTag to PromQL.
239+
// Returns step (possibly overridden by __step__ tag) and the PromQL selector string.
215240
func SeriesByTagToPromQL(step, target string) (string, string) {
241+
return convertSeriesByTagToPromQL(step, target, nil)
242+
}
243+
244+
// SeriesByTagToPromQLWithRenames is like SeriesByTagToPromQL but applies tagRenames
245+
// to tag names after parsing and before building the PromQL selector. For example,
246+
// passing map[string]string{"name": "__graphite__"} renames the Graphite "name" tag
247+
// to the VictoriaMetrics "__graphite__" label.
248+
func SeriesByTagToPromQLWithRenames(step, target string, tagRenames map[string]string) (string, string) {
249+
return convertSeriesByTagToPromQL(step, target, tagRenames)
250+
}
251+
252+
func convertSeriesByTagToPromQL(step, target string, tagRenames map[string]string) (string, string) {
216253
firstTag := true
217254
var queryBuilder strings.Builder
218255
tagsString := target[len("seriesByTag(") : len(target)-1]
219256
tvs := SplitTagValues(tagsString)
257+
258+
// Apply tag renames (e.g. "name" -> "__graphite__" for VictoriaMetrics).
259+
for oldName, newName := range tagRenames {
260+
if v, ok := tvs[oldName]; ok {
261+
delete(tvs, oldName)
262+
tvs[newName] = v
263+
}
264+
}
265+
220266
// It's ok to have empty "__name__"
221267
if v, ok := tvs["__name__"]; ok {
222268
if v.OP == "=" {

zipper/protocols/prometheus/helpers/helpers_test.go

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,3 +250,91 @@ func TestPromethizeTagValue(t *testing.T) {
250250
}
251251

252252
}
253+
254+
func TestSplitTagValues(t *testing.T) {
255+
tests := []struct {
256+
name string
257+
query string
258+
want map[string]types.Tag
259+
}{
260+
{
261+
name: "simple two tags",
262+
query: "'env=prod','host=web1'",
263+
want: map[string]types.Tag{
264+
"env": {OP: "=", TagValue: "prod"},
265+
"host": {OP: "=", TagValue: "web1"},
266+
},
267+
},
268+
{
269+
name: "comma inside quoted value",
270+
query: "'env=prod','desc=hello, world'",
271+
want: map[string]types.Tag{
272+
"env": {OP: "=", TagValue: "prod"},
273+
"desc": {OP: "=", TagValue: "hello, world"},
274+
},
275+
},
276+
{
277+
name: "empty input",
278+
query: "",
279+
want: map[string]types.Tag{},
280+
},
281+
{
282+
name: "single tag",
283+
query: "'name=cpu.load'",
284+
want: map[string]types.Tag{
285+
"name": {OP: "=", TagValue: "cpu.load"},
286+
},
287+
},
288+
}
289+
290+
for _, tt := range tests {
291+
t.Run(tt.name, func(t *testing.T) {
292+
got := SplitTagValues(tt.query)
293+
assert.Equal(t, tt.want, got)
294+
})
295+
}
296+
}
297+
298+
func TestSeriesByTagToPromQLWithRenames(t *testing.T) {
299+
tests := []struct {
300+
name string
301+
step string
302+
target string
303+
renames map[string]string
304+
wantStep string
305+
wantPromQL string
306+
}{
307+
{
308+
name: "VM: name renamed to __graphite__",
309+
step: "30",
310+
target: "seriesByTag('name=servers.web.cpu','env=prod')",
311+
renames: map[string]string{"name": "__graphite__"},
312+
wantStep: "30",
313+
wantPromQL: `{__graphite__="servers.web.cpu", env="prod"}`,
314+
},
315+
{
316+
name: "VM: hostname not affected by name rename",
317+
step: "30",
318+
target: "seriesByTag('hostname=web1','env=prod')",
319+
renames: map[string]string{"name": "__graphite__"},
320+
wantStep: "30",
321+
wantPromQL: `{hostname="web1", env="prod"}`,
322+
},
323+
{
324+
name: "no renames, same as SeriesByTagToPromQL",
325+
step: "60",
326+
target: "seriesByTag('__name__=~cpu.*','env=prod')",
327+
renames: nil,
328+
wantStep: "60",
329+
wantPromQL: `{__name__=~"cpu.*", env="prod"}`,
330+
},
331+
}
332+
333+
for _, tt := range tests {
334+
t.Run(tt.name, func(t *testing.T) {
335+
gotStep, gotPromQL := SeriesByTagToPromQLWithRenames(tt.step, tt.target, tt.renames)
336+
assert.Equal(t, tt.wantStep, gotStep)
337+
assert.Equal(t, tt.wantPromQL, gotPromQL)
338+
})
339+
}
340+
}

zipper/protocols/victoriametrics/fetch.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func (c *VictoriaMetricsGroup) Fetch(ctx context.Context, request *protov3.Multi
3333
logger := c.logger.With(zap.String("type", "fetch"), zap.String("request", request.String()))
3434
stats := &types.Stats{}
3535
var serverUrl string
36-
if len(c.vmClusterTenantID) > 0 {
36+
if c.vmClusterTenantID != "" {
3737
serverUrl = fmt.Sprintf("http://127.0.0.1/select/%s/prometheus/api/v1/query_range", c.vmClusterTenantID)
3838
} else {
3939
serverUrl = "http://127.0.0.1/api/v1/query_range"
@@ -75,8 +75,11 @@ func (c *VictoriaMetricsGroup) Fetch(ctx context.Context, request *protov3.Multi
7575
// Make local copy
7676
stepLocalStr := target.step
7777
if strings.HasPrefix(target.name, "seriesByTag") {
78-
target.name = strings.ReplaceAll(target.name, "'name=", "'__name__=")
79-
stepLocalStr, target.name = helpers.SeriesByTagToPromQL(stepLocalStr, target.name)
78+
// VictoriaMetrics stores Graphite metric paths in the __graphite__ label,
79+
// so rename the Graphite "name" tag accordingly before building PromQL.
80+
stepLocalStr, target.name = helpers.SeriesByTagToPromQLWithRenames(stepLocalStr, target.name, map[string]string{
81+
"name": "__graphite__",
82+
})
8083
} else {
8184
target.name = fmt.Sprintf("{__graphite__=%q}", target.name)
8285
}

0 commit comments

Comments
 (0)