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

feat: expose BatchCounter for MetricBatcher #28

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 5 additions & 3 deletions metrics/metrics.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
// Package metrics provides a simple API for sending value and counter metrics
// through the dropsonde system.
//
// Use
// # Use
//
// See the documentation for package dropsonde for configuration details.
//
// Importing package dropsonde and initializing will initial this package.
// To send metrics use
//
// metrics.SendValue(name, value, unit)
// metrics.SendValue(name, value, unit)
//
// for sending known quantities, and
//
// metrics.IncrementCounter(name)
// metrics.IncrementCounter(name)
//
// to increment a counter. (Note that the value of the counter is maintained by
// the receiver of the counter events, not the application that includes this
Expand All @@ -21,6 +21,7 @@ package metrics

import (
"github.com/cloudfoundry/dropsonde/metric_sender"
"github.com/cloudfoundry/dropsonde/metricbatcher"

Choose a reason for hiding this comment

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

Suggested change
"github.com/cloudfoundry/dropsonde/metricbatcher"
"metricBatcher github.com/cloudfoundry/dropsonde/metricbatcher"

Copy link

Choose a reason for hiding this comment

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

I agree with @b1tamara, the imported package should not be aliased in camel case notation.

@Dariquest Good package names are short and clear. They are lower case, with no under_scores or mixedCaps.. Check the whole blogpost on package names for more information ;)

Copy link

@Dariquest Dariquest Jan 15, 2025

Choose a reason for hiding this comment

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

Thanks for the blogpost @chombium. The name is not longer and more readable in camel case. Only imho. My suggestion might be more appropriate for other programming languages then Go, which I am used to :)

The package names should be at least unified, what I can find here is unfortunately:

import (
	"github.com/cloudfoundry/dropsonde/metric_sender"
	"github.com/cloudfoundry/dropsonde/metricbatcher"

Choose a reason for hiding this comment

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

@Dariquest I know what you mean and I would completely agree if it wasn't Go. Go has some different conventions. On the other hand, we have the same import used like here in many places in other CF components.

"github.com/cloudfoundry/sonde-go/events"
)

Expand Down Expand Up @@ -52,6 +53,7 @@ type MetricBatcher interface {
BatchIncrementCounter(name string)
BatchAddCounter(name string, delta uint64)
Close()
BatchCounter(name string) metricbatcher.BatchCounterChainer

Choose a reason for hiding this comment

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

Suggested change
BatchCounter(name string) metricbatcher.BatchCounterChainer
BatchCounter(name string) metricBatcher.BatchCounterChainer

Choose a reason for hiding this comment

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

the camel case is lost here, it does not correspond to the other code

Copy link
Author

Choose a reason for hiding this comment

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

as it is imported library name, I will not change it.

Choose a reason for hiding this comment

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

The name is not longer and more readable in camel case, that' s why I would prefer that. It was only a suggestion, of course you are free to keep it.

}

// Initialize prepares the metrics package for use with the automatic Emitter.
Expand Down
8 changes: 8 additions & 0 deletions metrics/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,4 +142,12 @@ var _ = Describe("Metrics", func() {
Consistently(newMetricBatcher.CloseCalled).ShouldNot(BeCalled())
})
})

Context("BatchCounter", func() {
It("gets a name", func() {
metricBatcher.BatchCounterOutput.Ret0 <- nil
metricBatcher.BatchCounter("test")
Eventually(metricBatcher.BatchCounterInput).Should(BeCalled(With("test")))
})
})
})
61 changes: 60 additions & 1 deletion metrics/mock_metric_batcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@

package metrics_test

import (
"github.com/cloudfoundry/dropsonde/metricbatcher"
)

type mockMetricBatcher struct {
BatchIncrementCounterCalled chan bool
BatchIncrementCounterInput struct {
Expand All @@ -15,7 +19,14 @@ type mockMetricBatcher struct {
Name chan string
Delta chan uint64
}
CloseCalled chan bool
CloseCalled chan bool
BatchCounterCalled chan bool
BatchCounterInput struct {
Name chan string
}
BatchCounterOutput struct {
Ret0 chan metricbatcher.BatchCounterChainer
}
}

func newMockMetricBatcher() *mockMetricBatcher {
Expand All @@ -26,6 +37,9 @@ func newMockMetricBatcher() *mockMetricBatcher {
m.BatchAddCounterInput.Name = make(chan string, 100)
m.BatchAddCounterInput.Delta = make(chan uint64, 100)
m.CloseCalled = make(chan bool, 100)
m.BatchCounterCalled = make(chan bool, 100)
m.BatchCounterInput.Name = make(chan string, 100)
m.BatchCounterOutput.Ret0 = make(chan metricbatcher.BatchCounterChainer, 100)
return m
}
func (m *mockMetricBatcher) BatchIncrementCounter(name string) {
Expand All @@ -40,3 +54,48 @@ func (m *mockMetricBatcher) BatchAddCounter(name string, delta uint64) {
func (m *mockMetricBatcher) Close() {
m.CloseCalled <- true
}
func (m *mockMetricBatcher) BatchCounter(name string) metricbatcher.BatchCounterChainer {
m.BatchCounterCalled <- true
m.BatchCounterInput.Name <- name
return <-m.BatchCounterOutput.Ret0
}

type mockBatchCounterChainer struct {
SetTagCalled chan bool
SetTagInput struct {
Key, Value chan string
}
SetTagOutput struct {
Ret0 chan metricbatcher.BatchCounterChainer
}
IncrementCalled chan bool
AddCalled chan bool
AddInput struct {
Value chan uint64
}
}

func newMockBatchCounterChainer() *mockBatchCounterChainer {
m := &mockBatchCounterChainer{}
m.SetTagCalled = make(chan bool, 100)
m.SetTagInput.Key = make(chan string, 100)
m.SetTagInput.Value = make(chan string, 100)
m.SetTagOutput.Ret0 = make(chan metricbatcher.BatchCounterChainer, 100)
m.IncrementCalled = make(chan bool, 100)
m.AddCalled = make(chan bool, 100)
m.AddInput.Value = make(chan uint64, 100)
return m
}
func (m *mockBatchCounterChainer) SetTag(key, value string) metricbatcher.BatchCounterChainer {
m.SetTagCalled <- true
m.SetTagInput.Key <- key
m.SetTagInput.Value <- value
return <-m.SetTagOutput.Ret0
}
func (m *mockBatchCounterChainer) Increment() {
m.IncrementCalled <- true
}
func (m *mockBatchCounterChainer) Add(value uint64) {
m.AddCalled <- true
m.AddInput.Value <- value
}