Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

add constant labels to gauges and cumulative metrics #1122

Merged
merged 4 commits into from
Apr 23, 2019
Merged

add constant labels to gauges and cumulative metrics #1122

merged 4 commits into from
Apr 23, 2019

Conversation

paivagustavo
Copy link
Contributor

This PR adds support for constant labels on gauges.

The proposed syntax is:

r := NewRegistry()

 	f, _ := r.AddFloat64Gauge("TestGaugeWithConstLabel",
		WithLabelKeys("k1"), WithConstLabel("const", "same"), WithConstLabel("const2", "same2"))

@rghetia this is my first contribution and as you were the one who created the issue, could you review this?

closes #1112

Copy link
Contributor

@rghetia rghetia left a comment

Choose a reason for hiding this comment

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

@paivagustavo thanks for the PR.

we are making commits to dev branch for now. I'll merge the dev branch to master just before the next release.
So can you please rebase your change to dev?

When you rebase, you will have conflict as I have created LabelKey type which contains key and description. So you will have to change that accordingly.

metric/registry.go Outdated Show resolved Hide resolved
@paivagustavo paivagustavo changed the base branch from master to dev April 23, 2019 02:01
@paivagustavo
Copy link
Contributor Author

@rghetia I've rebased to dev and change the target branch (I may have pick some commits from the master that was not on dev yet, is it a problem?)

While doing this, I noticed that the canonicalize was not deterministic and sometimes the TestGauge was failing, I have fixed this and made it more reliable while keeping the same purpose.

@rghetia
Copy link
Contributor

rghetia commented Apr 23, 2019

@rghetia I've rebased to dev and change the target branch (I may have pick some commits from the master that was not on dev yet, is it a problem?)

yes, that will be a problem. you can do git rebase --onto dev HEAD~2 since you two commits.

@paivagustavo paivagustavo changed the title add constant labels to gauges add constant labels to gauges and cumulative metrics Apr 23, 2019
@paivagustavo
Copy link
Contributor Author

Done, the PR now contains only the correct commits.

Copy link
Contributor

@rghetia rghetia left a comment

Choose a reason for hiding this comment

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

one minor nit. Otherwise LGTM.

metric/cumulative_test.go Outdated Show resolved Hide resolved
@rghetia rghetia merged commit 8986fa9 into census-instrumentation:dev Apr 23, 2019
rghetia pushed a commit to rghetia/opencensus-go that referenced this pull request Apr 25, 2019
…ntation#1122)

* Remove unused GetEntry.

* adds support for constant labels on Gauge and CumulativeMetric

* fixing format on tests.

* remove unused getentry
rghetia pushed a commit that referenced this pull request Apr 25, 2019
* Remove unused GetEntry.

* adds support for constant labels on Gauge and CumulativeMetric

* fixing format on tests.

* remove unused getentry
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants