cactus / go-statsd-client Goto Github PK

I've been doing some test with this package and for some reason, when I try to send stat data, no connection is created (I know this because I'm not even running a server), but no error is returned.

package main

import (
	"fmt"

	statsd "github.com/cactus/go-statsd-client/statsd"
)

func main() {

	conf := statsd.ClientConfig{
		Address: "127.0.0.1:8125",
		Prefix:  "",
	}

	client, err := statsd.NewClientWithConfig(&conf)
	if err != nil {
		fmt.Printf("%+v", err)
		panic(err)
	}

	err = client.Inc("test", 1, 1.0)
	if err != nil {
		fmt.Printf("%+v", err)
		panic(err)
	}
}

When this is run, no error is output.

There is definately no server running:

» telnet 127.0.0.1 8125
Trying 127.0.0.1...
telnet: connect to address 127.0.0.1: Connection refused
telnet: Unable to connect to remote host

I'm not sure what is going on here, any help appreciated. I'm using 3.2.0 version of the package:

[[constraint]]
  name = "github.com/cactus/go-statsd-client"
  version = "3.2.0"

I have a general question about the locking mechanism and was wondering if you could shed light on your intentions around this:
https://github.com/cactus/go-statsd-client/blob/master/statsd/sender_buffered.go#L64-L69

Specifically, what kind of situation are you protecting against with the read lock?

Thanks in advance.

Right now the message formatting occurs before we decide whether the sample rate will allow the current message to be sent.

i.e.

func (s *Client) Inc(stat string, value int64, rate float32) error {
    dap := fmt.Sprintf("%d|c", value) <-- causes CPU work and probably creates garbage
    return s.Raw(stat, dap, rate) // <----- this might do nothing!
}

I'm assuming that calculating the random sample is faster and causes no GC [citation of benchmark needed ;) ], compared to string formatting, so applications will benefit from having the sample rate checked before formatting on stats that are sent very frequently.

This will also allow client.Raw to possibly do all formatting in one go, further reducing garbage and CPU.

Again, if you're interested I'll just go ahead and implement it.

@ivaxer has made some additions to your project:
https://github.com/ivaxer/statsd

Notably, it has added methods with a default sampling of 1.0.

Similar to #43 but did not want to try to revive a year old closed ticket. I also have some additional information on the problem and what I think is the solution.

Since go.mod is not at the root of the project, the required tag for the semver would be statsd/v3.2.1, whereas the current release tag is v3.2.1.

Each module has its own version information. Version tags for modules below the root of the repository must include the relative directory as a prefix. (source)

Additional Context

This is the error when trying to use v3 with go modules, which is looking for the missing tag statsd/v3.2.1.

reading github.com/cactus/go-statsd-client/statsd/statsd/go.mod at revision statsd/v3.2.1: unknown revision statsd/v3.2.1

When using the buffered method each cmd added to the buffer ends with a newline character.

This caused the etsy statsd server and log zillions msgs like:

5 Oct 15:51:11 - DEBUG: Bad line: 1 in msg ""
5 Oct 15:51:11 - DEBUG: Bad line: 1 in msg ""
5 Oct 15:51:11 - DEBUG: Bad line: 1 in msg ""
5 Oct 15:51:11 - DEBUG: Bad line: 1 in msg ""
5 Oct 15:51:11 - DEBUG: Bad line: 1 in msg ""
5 Oct 15:51:11 - DEBUG: Bad line: 1 in msg ""
5 Oct 15:51:11 - DEBUG: Bad line: 1 in msg ""

Could you strip the last newline when flushing the buffer?

I would like to propose a new method for the Statter interface, that would create a SubStatter.
A SubStatter would use the same Sender as the parent Statter but would have it's own prefix.
This would allow users to add instrumentation to utility functions that are used by numerous packages with out having to create a new client.
The Sub method that creates the SubStatter would append an additional prefix to the existing Statter prefix.

I would be happy to implement this if you think it is worthwhile.

Two calls to Gauge with the values -1 and -2 will result in the metric having value -3, not -2 as intended.

This is really statsd's fault, as noted at https://github.com/etsy/statsd/blob/master/docs/metric_types.md#gauges where it says "This implies you can't explicitly set a gauge to a negative number without first setting it to zero."

I suggest that the Gauge implementation should set the metric to zero first when the desired value is negative.

I am happy to work on a PR if you think this is a good idea.

I note that our Gauge() function takes an int64 value, and thus cannot represent floating-point values, and I have an application which uses floating point values to represent a fraction between 0 and 1. I also note that at least one other library takes a float64 for the gauges value (see: https://github.com/DataDog/datadog-go/blob/v5.0.2/statsd/statsd.go#L504). So, I'm curious if you would consider the addition of a new function with the following signature, or the like:
func (s *Client) FloatGauge(stat string, value float32, rate float32, tags ...Tag) error

If so, I might be able to PR it.

Statsd provides a TCP based API. It is preferable for many to use it because using UDP for sending metrics will cause loss of metrics for the hosts that you most want data from.

Sadly its spelled 'gauge' and not 'guage'.

Hey!

What's the ETA on a 3.1.0 release? There are quite some interesting commits after the latest 3.0.x release: v3.0.3...master.

like this:

https://github.com/DataDog/datadog-go/blob/master/statsd/statsd.go#L176

I would like to be able to send metrics to statsd with a sample rate attached, without the client actually doing the random sampling. That sounds a little weird, so here are a couple motivating examples, both of which I deal with in real life:

1. In one case, the sampling happens upstream of me. I'm sending access logs across the network, parsing them and sending the data to a local statsd server. I want to avoid sending too many megabits of log lines across the network so I apply sampling at the remote end. But I would still like the numbers in statsd to be accurate. Since I know the upstream sampling rate, I want to pass it on in the statsd metrics.
2. In another case, the data is local, but the parsing is expensive. Instead of parsing every line, generating the metrics, and then throwing away 90% of them, I would like to parse 10% of the lines, generate metrics for every one I do parse, and then, again, send them with with @0.1 attached.

I'm not sure of the best way to add this to the interface, which is why I haven't gone about sending a PR, but perhaps a client could just have a sample rate multiplier field, so that the rate passed into includeStat is still the same rate passed to Inc/Timing/etc, but submit receives rate * s.SampleMultiplier.

Can this client be used across go routines without instantiating it in every go routine?

The Statter API has:
Timing(stat string, delta int64, rate float32) error

And it expects delta to be in milliseconds. However, the protocol supports floating point numbers, and in a Python library I'm using sub-ms timings and it works perfectly fine with Graphite.

I'm dealing with sub millisecond timings in Go now, and I can't get accurate reading because of this.

Ideally, the Statter's API should look like this IMO:
Timing(stat string, delta time.Duration, rate float32) error
And internally should convert to float for low numbers, and to int for larger ones.

However, in order not to break the API, I'd add a separate method for it, e.g:
TimingDuration(stat string, delta time.Duration, rate float32) error

Anyway, if you're interested I'd be happy to implement and add a PR. let me know.
Thanks :)

hi,

I have a question to SetInt, lets say I have a for loop and call SetInt multiple times, what value gets displayed in Grafana? my Grafana refreshes a value every 30 sec.

SetInt("someMetrics", 10, 1.0)
SetInt("someMetrics", 15, 1.0)
SetInt("someMetrics", 7, 1.0)
SetInt("someMetrics", 9, 1.0)
SetInt("someMetrics", 11, 1.0)

Looking at the statsd protocol (<metricname>:<value>|<type>) and empirical testing suggests that value should be able to be any number. This client supports only int values -- intentional decision or just haven't had a need for floats?

for example. When I send using

client, _ := statsd.NewClient("localhost:8125", "prefix")
stat.Timing("event", 42, 1.0)

I wonder what is the key and value we actually send
Is there any way or document to know it?

Hi- I was wondering why the interface for Gauge doesn't accept float64.

In my poking around for #27 I noticed that the Statter interface doesn't expose SetSamplerFunc -- is there any reason for this?

If there isn't it seems reasonable to reflect that the base Statter also has a sampler that can be configured.

Hi,
I noticed you recently marked the "NoopClient" as deprecated with note to use a "nil *Client" instead. We are using a NoopClient in unit tests to ignore the events, but trying to use a nil *Client in the same way as a NoopClient causes a panic because it tries to call methods on the nil pointer.
Creating a client with an empty config generates errors when actually writing because it tries sending to the empty address, which show up in our unit test logs because we log errors from the various methods.
Am I misunderstanding the recommendation, or is there another way to get similar functionality?

Thanks!
Doug

Hi,

I ran into the following issue using this client with go and modules.

build github.com/my/service: cannot load github.com/cactus/go-statsd-client/statsd: ambiguous import: found github.com/cactus/go-statsd-client/statsd in multiple modules:
    github.com/cactus/go-statsd-client v3.1.1+incompatible (/Users/jlindamo/go/pkg/mod/github.com/cactus/[email protected]+incompatible/statsd)
    github.com/cactus/go-statsd-client/statsd v0.0.0-20190501063751-9a7692639588 (/Users/jlindamo/go/pkg/mod/github.com/cactus/go-statsd-client/[email protected])

This doesn't happen if you're directly referencing this client, but can if multiple libraries are referencing the client in different ways and you're trying to use both libraries at once.

I don't think the change to add the go.mod file was correct: https://github.com/cactus/go-statsd-client/blob/master/statsd/go.mod

With go.mod, since you're on tag v3, the path of everything inside there should be github.com/cactus/go-statsd-client/statsd/v3 instead of github.com/cactus/go-statsd-client/statsd. That includes the go.mod file as well.

It may be best to either add v3 to the path in the module file, or remove the go.mod file.

I'm toying around with what an impl for this might look like but wanted feedback before spending much time on it.

For the most part I'm not super worried in being able to configure the sample rate for each individual stat that I'm collecting -- there are likely to be some things on the hot path that I'll want to turn down and some low-volume-but-essential things I need 100% visibility. It would be nice if there was a way to build a Statter with preset sample rates so that when you call Inc etc you don't need to provide your own sample rate.

Expressing this as code (pretending we get method overloading):

func (s *Client) Inc(stat string, value int64, rate float32) error { ... }

func (s *Client) Inc(stat string, value int64) error {
    return s.Inc(stat, value, s.sampleRate)
}

Is this something you would not want merged in or is it worth poking at a bit more?

Since the last issue over two years ago (#38), Graphite, Datadog, and InfluxDB have all accepted tags as relatively important functionality, with slightly different formatting between them (example: https://github.com/smira/go-statsd/pull/29/files). Would it be possible to support this?

We use DNS to resolve a statsd endpoint IP. If the IP address behind the DNS changes, go-statsd-client will never use the new IP. This means metrics will fail until the application restarts. Ideally if the IP address behind a DNS name changes, the sender will send stats to the new IP.

It seems to start at this line:

	ra, err := net.ResolveUDPAddr("udp", addr)

Once ra is resolved, it is never re-resolved. There are a few options for this.

1. Add a function to SimpleSender that allows me to update ra. I could then call this if I detect a change. This won't fix the problem for future people, but will let me work around it.
2. Change SimpleSender to execute net.ResolveUDPAddr("udp", addr) each time on Send: this could cause a performance problem. Optionally, we could only update ra every X units of time.
3. Modify BufferedSender to take a Sender as input. I could then pass my own sender, instead of using the provided one.

Also, any way we change this, we'll need to extend BufferedSender to call this function on the SimpleSender, or allow BufferedSender to return the wrapped Sender, and I could call it myself on the BufferedSender object.

The way you are handling sampling rates is not intuitive to me. You should probably mention in your documentation that you are randomly dropping requests that the client is making based on the rate being specified. Really this seems like something that you should be leaving to individuals who are using your library rather than baking it in.

I ran into an issue where I was sending stats to a statname with a host IP/port where I replaced the . in the IP with _ but forgot the port delimiter :, like so: stats.gauges.<service>.production.11_22_33_44:6800

Because the : is treated as a delimiter in statsd, it was ignoring the gauge value I passed in and logging the statname stats.gauges.<service>.production.11_22_33_44 with the value 6800. Should the statsd client reject a statname with a : (and possibly other delimiters I may not be aware of)?

Hi,

Is statsd compatible with brubeck (https://github.com/github/brubeck)?

Thank you for reference statsd client.

Please, consider merging safe.go from g2s project. It may not look like a big deal, but it makes a difference.

s, err := g2s.Dial("udp", someEndpoint)
if err != nil {
    log.Printf("not sending statistics to statsd (%s)", err)
    s = g2s.Noop()
}

Hi,

I noticed that your client does not have any version number or official release number for it. Would it be possible for you guys to tag the code with a version number or create a zip file with version number?

Thanks,
--Tejas