Skip to content

Fix target_info labels #2546

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.

Sign up for GitHub

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

Merged
ie-pham merged 13 commits into from
Jun 13, 2023
Merged

Fix target_info labels #2546

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
728f91d
testing
ie-pham Jun 8, 2023
eb11226
fix gauge labels
ie-pham Jun 8, 2023
133b314
lint
ie-pham Jun 8, 2023
e6a7722
remove test log
ie-pham Jun 8, 2023
b0ee1fb
CHANGELOG
ie-pham Jun 8, 2023
e521f16
git oops
ie-pham Jun 8, 2023
c703db1
oops again
ie-pham Jun 8, 2023
fe27d0b
address comments
ie-pham Jun 8, 2023
6bb6719
oops
ie-pham Jun 8, 2023
7e75e6f
add test
ie-pham Jun 9, 2023
809beed
fix counter as well just in case
ie-pham Jun 9, 2023
91ff984
order
ie-pham Jun 9, 2023
a28cb6d
i hate this
ie-pham Jun 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ storage:
* [CHANGE] Ignore context canceled errors in the queriers [#2440](https://github.com/grafana/tempo/pull/2440) (@joe-elliott)
* [CHANGE] Start flush queue worker after wal replay and block rediscovery [#2456](https://github.com/grafana/tempo/pull/2456) (@ie-pham)
* [CHANGE] Update Go to 1.20.4 to match GET [#2486](https://github.com/grafana/tempo/pull/2486) (@ie-pham)
* [BUGFIX] Fix issue where metrics-generator was setting wrong labels for traces_target_info [#2546](https://github.com/grafana/tempo/pull/2546) (@ie-pham)

## v2.1.1 / 2023-04-28
* [BUGFIX] Fix issue where Tempo sometimes flips booleans from false->true at storage time. [#2400](https://github.com/grafana/tempo/issues/2400) (@joe-elliott)
Expand Down
26 changes: 11 additions & 15 deletions modules/generator/registry/gauge.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,31 +118,27 @@ func (g *gauge) collectMetrics(appender storage.Appender, timeMs int64, external

activeSeries = len(g.series)

labelsCount := 0
if activeSeries > 0 && g.series[0] != nil {
labelsCount = len(g.series[0].labels.names)
}
lbls := make(labels.Labels, 1+len(externalLabels)+labelsCount)
lb := labels.NewBuilder(lbls)

// set metric name
lb.Set(labels.MetricName, g.metricName)
// set external labels
for name, value := range externalLabels {
lb.Set(name, value)
}

for _, s := range g.series {
t := time.UnixMilli(timeMs)
lbls := make(labels.Labels, 1+len(externalLabels)+len(s.labels.names))
Copy link
Copy Markdown
Contributor

@zalegrala zalegrala Jun 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to keep the allocations outside of the loop. I think I saw a Reset() method on the lb object that might empty this out.

Copy link
Copy Markdown
Contributor Author

@ie-pham ie-pham Jun 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call

lb := labels.NewBuilder(lbls)

// set metric name
lb.Set(labels.MetricName, g.metricName)
// set external labels
for name, value := range externalLabels {
lb.Set(name, value)
}

// set series-specific labels
for i, name := range s.labels.names {
lb.Set(name, s.labels.values[i])
}

_, err = appender.Append(0, lb.Labels(nil), t.UnixMilli(), s.value.Load())
if err != nil {
return
}

// TODO support exemplars
}

Expand Down