Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not use . in labels in the flatten function of converter.go #10

Closed
kyasbal opened this issue Nov 18, 2023 · 2 comments
Closed

Do not use . in labels in the flatten function of converter.go #10

kyasbal opened this issue Nov 18, 2023 · 2 comments

Comments

@kyasbal
Copy link

kyasbal commented Nov 18, 2023

I'm using slog-loki just for my personal project. I've tried the newer version with updating the client of loki before merging #5 .

I found slog-loki might need to convert label names in different way.

slog-loki/converter.go

Lines 39 to 55 in 7da5375

func flatten(prefix string, src map[string]any, dest model.LabelSet) {
if len(prefix) > 0 {
prefix += "."
}
for k, v := range src {
switch child := v.(type) {
case map[string]any:
flatten(prefix+k, child, dest)
case []any:
for i := 0; i < len(child); i++ {
dest[model.LabelName(prefix+k+"."+strconv.Itoa(i))] = model.LabelValue(fmt.Sprintf("%v", child[i]))
}
default:
dest[model.LabelName(prefix+k)] = model.LabelValue(fmt.Sprintf("%v", v))
}
}
}

The function is using . to express enclosed slog.Attrs. But this seems not to be allowed in loki[1].

I tweaked that to use _ in my usecase and it's easy to send a PR for this fix. But this solution might be not the ideal that the auhor of this library think of.

[1]https://grafana.com/docs/loki/latest/get-started/labels/#format

@samber
Copy link
Owner

samber commented Nov 18, 2023

Hi @kyasbal and thanks for the bug report.

I would suggest __ separators (double _), to prevent collision with user defined _.

Using : would be even better IMO, but Prometheus recommends not using it. Moreover it looks like it is only available for __name__ label name...

@samber
Copy link
Owner

samber commented Dec 24, 2023

added in v3.1.0

@samber samber closed this as completed Dec 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants