Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 4 additions & 0 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ type ContainerMetric struct {
AbsoluteCPUUsage uint64
AbsoluteCPUEntitlement uint64
ContainerAge uint64
RxBytes uint64
TxBytes uint64
Tags map[string]string
}

Expand Down Expand Up @@ -223,6 +225,8 @@ func (c client) SendAppMetrics(m ContainerMetric) error {
loggregator.WithGaugeValue("disk", float64(m.DiskBytes), "bytes"),
loggregator.WithGaugeValue("memory_quota", float64(m.MemoryBytesQuota), "bytes"),
loggregator.WithGaugeValue("disk_quota", float64(m.DiskBytesQuota), "bytes"),
loggregator.WithGaugeValue("rx_bytes", float64(m.RxBytes), "bytes"),
loggregator.WithGaugeValue("tx_bytes", float64(m.TxBytes), "bytes"),
Copy link
Contributor

Choose a reason for hiding this comment

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

In the demo of this, it looked like rx_bytes + tx_bytes were cumulative counters (value increasing over the life of the interface/container), rather than gauges (point-in-time values of usage/consumption).

Does it make sense to submit these via loggregator's EmitCounter logic instead?

Copy link
Member

Choose a reason for hiding this comment

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

On the metrics side we prefer modeling everything as counters instead of gauges when possible. When using a counter a lost datapoint results in lower precision, while when using a gauge a lost datapoint is lost forever.

Specifically in this case it's often useful to know how much data a container has transmitted over a given window, which counters make possible.

Copy link
Contributor Author

@geigerj0 geigerj0 Aug 10, 2023

Choose a reason for hiding this comment

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

Very good points from both of you!

We've changed the tx_bytes and rx_bytes to be sent as counters 👍 . See https://github.com/cloudfoundry/diego-logging-client/pull/82/files#diff-4b667feae66c9d46b21b9ecc19e8958cf4472d162ce0a47ac3e8386af8bbd8cfR246-R253

Example from having this deployed to one of our dev landscapes:

2023-08-10T13:27:50.27+0000 [ping/0] COUNTER rx_bytes:24842
2023-08-10T13:27:50.27+0000 [ping/0] COUNTER tx_bytes:24306

loggregator.WithEnvelopeTags(m.Tags),
)

Expand Down
13 changes: 13 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,10 @@ var _ = Describe("DiegoLoggingClient", func() {
AbsoluteCPUUsage: 1,
AbsoluteCPUEntitlement: 2,
ContainerAge: 3,

RxBytes: 42,
TxBytes: 43,

Tags: map[string]string{
"source_id": "some-source-id",
"instance_id": "345",
Expand Down Expand Up @@ -332,6 +336,15 @@ var _ = Describe("DiegoLoggingClient", func() {
Expect(metrics["container_age"].GetUnit()).To(Equal("nanoseconds"))
})

It("sends network traffic usage", func() {
metrics := batch.Batch[0].GetGauge().GetMetrics()
Expect(metrics["rx_bytes"].GetValue()).To(Equal(float64(42)))
Expect(metrics["rx_bytes"].GetUnit()).To(Equal("bytes"))

Expect(metrics["tx_bytes"].GetValue()).To(Equal(float64(43)))
Expect(metrics["tx_bytes"].GetUnit()).To(Equal("bytes"))
})

It("sends tags", func() {
Expect(batch.Batch).To(HaveLen(1))
Expect(batch.Batch[0].GetTags()).To(Equal(map[string]string{
Expand Down